DOM

Properder: If ((MatKleur = "Z") & (LadeDiepte > 450))
Dat geeft at runtime exact hetzelfde resultaat, maar is 1 If ipv een geneste If, dus leesbaarder.

Idem voor die hieronder...

Hier een beetje hetzelfde... nieuwe code, ook al is het gebaseerd op een copy/paste, graag refactoren volgens huidige conventies. Je hoeft het nu niet meer aan te passen. Onthoud het voor de volgende keer.

"$lb" = $ListBuild
"$lg" = $ListGet
"n" = New
"s" = Set
"q" = Quit
Wat spaties hier en daar voor en na =-teken en na komma en zo.

Opletten voor casing graag

Variabele-namen beginnen met hoofdletter (Msg, Bool).
Misschien de variabele Bool dan ineens hernoemen naar het meer beschrijvende: IsValid.
Set met hoofdletter.

P.s.: OMG wat een klasse!!! lol Je hoeft niet de hele klasse aan te passen in dit geval (veel te veel werk), maar als je code toevoegt, zet die dan wel ineens volgens onze huidige conventies.

Als alle lijnen in comment staan, misschien toch een extra comment-lijntje toevoegen met de reden waarom. Ik vermoed in afwachting van dat de code actief moet gezet worden? Maar de call zelf staat toch in comment in de klasse APPS.EC.impl.ConfiguratorService.KenmerkMogelijkhedenPostProcessor?

Anderzijds... moet je hier niet enkel diegene voorzien die in klasse APPS.EC.impl.ConfiguratorService.KenmerkMogelijkhedenPostProcessor nog niet voorzien waren?

Louter ter info: Ook nog even meegeven dat iedere enum, die dus per definitie af moet leiden van TECH.Enumeration, automatisch daarom ook beschikt over een ValueListIterator. Als je dus alle enumwaarden van een enum moet overlopen, dan is gebruik maken van die iterator wellicht overzichtelijker dan voor iedere waarde een codelijn te voorzien.

Idem voor de cleanup van de UTDataVoorKvik --> aparte method
Hier zelfs geen parameters nodig, denk.

Ik heb lang gezocht naar het verschil in setup tussen deze method en de method erboven.
Door de setup (de lijnen hierboven) in een aparte private method te zetten wordt het onderscheid meteen duidelijk:

bvb. Do ..SetUpUTDataVoorKvik ( PalletIdVoorEWMS, PalletIdVoorEWPAL )

En hierdoor worden de 2 tests meteen een pak overzichtelijker

MethodNaam volgens vh-conventies : ZetOpEWPAL() en KuisOpEWPAL() en ZetOpEWMSData()

Early quit :-O
Gelukkig gaat ie binnenkort wel weg.

Als reviewer ben ik verplicht om dit aan te geven :

embedded sql aanpassen :
beter : wegwerken en vervangen door TECH.Resultset
reden: gedrag is beter onder controle bij errors, early quits, onvoorspelbaar gedrag indien 2 sql's gemixed worden (lees op andere stacklevel een andere &sql doorlopen), compile-issues.

In dit specifiek geval zou ik misschien toch ook de &sql behouden.

Zeer goed dat je deze lijn erbij zet!

Count() is niet nodig, want als Count=0 , dan zal Find() ook niks vinden.

DOM.PM.enu.LinkType (hieronder nog een paar keer) (en ook in ander testje)

my point exactly (zie comment).
Afzonderen naar apart klaske dat getest kan worden

Boyscout principe: er bestaan geen testjes van deze klasse. Misschien is het niet veel moeite om een paar dingen toe te voegen?

Op den duur zou een mens zich beginnen afvragen of al deze StartVerwerking-code niet beter in een aparte klasse gestoken wordt. Ooit is dat begonnen als een klein methodje dat niet te veel doet (juist wat parameters zetten enzo) en dan was het geen probleem dat het niet in een niet-testbare classmethod steekt. Met al de extra logica die er ondertussen is bijgekomen is dat eigenlijk niet goed meer.