AXimpl

Is dit niet 'gevaarlijk'? Moet je niet checken of er 1 product aanwezig is en anders een error gooien?

Is écht muggenziften, maar deze 'if' mag op lijn 566. Ik durf het bijna niet typen :-p

algemene opmerking over onderstaande tests:
Ze bevinden zich op het randje van 'te lang'. Misschien zou je kunnen overwegen om de setup van de tests (zeker het maken van de mockobjecten, die op het eerste zicht overal dezelfde zijn), in een BeforeOneTest te steken. Alle Verifieers kan je ook in een private method kwijt.
Soit, dit is maar een voorstel hoor. De lijn is soms moeilijk te bepalen. Stukken code herhalen is op zich niet fout hé, zeker als de rest van de (test)method vrij kort is. Het voordeel van er een beetje generieke dingen uit te peuteren is dat de verschillen tussen de overgebleven code duidelijk kunnen opvallen.

typo: Service ipv Serive

Logger injecteren zodat ge in uw tests kunt verifiëren dat ie opgeroepen wordt als er iets is misgegaan.

Bij nakijken van tests gerealiseerd dat dat misschien voor problemen zorgt als er gerethrowed wordt, maar ik weet het niet exact meer.

#dim A As X = ##class(X).%New()
vervangen door
Set A = ##class(X).%New()

Die stijl heeft de voorkeur bij constructie van nieuwe objecten, zeker als ze een lange naam hebben. Stel dat de %New() nog wat parameters nodig heeft, dan is de kans groot dat ze rechts van het scherm vallen, wat de code minder leesbaar maakt.

Deze opmerking kunt ge hieronder nog een paar gebruiken (en misschien in andere klasses ook, zover ben ik nog niet)

weet ge al of het nog nodig is of niet? Indien niet nodig -> wegsmijten

normaalgezien zetten we geen comments in de code. In principe wil dat zeggen dat het niet duidelijk is wat de code doet. Misschien is het beter om de verschillende blokken in de method op te splitsen in private methods en in je main method krijg je dan iets als:

Do ..HaalMarkupOpVoorPersonality
Do ..GeefMessageId
Do ..MaakRequest
... enz enz

Op die manier is de BerekenProductPrijzen-method korter en leesbaarder.
Heeft ook het voordeel dat, bij een mogelijke latere uitbreiding de wijziging vrij gelokaliseerd zit in 1 (private) method die indien nodig eenvoudig in een helperklasse met zijn eigen tests afgezonderd kan worden.

als iets al overerft van TECH.RegisteredObject en je maakt het mockable, mag de TECH.RegisteredObject weg uit de parents-lijst, want TECH.Mockable is zelf een TECH.RegisteredObject

zelfde opmerking: aparte parameter gebruiken, zelfs als ie momenteel gelijk is.

aparte parameter maken voor Orgalux. Kan zijn dat dat nu toevallig gelijk is aan LBX.

Repository maken over Flow.Uitlevering.Hoofding ? encapsulaties gebruiken waar het kan? overweeg met PRA om te zien of het de investering waard is.
Indien niet gebruik maken van encapsulatie:
nu wordt 2 x $Order uitgevoerd voor één uitlevering. zo ook voor de volgende whiles
Dat kan beter
Na de rest van de code te bekijken : Repository maken (resulteert in één iterator en testbaarheid of meer dan één uitlevering correct afgehandeld worden.

ik zou dit toch over een paar lijnen spreiden, gaat wat leesbaarder worden dan.

Private?

Wat doen we met Data?

grrrrr...

grrrr....

Maak hier een specifieke method voor waarvan de naam betekenis heeft. 63 zegt niet veel. cfr andere opmerking op de impl hiervan.

cfr andere opmerking in implementatie

Lijkt me 2 mooie methods om private te zetten en dan in specifiek genaamde methods voor speciefieke gegevens te gebruiken.
Nu staat er ergens anders in de code een getal 63 te blinken....