Eind november 2018 hebben we bij het backend-overleg hoofdstuk 1 en 2 doorgenomen (link). Tijd om door te gaan met de volgende twee hoofdstukken van Uncle Bob.
Hoofdstuk 3.
Dit hoofdstuk gaat over functies.
Functies moeten klein zijn.
De eerste regel is dat een functie klein moet zijn. En als deze klein is, moet ie nog kleiner worden. Dit laat me even terug denken aan mijn eerste programmeeropdracht in Delphi bij de opleiding Hogere Informatica. In december begon in met de opleiding (ik kwam van de Accountancy-opleiding, mijn medestudenten hadden dus al 2.5 maand les gehad, ik begon die week) en mijn eerste opdracht was: maak een formulier, daar vul je de geboortedatum in en als je op de knop "geef weekdag" klikt, moet je zien op welke dag van de week dit valt. In die "onbutton_click" zat vervolgens mijn complete code om dit te berekenen. Toen had ik nog geen basiskennis van eigen functies maken en die aanroepen. Dat is inmiddels wel anders.
Blokken en inspringen.
In die korte functie mag je eigenlijk maar 1 of 2 keer inspringen. Zijn het er meer dan doe je waarschijnlijk teveel in die functie en wordt je code minder leesbaar.
Doe 1 ding!
De naam van de functie moet duidelijk beschrijven wat de functie doet. Een functie mag maar één ding doen. Doet je functie meer dingen, dan moet je dit refactoren naar een eigen functie. Zorg dat je functie één ding doet, dat de functie dit goed doet én dat deze functie de enige functie is die deze zaken doet. Als je ziet dat je in een functie zaken kunt onderverdelen naar secties, dan geeft dat aan dat die secties eigen functies kunnen zijn.
Houd het abstractielevel per functie op één.
Als je binnen je functies een object maakt, daar een bepaalde functie van aanroept en vervolgens achter het resultaat zelf nog met += "tekst..." zaken plakt, dan mix je teveel abstracties door elkaar heen.
Lees code van boven naar beneden.
Als je door de code heen leest zou eigenlijk de volgende actie die je ziet de actie moeten zijn die je al verwacht had.
Switch en if / then / else statements.
Deze statements zou je eigenlijk zo weinig mogelijk moeten gebruiken. Maar soms ontkom je er niet aan. Plaats die statements dan zo diep mogelijk in de code. Want als je bepaalde checks in de toplaag van je code hebt, dan ontstaat het risico dat die checks op meerdere plaatsen toegevoegd worden. En heb je het probleem dat als er een optie bij komt je dat op al die plekken door moet voeren. Voorbeeld? Ik heb dit zelf toegepast door in Razor-templates te controleren of iemand een bepaalde rol of hoger heeft, zodat bepaalde zaken zichtbaar/beschikbaar worden. Maar als er nu een extra rol bij komt, die heeft meer bevoegdheden en er zou op die rol gecontroleerd moeten worden, dan moet ik nu alle if-statements in de code gaan aanpassen. Terwijl iedereen een UserRole-object heeft en je daar die controle centraal had kunnen plaatsen... Ik zie een eigen stukje refactoring aankomen :)
Gebruik beschrijvende namen.
Een naam van een functie mag best lang zijn. Zorg dat de naam van je functie precies beschrijft wat de functie doet / verwacht. Zo heb ik dit bij mijn vorige blog proberen te doen met de extensions op de waarde die uit de app.config gelezen wordt: .ParseToCommandAndParameters(); Je verwacht dan dat het resultaat van deze functie je een Command en een Parameters-object terug geeft (en dat is ook zo).
Functie argumenten.
Je kunt argumenten meegeven aan functies. Volgens het boek is 1 parameter optimaal, 2 parameters kan nog, maar 3 of meer, dat zijn er eigenlijk teveel. Nu had ik bij mijn vorige blogpost de functie ProcessParameters waarin je alle lokale variabelen mee moet geven. Dat zouden er 100 kunnen zijn. Maar dat voldoet nog steeds aan de regel, want eigenlijk is dat 1 variabele: een lijst met waardes. Zorg wel dat duidelijk is waarom een bepaalde parameter in deze functie benoemd is. Een StringBuilder meegeven, waar is die dan voor? Hetzelfde geldt voor out-variabelen. Je verwacht inputs, maar soms heb je ook een output variabele. Volgens het boek zou dit eigenlijk niet meer nodig moeten zijn. Omdat je in het object waar je zit zelf de acties op uit kunt voeren. Ik heb dit wel eens gebruikt (boolean resultaat of iets lukt en daarna als de waarde true was de tekst in een output variabele gebruikt), maar het is de moeite waard om bij toekomstige projecten te kijken of het inderdaad anders kan.
Dan krijgen we de "flag" argumenten. Je roept een functie aan met "true" of "false". Wat is het verschil? Volgens het boek had je hier gewoon 2 functies voor moeten maken, zodat duidelijk wordt dat de functie wrapProduct(boolean check) eigenlijk bedoeld is als wrapProductTransparantPaper() / wrapProductNormalPaper().
Voorkom bij-effecten.
Dit komt weer terug op het punt dat een functie maar 1 ding moet doen. In het boek heb je de functie checkPassword die een true of false teruggeeft, maar als de waarde true is, ook nog eens een sessie initialiseert. Waarmee je huidige sessie (met niet opgeslagen waardes) weg is. Oeps!
Command - Query scheiding.
In het voorbeeld van een boek krijgen we een "set" functie die iets doet en een boolean teruggeeft of het wel of niet gelukt is. Omdat je in de aanroepende code een if .. gebruikt is in die code niet altijd duidelijk dat de functie een controle doet (query) of dat deze een daadwerkelijke actie (command) uitvoert.
Gebruik excepties en geen error-codes.
Als je een error-code teruggeeft moet je meteen de waarde afvangen en controles uitvoeren. Waarbij ook nog eens, als er een error-code toegevoegd wordt je dit op tig plaatsen aan moet passen. Throw een exceptie en specificeer zelf die exceptie. Gaat er in je code wat fout met een HTTP-call, throw dan die System.WebException en vang dit later in je code op.
Zet je try / catch op de juiste plaats.
In het boek wordt benoemd dat try / catch je code minder leesbaar maakt en je deze dan ook op de juiste plaats moet zetten. Het voorbeeld is een delete-functie, deze bevat de try-catch en roept binnen de try de andere functie aan die alle acties uitvoert. Gaat het fout, dan spring je terug in de aanroepende try/catch en heb je niet allemaal try/catches binnen alle aangeroepen functies. Hoewel ik zelf iemand ben die dit juist wel doet, is het wel waar dat nu wel een heel groot deel van de code try-catch is. In het vervolg dus maar eens goed kijken of die try-catch wel nodig is, of dat het in het aanroepende deel al goed opgevangen wordt. Als er ergens bij een import een fout optreedt in een bepaalde regel van een importbestand, wil ik die fout op dat punt afvangen, zodat ik bij het "throwen" van de fout het regelnummer mee kan geven. Dat is een legitieme reden om wel dieper in je code een try/catch toe te voegen.
Dupliceer jezelf niet.
Kom je dezelfde controle op meerdere plaatsen van je code tegen? Dan kun je het vast verplaatsen naar 1 functie die dit voor je doet.
Structured Programming.
De waarden van Edsger Dijkstra komen voorbij. Die luiden dat elke functie en elk blok binnen die functie 1 ingangspunt en 1 uitgangspunt zouden moeten hebben. Één return, geen break of continue in een loop en nooit een go-statement. In oude code met hele lange functies is dat geldig, maar in het huidige gebruik is het afwijken van die regels geen probleem. Maar die goto gebruiken we (als het goed is) sowieso al niet meer.
Hoe schrijf je deze functies?
En nog even het voorbeeld van de auteur, dat je niet zomaar al die losse functies schrijft. Je begint met één grote hoop aan code en door te optimaliseren / refactoren zou je uiteindelijk op een nette structuur uit moeten komen.
Hoofdstuk 4.
Dit hoofdstuk gaat over commentaar / comments.
Commentaar zou niet nodig hoeven zijn. Als er commentaar in de broncode staat, dan betekent dit dat de functie(naam) niet duidelijk is, dat er slechte code gemaakt is of dat er iets anders niet goed is.
Goed commentaar.
Je zou het niet verwachten, maar dit is er toch nog.
- De legal comments (copyright, public license).
- Informatief commentaar (bij een regex welke input matched).
- Explanation of intent (uitleg van de bedoeling, deze delay zit er in om time-outs te testen).
- Clarification (verhelderen van bepaaalde waardes, als je zaken uit externe libraries moet halen, je daar niets aan kunt passen en dan tig verschillende checks hebt, dan is een stukje commentaar erbij verhelderend).
- Warning of consequences (waarschuwen dat je van zaken af moet blijven, bijvoorbeeld dat functie niet thread safe is en door een mogelijk "slimme optimalisatie" juist foutieve resultaten terug gaat geven)
- TODO commentaar voor code die je later nog moet maken.
- Amplification (benadrukken waarom je iets doet, bijvoorbeeld dat als een waarde niet getrimd wordt deze fouten naar externe API's gaat geven).
- Javadocs (documentatie van een API wordt automatisch gemaakt op basis van commentaar in de code)
Code wordt vaak aangepast, maar commentaar blijft vaak staan, wat weer kan zorgen voor verouderde of zelfs foutieve informatie. Dit geldt natuurlijk ook voor "goed commentaar".
Slecht commentaar.
- Mumbling (mompelen, er staat een stuk commentaar in de code en het enige wat je denkt is "waar gaat dit over????")
- Redundant comments (overbodig commentaar. een uitgebreide beschrijving van een functie waarbij het lezen van het commentaar langer duurt dan het interpreteren van de code.)
- Misleading comments (misleidend commentaar. Zegt dat als je de waarde 1 in de functie stopt er altijd een 2 uit komt. Bekijk je de code, dan zie je dat in sommige gevallen een 3 teruggegeven wordt. Oeps.)
- Mandated comments (al die javadoc commentaar, waarbij je elke functie, elke parameter gaat beschrijven en het gevaar van foutieve informatie op de loer ligt.)
- Journal comments (volledig logboek met wijzigingen. Hier hebben we versiebeheer voor, niet doen dus.)
- Noise comments (totaal overbodig commentaar. Als de variabele al dayOfWeek heet, waarom moet in commentaar erbij dat dit "de dag van de week is"?)
- Position markers (in je code // ACTIONS ////////. Je kunt in Visual Studio met regions werken om zaken bij elkaar te houden. Maar dat gebruik ik bijna nooit. En wat is de toevoeging / informatieve waarde dat alle acties in dit deel van de code staan?)
- Closing Brace Comments (afsluiten if / case en daar //end while e.d. plaatsen. Als je dat nodig hebt, geeft het aan dat je code te lang is, refactoren dus.)
- Attributions and bylines (/* fixed door DH2 */. Niet nodig om te doen, hier heb je versiebeheer voor.)
- Uitgecommentarieerde code (waarom staat het in commentaar? Is het ooit nog nodig? Heeft iemand dit per ongeluk gedaan? Waarschijnlijk blijft het tot het eind der tijden hier staan. Ook hier weer, gebruik er versiecontrole voor.)
- Html-comments (betrekking op Javadoc, als je HTML karakters in je commentaar wilt gebruiken moet je zelf de < e.d. in het commentaar plaatsen. Dat zou die tool moeten doen!)
- Non-local information (je zet wat in commentaar wat niet op de functie slaat waar het bij staat, maar ergens op een waarde ergens verderop in de code.)
- Teveel informatie (voorbeeld van het boek, BASE64 wordt gebruikt. Niet alleen verwijziging naar de RFC, maar volledige uitleg van het principe.)
- Onduidelijke connectie (er staat een stukje commentaar bij een functie, maar in dat commentaar worden zaken benoemd die je niet uit deze code haalt en waarbij je verder de code moet doorspitten om te snappen wat er bedoeld wordt.)
- Function headers (grote omschrijving van wat de functie doet. Niet nodig, geef de functie een goede naam.)
- Javadocs in niet publieke code (javadoc is bedoeld voor API's e.d., dus in je systeemklassen e.d. heb je dit helemaal niet nodig.)
Gebruik een variabele of functie als je daarmee commentaar kunt voorkomen.