Eind november 2018 hebben we bij het backend-overleg hoofdstuk 1 en 2 doorgenomen (link), in januari 2019 hoofdstuk 3 en 4 (link), in maart 2019 hoofdstuk 5 en 6 (link), in april hoofdstuk 7 en 8 (link), in mei hoofdstuk 9 en 10 (link), in juni hoofdstuk 11 en 12 (link), in augustus hoofdstuk 13 en 14 (link) en in september hoofdstuk 15 en 16 (link).
We nemen meestal 2 hoofdstukken, maar we zijn er nu bijna klaar mee, alleen hoofdstuk 17 staat er nog in (en nog wat bijlages, maar die mag je zelf doorlezen).
Hoofdstuk 17.
We hebben in dit boek geprobeerd de vinger te leggen op de oorzaken van foute code en hoe we dit door refactoren goed kunnen krijgen. In dit hoofdstuk gaan we een hele lijst van zaken langs die, als het onderdeel van jouw of andermans code uitmaakt, kan zorgen voor fouten, foutieve informatie, niet afvangen van "boundary cases" en veel meer.
Comments / commentaar
Niet ter zake doende informatie
Wat kan jou het schelen dat Pietje Puk op 1 april een bepaalde versie verhoogd heeft? Zulk soort informatie hoort in het source-control programma thuis wat je gebruikt en niet in de code.
Verouderd commentaar
Als het oud, irrelevant of foutief commentaar betreft moet je het bijwerken of weghalen.
Overbodig commentaar
Als het commentaar niets toevoegt, het omschrijft wat je al in één keer ziet als je de beknopte code doorkijkt, haal het dan weg.
Karig geschreven commentaar
20 regels commentaar voor een functie, als je iemand het commentaar laat lezen en het resultaat is dat degene zegt "huh, wat doet dit?", dan valt het onder dit blokje. Houd je commentaar beknopt en duidelijk.
Code uitgezet in commentaar
Een hele functie of class is in commentaar gezet. Waarom? Moet het weer aangezet worden? Iedereen is bang om zijn handen te branden en zo "rot" deze code langzaam maar zeker. Haal het weg, je source-control programma heeft de code nog beschikbaar mocht het ooit weer gereanimeerd moeten worden.
Environment / omgeving
De build heeft meer dan één stap nodig
Je wilt je programma builden. Dan blijkt dat je eerst nog een ander project moet uitchecken, dat moet builden en je dan pas dit project kan builden. Nope, zo moet het niet.
Tests hebben meer dan één stap nodig
Er moet één knop zijn waarmee je al je testen kunt uitvoeren. Lukt dat niet, dan is het risico groot dat testen helemaal niet meer uitgevoerd worden.
Functions / functies
Teveel argumenten/parameters
Geen is goed, 1, 2 of 3 parameters, dat kan. Heb je er meer nodig, ga dan kijken naar alternatieven (zie function arguments in hoofdstuk 3).
Output argumenten
Je verwacht dat je een resultaat van de functie terugkrijgt (het functieresultaat). Dat je een parameter meegeeft, en dat die bij terugkomst mogelijk aangepast is, dat zijn eigenlijk neveneffecten die je niet wilt hebben (zie output arguments in hoofdstuk 3).
Flag argumenten
Als er een boolean meegegeven wordt aan een functie schreeuwt dat eigenlijk dat je functie meer dan één ding doet.
Dat wil je niet, want dat is strijdig met het " Single Responsibility Principle ".
Dode functies
Een functie die nooit aangeroepen wordt. Verwijder 'm. Hij neemt alleen maar ruimte in de sourcecode in en maakt je class minder leesbaar.
Algemeen
Meerdere talen in één bestand
Iets wat niet altijd te voorkomen is, maar het liefst wel voorkomen moet worden. Zie onze classic .ASP bestanden, een mix van VB (ASP) en HTML.
Verwacht gedrag is niet geïmplementeerd
Stel dat je een functie addDays maakt, met een parameter int, waarbij je bij een datum dagen kunt optellen. Je verwacht dan dat, door een negatieve waarde te gebruiken, je ook "terug" kunt met de dagen. Als dat echter niet wordt geïmplementeerd in de functie zorg je dat iets wat verwacht wordt niet uitgevoerd wordt. Daardoor raakt een gebruiker van je code het vertrouwen kwijt en gaat expliciet alle code doorlopen om te controleren of die functie wel doet wat hij/zij verwacht.
Onjuiste gedrag bij de grensgevallen
Vertrouw niet op je intuïtie. Zoek elk grensgeval op en schrijf er een test voor. Je kunt wel verwachten dat er nooit door 0 gedeeld gaat worden, maar als het wel gebeurt, gaat je code dan kapot of vang je het af?
Het uitschakelen van beveiligingen
Je code wil niet compilen. Door een bepaalde "pragma warning disable" in je code op te nemen wordt de beveiliging omzeild en heb je werkende code. Maar kun je hierdoor niet onverklaarbare fouten krijgen waardoor je later eindeloos moet debuggen?
Kopieën
20 keer hetzelfde if-statement of blokje code wat steeds terugkomt. Don't do it, ruim het op!
Code op de verkeerde abstractie-laag
We zien een stukje voorbeeldcode, een stack-Interface. Met een percentFull-property. Maar sommige stacks zouden in theorie oneindig kunnen zijn. Deze property hoort dus niet hier, maar bij de overgeërfde klasse die een limiet heeft.
Base-class afhankelijk van de afgeleide classes
Er kunnen uitzonderingen zijn (bepaalde sterk gekoppelde classes die altijd aanwezig zijn en waarbij in de base-class een beslissing uitgevoerd moet worden), maar in het algemeen geldt: dit mag niet voorkomen!
Teveel informatie
Een interface met 100 functies, een base-class met 50 variabelen. Probeer alles zo beknopt mogelijk te houden, scherm je functies en variabelen af.
Dode code
We hadden al dode functies, dit gaat over bijvoorbeeld een if-statement wat nooit uitgevoerd gaat worden en waarbinnen dus "dode code" zit. Haal het weg, ruim het op.
Verticale scheiding van code
Zet variabelen en functies in de buurt van waar ze gebruikt worden. Maak geen algemene variabele "int nrOfDays" in je class aan, als je die alleen maar binnen één functie gaat gebruiken.
Inconsistentie
Zorg dat je functies en variabelen duidelijke en vergelijkbare namen hebben. Dus als je een addPerson en editPerson hebt, maak dan ook een deletePerson aan en niet een functie personRemove.
Rommel
Blanco constructors, gedeclareerde variabelen die nooit gebruikt worden. Weg ermee.
Kunstmatige koppeling
Als je bepaalde globale variabelen hebt, declareer die ook globaal en niet ergens in een bepaalde class. Want dan moet ook de rest van je code op de hoogte zijn van die class.
Afgunst van andere classes
Class A gebruikt class B. Class B heeft een bepaalde functie die een object van class C teruggeeft. Die class C heeft ook weer bepaalde functies. Nu maakt class A een instantie aan van class B en roept daarop de functie aan waarbij object C teruggegeven wordt en roept daar meteen een functie van class C op aan. Dus iets wat eigenlijk alleen van class B heeft (die heeft kennis van class C en mag/kan daar zaken op uitvoeren), wordt nu door class A gedaan. Wijzigt de implementatie van die functie in class C, dan weten we dat we class B ook moeten gaan aanpassen. Maar waarschijnlijk zal class A hier vergeten worden (of we zien het niet), waardoor deze met foutieve data gaat werken. Nooit doen dus!
Selectie argumenten
Komt bij mij over alsof dit hetzelfde is als flag argumenten, de boolean waardoor in je functie verschillende acties uitgevoerd worden.
Verborgen bedoelingen
Je kunt code soms heel beknopt schrijven. Maar als je het dan later terug ziet en je ziet allemaal vaste getallen en if-statements, dan vraag je je af, wat doet dit nu? Maak je code dus gerust wat uitgebreider, extra lokale variabelen die je met een duidelijke naam gebruikt, zodat je ook na 12 maanden nog snapt wat er gedaan wordt.
Misplaatste verantwoordelijkheid
Je class heeft een print-functie van het urenoverzicht. In die functie worden ook de uren berekend. Dat verwacht je niet als de functie printHours() heet, dan zou het eerder printAndCalculateHours() o.i.d. moeten zijn.
Ongepaste "static"
Math.max(a,b,) is een prima functie om static te zijn, het heeft 2 variabelen als input en op basis daarvan wordt het resultaat gegeven. Maar over het algemeen is het aan te raden om een "instance" van een class te maken en niet een static versie.
Gebruik beschrijvende variabelen
var a=1; var b=... Geen idee wat deze variabelen betekenen, geef ze een duidelijke naam.
Functie-namen moeten zeggen wat ze doen
Voorbeeld van date.add(5). Zijn het dagen, uren, maanden, jaren? Geef je functies dus duidelijke namen.
Snap het algoritme
Je bouwt een functie met allemaal if-else statements en uiteindelijk krijg je jouw gewenste resultaat, de testen werken ook. Maar zorg wel dat je snapt wat het doet. Want als je dat niet doet en je loopt tegen een situatie aan dat er een grensgeval wordt geraakt en het resultaat onjuist is, dan is het nog steeds trial-and-error om het werkend te krijgen.
Maak logische afhankelijkheden fysiek
Voorbeeld in het boek is dat de class HourlyReporter een print-functie heeft. Deze heeft een controle op de constante PAGE_SIZE. Die constante waarde komt uit een andere class, die moet namelijk bepalen hoeveel regels er op een pagina kunnen komen. Dus er moet een functie komen in die class die aangeroepen wordt in HourlyReporter.
Geef de voorkeur aan polymorfie in plaats van if-else en switch-statements toe te voegen
Met interfaces en classes kun je zorgen dat een object zijn eigen functie uitvoert in plaats van dat je 20 if-statements moet bouwen in één functie om zo de juiste functie uit te voeren.
Volg de standaard-afspraken
Hiervoor gebruiken we CodeMaid, vaste opmaak en indeling van je code. Geen gedoe.
Vervang magische nummers door constanten met een duidelijke naam
Maakt niet alleen je code beter leesbaar, maar als een waarde aangepast moet worden, hoef je alleen de definitie van de constante aan te passen.
Wees exact
Als je één waarde wilt hebben, gebruik dan in je Linq-query .Single() en niet .First(), omdat .Single() een exceptie geeft als er meerdere resultaten zijn. Als je verwacht dat een functie een NULL-waarde terug kan geven, zorg dan dat je hierop controles toevoegt. Wees niet lui of makkelijk, maar lever goed werk af!
Structuur boven conventie
Uhm, wat wordt hier nu exact bij gezegd? Dat het mooi is dat je in het switch statement alle enums een duidelijke naam geeft is mooi, maar bijvoorbeeld een base class dwingt af dat een concrete class de functies moet implementeren.
Voeg condities samen
Als je een if-statement hebt met daarin 5 condities die waar moeten zijn, dan kun je daar beter een losse functie van maken die een duidelijke naam heeft wat een "true"-uitkomst van deze condities eigenlijk betekent.
Voorkom negatieve voorwaarden
Een if (!(isComplete()) is niet echt goed leesbaar, voorkeur gaat dan naar if (isNotComplete()).
Functies zouden één ding moeten doen
Hebben we het al meerdere keren over gehad.
Verborgen tijdelijke koppelingen
Koppelingen zijn vaak nodig, maar je moet ze niet verbergen. Voorbeeld in de code zijn 3 functies die na elkaar aangeroepen worden. Wat je niet ziet, is dat deze volgorde aangehouden moet worden, je mag ze niet omwisselen! Door de ene functie een resultaat op te laten leveren en dat als parameter aan de volgende functie mee te geven is wel meteen de afhankelijkheid duidelijk.
Wees niet arbitrair (dus doe dingen niet willekeurig, maar juist voorspelbaar)
Voorbeeld in het boek is dat er een class binnen een andere class geplaatst is. Reden daarvan is onduidelijk en lijkt ook helemaal niet nodig te zijn. Voorkom zulke situaties!
Voeg grensgevallen samen in een variabele
We zien een stukje code waarbij op 2 plaatsen een variabele -1 in staat. Zo'n -1, +1 op meerdere plaatsen, dat maakt de boel minder leesbaar en foutgevoeliger. Maak een lokale variabele aan met bijvoorbeeld de naam int maxAmount = variabele -1 en gebruik die maxAmount in de rest van je functie. It makes your life en die van degene die bugs moeten opsporen een stuk makkelijker!
Functies zouden slechts één level van abstractie mogen hebben
In het stukje voorbeeldcode zien we hoe een HR html-tag wordt opgebouwd en hoe daar een eigenschap "size" wel of niet aan wordt toegevoegd. Het opzetten van de HR zou deze functie moeten doen, het toevoegen van andere attributen moet niet binnen deze functie, maar door een eigen functie uitgevoerd worden.
Houdt configurabele data op een hoog niveau
Als je een web-applicatie bouwt en die laat je op een bepaalde poort draaien, dan stel je dat in bij de code om de applicatie te starten. Dat ga je niet ergens diep verborgen in een eigen class definiëren, omdat het overal beschikbaar moet zijn.
Voorkom transitieve navigatie
Dit komt volgens mij redelijk overeen met de afgunst van andere klassen, zorg dat je class alleen acties uitvoert op zichzelf of de classes waar het mee samenwerkt.
Java
Voorkom een lange lijst met imports door de wildcard
import System.Text, Import System.Text.RegularExpressions, vervang het door 1 regel import System.Text.*
Ga geen constanten overerven
We zien een voorbeeld van een class B die erft van een andere class A. In class B worden bepaalde constanten gebruikt. Die zien we niet in class A. Maar na verder zoeken zien we dat class A een implementatie van interface X is waar ie constanten gedeclareerd worden. Dat is "een stap te ver". Doe in class B een import van deze interface zodat duidelijk is waar de waardes vandaan komen.
Namen
Kies beschrijvende namen
Al veel vaker benoemt, geef je variabelen en functies een heldere naam.
Kies namen op het juiste niveau van abstractie
Voorbeeld van een interface voor Modems. Zit een functie getConnectedPhoneNumber() in. Maar na invoering van kabelmodems zijn er geen telefoonnummers meer en is bijvoorbeeld het MAC-adres de bepaling van of we het juiste modem hebben. Hernoem de functie naar "getConnectedLocator()".
Gebruik standaard termen waar mogelijk
Als je het Decorator-pattern doorvoert en hier classes voor aanmaakt, noem deze dan ook 'classNaam'Decorator om duidelijk te maken wat hier de bedoeling van is.
Kies namen die niet dubbelzinnig zijn
Een algemene naam als "deleteItem" kan veel doen (zet een vinkje op deleted, verwijder een record uit de database) en dan blijkt dat het "alleen" een bestand van schijf verwijdert, noem het dan "deleteFileFromDisk()".
Gebruik lange namen voor grote codeblokken
Als je ergens een kort for-loopje hebt, kun je prima een int a gebruiken in plaatst van er een int counterForPages van te maken. Een variabele door de hele functie gebruikt wordt moet je juist wel zo'n lange (duidelijke) naam geven.
Voorkom encoding
In de naam van je variabele het type meegeven (f_ voor float, n_ voor numeriek). Niet doen, nergens voor nodig.
Namen moeten neven-effecten omschrijven
Als je een functie getSessionVariable(naam) hebt, dan verwacht je dat deze functie alleen kijkt of de waarde in de sessie geplaatst is. Als er geen sessie is en deze functie maakt een sessie aan, dan is dat een neven-effect wat ook in de naam van de functie verwerkt moet worden!
Testen
Onvoldoende testen
Testen zouden alles moeten testen wat potentieel kapot kan gaan. Testen zijn onvoldoende als er voorwaarden zijn die niet getest worden of als er berekeningen zijn die niet gevalideerd zijn.
Gebruik een dekkings-tool
Zo'n tool laat zien welk deel van je code nog niet getest wordt.
Sla testen die onbeduidend lijken niet over
Zulke testen zijn meestal snel en makkelijk te maken en leveren meer op dan ze kosten.
Een genegeerde test is een vraag over dubbelzinnigheid
Testen die je negeert zijn testen waarbij de specificaties nog niet toereikend zijn en je niet weet wat de verwachte output is.
Test grensgevallen
We checken altijd de normale waardes en wat we verwachten, maar meestal testen we zelf niet wat er gebeurt met waardes in de grensgevallen (happy-flow).
Test uitputtend "bijna bugs"
Als je een bug tegen komt, dan zijn er meestal meer. Test daarom uitgebreid de functie met je bug om ook de andere op te sporen.
Patronen die betrekking hebben op het falen worden zichtbaar
Als je een grote testset hebt zal eerder een patroon bij fouten (ik zie dat bij alle input met meer dan 5 getallen het fout gaat) zichtbaar worden.
Het dekkingspercentage van je testen kan patronen tonen
Als je ziet welke code wel en welke niet wordt uitgevoerd bij falende testen zul je eerder een patroon/oorzaak vinden.
Testen moeten snel zijn
Als testen niet snel zijn worden ze niet gedaan.