Clean Code: practise what you preach!

Ingediend door Dirk Hornstra op 25-sep-2023 21:50

In de afgelopen jaren heb ik hier een aantal samenvattingen gedeeld van "clean code" van "Uncle Bob" en andere gerelateerde zaken. Als je dan zelf "even" met een projectje start, dan denk je daar waarschijnlijk niet aan en ga je voor de snelle oplossing. Maar als je dan tegen wat probleempjes aan loopt, door je eigen code moet bladeren en denkt: dit moeten we refactoren, dan komt de vraag: ga je dat doen, of begin je "gewoon opnieuw"? In dit geval was de code niet heel uitgebreid, had ik wel een goed idee wat de code moest doen, dus ben opnieuw begonnen. Laat ik hier even mijn fouten delen, zodat je ervan kunt leren en die fouten straks niet maakt ;)

Waar was ik mee bezig?

Oh ja, even wat context. De afgelopen 2 weken heb ik geen artikel op mijn Techblog geplaatst (en via LinkedIn gedeeld), omdat ik bezig was met een Sudoku-uitleg programma. Als je een programma wilt wat een Sudoku kan oplossen, waarschijnlijk kan ChatGPT je de kant-en-klare code aanleveren. Ik wil echter een programma/app maken waarbij je de Sudoku in kunt voeren en waarbij je dan stap-voor-stap te zien krijgt welke waarde gevuld wordt en waarom. De snelste methode is niet altijd de methode hoe een "mens" een Sudoku oplost. Daar kijk je namelijk naar de ingevulde getallen en kijk je of je via de rij of de kolom tot een oplossing komt. En welke mogelijke waardes je kunt invullen. De uitleg die je bij een Sudoku krijgt is beknopt: "Alle cijfers van 1 t/m 9 moeten een keer voorkomen in elke kolom, elke rij en in elk vierkant van 3 bij 3 vakjes". Ook na te lezen via Denksport. Maar dan nog, waarschijnlijk begin je en kom je er wel uit, maar mocht dat niet zo zijn, dan wil ik je met mijn programma/app op gang helpen!

Goede naamgeving: noem niet al je private variabelen _items.

Ik ben begonnen met een blokje, de Cell. Vervolgens met een CellGroup. Die heeft een lijst met 9 Cells en die had ik "_items" genoemd. De Matrix (het complete veld) bevat 9 cell-groepen. Die lijst had ik dus ook "_items" genoemd. Omdat zaken nog wel eens door elkaar lopen en je dus de ene _items hebt en de andere _items, heb ik die zaken hernoemd naar _cells en _groups, zodat duidelijk is waar je over praat.

Gebruik KeyValuePairs alleen voor value-type variabelen, niet voor classes.

Een cel heeft een positie in een groep en een groep heeft een positie in een matrix. Dus het is heel makkelijk om van die items een lijst van KeyValuePairs te maken, waarbij de Key een Point is (x- en y-coördinaat) en de Value de cell (of een groep). Alleen, als je in je code met deze objecten werkt, dan loop je tegen het volgende aan. Een Cell heeft namelijk een Value-property. En die kan NULL zijn als je nog niet weet welke waarde deze moet hebben. En als dan ergens in de code je daar gebruik van maakt, zeker weet dat die eigenschap een waarde heeft, dan krijg je iets als lijstobject.Item.Value.Value!.Value. Kijk, als je zoiets ziet, daar word je niet blij van! Daarom heb ik een struct gemaakt met een Point en een Cell property. Daarmee krijg je een lijstobject.Item.Cell.Value!.Value en daardoor zie je dat het over de cell gaat en niet over een value van een value van een value :D Een KeyValuePair zou je eigenlijk alleen voor value-types moeten gebruiken en ja, een struct is ook een value-type!

Hou je code "dom".

In mijn eerste versie had ik bij het Cell-object ook de referentie toegevoegd in welke groep deze zat. En zo kon deze ook bij de buren ("neighbours") komen, om daar de waarde in te vullen. Maar is dat de taak van een cell? Nee dus! Dat is wat mij betreft de taak van de Matrix, die kan door de groepen heen gaan en bepalen als daar een ingevulde waarde is, dat je ook de naast liggende groepen gaat controleren of de waarde daar in te vullen is.Een cell heeft daarom eigenlijk alleen maar zijn waarde en opties waaruit gekozen kan worden. Een cellgroup is zelf nog beknopter qua code, deze heeft alleen maar de cellen een een "Calculate" functie die van alle cellen de "Calculate" functie aanroept. De Matrix, het alles-omvattende object, die bevat dus de echte functies om te kijken welke waardes nu waar ingevuld kunnen worden.

