common

Mag gerust ingekort worden naar:
"Test: Zet 0 karakters van korttekst in bold indien TBX"

Idem voor die van TAX hierna.

Ter verduidelijking zou je kunnen werken met 3 lokale variabelen (booleans). Dan kun je iets beschrijvends meegeven aan de method GeefProductDetailLijnen, waardoor degene die na jou komt (of jijzelf als het lang geleden is dat je de code gezien hebt) niet naar die private method moet gaan kijken om te weten waarvoor er booleans worden doorgegeven.

Dit is de assert die er toe doet. Aangezien het hier om stringmanipulatie gaat kun je er best voor proberen zorgen dat ofwel:

• deze zo waarheidsgetrouw mogelijk is door echte kortteksten te gebruiken.
• deze zodanig ineen te steken dat datgene wat getest wordt zo snel en duidelijk mogelijk weergegeven wordt.

Je zou dit, in samenspel met de opmerking over de naamgeving van de test, zo kunnen aanpakken dat je deze kortteksten begint met een kleine nummerreeks, gevolgd door iets beschrijvends. Bv:
"123456789 frontbevestiging Exp T"
Dat zou dan resulteren in:
"123<b>4</b>56<b>7</b>89 frontbevestiging Exp T"
Zo is het direct duidelijk dat karakters 4 en 7 in bold komen te staan bij frontbevestiging Exp T.

Je moet niet testen of het V1 is, want je WEET al dat het V1 is, want je hebt in de setup van de test ervoor gezorgd dat het ZEKER V1 is, dus deze assert mag er overal uit.

"Zet 2 karakters" blijft te algemeen. Je test moet zeggen welke twee karakters, dus bv:
"Test: Zet karakter 4 en 7 van korttekst..."

Naamgeving parameters:
IsV1 i.p.v. IsV1Aanwezig
HeeftFrontBevestiging i.p.v. FrontbevestigingAanwezig

Uitleg:

• We gebruiken elders in de code ook bv IsProbox of IsFlatpackV1. In gedachten zetten we er nog het woord "Verpakking" achter Het woord "Aanwezig" biedt geen meerwaarde.
• Booleans beginnen bij grote voorkeur met "Is" of "Heeft".

Ook bovenstaand codeblok mag wat refactoring ondergaan...
Je checkt 2 keer op HeeftFrontBevestiging, dus daar kun je dan beter eerst een selectie op maken:

If HeeftFrontBevestiging {
    ... Hier dan een genestte If-Else-structuur voor al dan niet FrontbevestigingExandoT
} Else {
    ...
}

P.s.: Geen ElseIf nodig dus, maar als je hem wel gebruikt, dan een grote I alstublieft

Bovenstaand codeblok zou gerefactored moeten worden. Aangezien de helper-methods ook werken met booleans als parameter, hoef je hier geen if-else-structuur te maken. j kunt hier gewoon 2 calls doen en de aangeleverde booleans hieraan meegeven:
Do ProductDetailLijnen.Insert(..GeefProductDetailLijnVerpakking(IsV1Aanwezig)
Do ProductDetailLijnen.Insert(..GeefProductDetailLijnOnderdeel(FrontbevestigingAanwezig, FrontbevestigingExpandoTAanwezig)

Dit moet worden: Do ..ZetBoldKorttekstVoorFrontbevestiging(DetailLijn)

Deze Quit-lijn mag weg hé. Deze method is een procedure, geen functie.

Hier mag een leeg lijntje tussen (tussen dimmekes en logica)

Naamgeving: Misschien beter ZetboldKaraktersInKorttekstVoorFrontbevestiging

Ik zou de variabelenaam nog aanpassen, van EindVanLijstBereikt naar VerdereIteratieIsOnnodig, want hij gaat eigenlijk nooit tot aan het einde van de lijst als het V1 en Frontbevestiging is, maar slechts tot waar het nodig is.

Leeg lijntje mag weg.

Zie comment in ProductDetailDataLijnFilter.

Zie comment in ProductDetailDataLijnFilter.

Waarom TECH.Mockable? Is niet nodig, omdat de objectjes die je in je testen gebruikt ook van dat type zullen zijn en de filter daarop dus ook perfect zal werken. Je hoeft hem dus niet uit te mocken (en dat doe je trouwens ook niet in je testen), dus er is geen nood om hem mockable te maken. Dus graag de super TECH.Mockable ervan tussenuit halen en de mock-klasse en de fake-klasse deleten.

Het werkt allemaal maar ik zou het inderdaad beter refactoren naar een betere if-else structuur. Bij ZetKorttekst moet er effectief een TagBasedTekst meegegeven worden en niet een string. Het is de ZetKorttekst van APPS.Halux.PPS.Document.ProductieDetailDocument.impl.common.ProductDetailDataLijn dat gebruikt zal worden en niet die van APPS.Halux.PPS.Document.ProductieDetailDocument.impl.common.Inhoud

De ProductDetailLijnen zijn niet van het type string maar APPS.Halux.PPS.Document.ProductieDetailDocument.common.Inhoud.ProductDetailLijn

Er staat V1 in de titel van het kaartje dus ik dacht dat het daarom enkel voor V1 was, niet?

Het codeblok hierboven moet je even refactoren, want zit qua logica niet goed. De hoogte in vet zetten doe je 2 keer als het ook expando-t is. Is dat nodig? Heb je dat uitgeprobeerd?
Bovendien (en veel belangrijker)... Je moet bovenstaande eens uitproberen in PuTTy, dan zul je zien dat je aan ZetKorttekst() geen string meegeeft, maar een object van het type APPS.common.Document.impl.TagBasedTekst. Ik denk dat je op dat object nog de method GeefTekst() moet aanroepen, dat je dan pas de string terugkrijgt met de tags erin.

Wat als er al eens iets anders in vet werd gezet? Ga je dan hoogte en expando-t niet meer in vet zetten?

Oppassen dat je de naam van je variabele beschrijvend houdt. Nu steek je de inhoud van de property Langtekst in de variabele LadeType. Ook al extract je op de volgende lijn het LadeType eruit, Langtekst <> LadeType. Beter zou zijn iets in deze zin:

#dim GezochtLadeType As %String = "LEGRABOX"
If ($Extract(..Langtekst,1,8) = GezochtLadeType) {

Maar je kunt er net zo goed 1 regel van maken:
If ($Extract(..Langtekst,1,8) = "LEGRABOX") {

En ook nog: Probeer er erg in te hebben om alles met een $ ervoor volledig uit te schrijven. Dus niet $E maar $Extract (ook verderop in dit codeblok). Het is niet fout, maar zorgt voor leesbaarheid, vooral voor mensen die nieuw zijn in de code.