OnderdelenPicking

Ik vond het in het begin ook lastig om te snappen hoe het net in zijn werk ging. Ik wist dat het een bepaalde term in het vet zette maar de logica zelf heb ik me niet zo mee bezig gehouden. Het gaat erom om een symbool voor en na het woord te zetten die je in het vet wilt om zo aan de printer duidelijk te maken dat het woord vet is. Dat is eigenlijk een printercommando. Hier ga je dus tussen de 3e en 4e komma alles in het vet zetten

Misschien is het totaal niet nodig, maar ik persoonlijk vind het niet allemaal zo duidelijk wat hier gebeurt. Misschien is het beter om dit wat op te splitsen in private methods? (Is maar een suggestie, maar het duurde wel wat lang om te snappen wat hier exact gebeurt aangezien ik er niets van ken)

Zou dit niet in een aparte method kunnen komen met iets als 'GeefProdDetailLijnOuterDynamicIterator()' aangezien dit duplicate code is met LBX?

Zelfde opmerking al in de vorige review (die je wellicht nog niet verwerkt hebt, waardoor je dit nog niet wist )...
Qua naamgeving i.g.v. een iterator eindigen we met het woord "Iterator", dus eerder iets in de zin van "..GeefAlleProductenBehalveBodemplatenIterator"

Zoals gezegd in de method die deze aanroept... naamgeving is ontoereikend.
Wat de implementatie van deze method betreft... ik zal die reviewen nadat je ze eerst gerefactored hebt, wat dat kan het wel gebruiken
Je hebt hier tot 6 niveau's diep nestingen!! Ik tel 2 While's, 9 Ifs en een Else. Dat is wat veel van het goede binnen één en dezelfde method. Probeer kleine codeblokjes af te zonderen naar private methods, die een specifieke verantwoordelijkheid hebben.
Probeer daarbij ook oog te hebben voor de stukken die eventueel generischer gemaakt kunnen worden, zodat je niet aan code-duplicatie doet. Ik zie daar bijvoorbeeld 2 haast identieke codeblokjes, waarbij er een iterator genewed wordt, de 1e iteratie ervan wordt opgehaald indien die er is, gecheckt wordt of de Count() daarop > 0 en er dan een OnderdelenPickingUitvoeringInfo van wordt opgehaald. Dit leent zich er geweldig toe om af te zonderen met gebruik van generische naamgeving en het doorgeven van louter de juiste list als parameter. Dat zijn zes codelijnen maal twee, die je al kunt vervangen door twee keer één codelijn, bv.:
#dim SequentieNummerOorspronkelijkProduct As %Integer = ..GeefSequentieNummer(OorspronkelijkProduct.PickingsPerMaatwerkProductID)
en
#dim SequentieNummerProduct As %Integer = ..GeefSequentieNummer(Product.PickingsPerMaatwerkProductID)

Waarom een NieuweGesorteerdeUitvoeringInfoIt toevoegen? Is toch niet nodig? Gooi de Else weg, de If zorgt ervoor dat de iterator enkel in de juiste case wordt overschreven.

Minstens even belangrijk: De naamgeving van de method TelAantallenSamenVoorGeleiders laat nogal te wensen over hé, daaruit moet vooral blijken dat ze een iterator teruggeeft en dat blijkt uit deze naam zeker niet.

En ook nog: Tussen de declaratie van lokale variabelen en de eigenlijke logica laten we graag een lege lijn. Dit zorgt voor leesbaarheid