Maak gebruik van LINQ-methodes.

Het credo is om je functies klein te houden. En met LINQ kun je heel veel dingen doen. Waar je anders een foreach(var i in items) zou doen, kun je ook items.ForEach(rec => { ... } ); uitvoeren. Ik heb mijn code daardoor behoorlijk beknopt kunnen houden.

Gebruik de Yield-methode!

Ik denk dat het ruim 10 jaar geleden is dat mijn collega Peter Kiers liet zien dat hij het "yield" keyword gebruikte. Ik vond dat cool om te zien, alleen heb ik in de afgelopen jaren dat keyword niet gebruikt. Tot nu, want in deze code heb ik het wel nodig. Eindelijk! Wat doet het eigenlijk? Nou, je bent bezig met een lijst, in dit geval de 9 cellen binnen een groep/blok. Je bent zover dat één van die cellen waarvan de waarde onbekend was nu gevuld kan worden: er is nog maar 1 waarde over. Dat ga je terugkoppelen naar je aanroepende code. Je wilt "nu" dus uit deze code springen en dan door met de volgende cellen die misschien ook gevuld kunnen worden. In de aanroepende code waar je terugkeert kun je dan wat acties doen. Zo zorgt ik daar dat alleen de eerste teruggekoppelde waarde verwerkt wordt, alle andere waardes die terug komen als "dit kunnen we ook vullen!" worden genegeerd, we willen de gebruiker namelijk "stap-voor-stap" laten zien welke waarde ingevuld moet worden en daarom moet er maar 1 waarde aangepast worden. Met yield kun je dat dus redelijk simpel doen. Want in het stuk code waarin ik zeg "los het maar op" verwerk ik gewoon alle items die in de lijst terug gegeven worden. Ik hoef dus verder niets in de functie aan te passen die controleert welke waarde ingevuld kan worden.

Geef je functies dezelfde naam!

Ik had de class voor de Matrix een IsSolved-property gegeven. Die controleert of alle IsSolved-properties van de groepen een true terug geven. Die IsSolved-property van een groep keek of alle cellen een waarde hadden dus: Cells.All(rec => rec.Value.HasValue). Maar toen dacht ik: nu weet "de groep" hoe van een cell bepaald wordt of deze een waarde heeft of niet. En dat is niet goed! Daarom heb ik de class Cell een IsSolved property gegeven. En hoewel deze natuurlijk hetzelfde doet (geeft terug of Value.HasValue), mocht ik besluiten dat ik dit anders aan ga pakken, dan heeft dat alleen impact op de Cell en niet op de CellGroup-class, die roept "gewoon" IsSolved aan. Hierdoor hebben we nu Matrix.IsSolved = Matrix.AllGroups.IsSolved = Allgroups.AllCells.IsSolved.

Vervolg

Ik had een tijdje geleden al een soort "playground" aangemaakt. Om mijn code te testen heb ik een codebibliotheek aangemaakt met de cel, groep en matrix en daarbij ook een Console-project. Zodat ik even 3 voorbeelden kon testen en de output naar de console kon wegschrijven. Ik ga kijken of ik dit in een formulier op een webpagina kan krijgen. In een ver verleden heb ik al eens zoiets gemaakt en het leuke is, ik heb toen nog eens een reactie van iemand ontvangen die mijn oplossing gebruikt heeft en zo haar probleem op kon lossen! Nog even opgezocht, ik zie dat dit in 2010 was: link. Het iframe in die pagina werkt niet meer, want dat domein draait inmiddels op PHP.

De code ga ik in mijn Github-repo zetten en via de playground-omgeving krijg je de mogelijkheid om je eigen Sudoku te testen! Ik hoop dat ik in mijn volgende blogpost de URL kan delen.

Dit wordt trouwens een kleine serie, dit was de (simpele) Sudoku (je hebt ook varianten waarbij de diagonalen ook maar 1x de waarde 1 t/m 9 mogen bevatten, dat ondersteunt mijn versie niet), ik wil ook kijken of ik een zelfde soort "oplosser" kan maken voor Tectonic en van "Boompje-Tentje"!