Refactor je code: start je functie met return ...., start met het resultaat

Ingediend door Dirk Hornstra op 22-jan-2024 21:44

In de loop der jaren heb ik een web-applicatie ontwikkeld die gebruikt wordt voor het inzichtelijk maken welke server, sites en bindings in ons beheer zijn. We koppelen dit aan externe systemen zoals Zabbix en Simplicate. Er draaien een flink aantal taken, onder andere het automatisch bijwerken van de SSL-certificaten van onze sites. Dat gaat goed, maar het systeem is in de loop van de jaren steeds meer uitgebreid. Zo heb ik in het begin het verwerken van taken uitgewerkt om een losse taak uit te voeren. Daarna kwam er een taak die uit verschillende sub-taken bestond. En inmiddels ben ik bij het punt gekomen dat een bepaalde sub-taak dynamisch een aantal subtaken toevoegt als deze uitgevoerd wordt.

In het begin heb ik bepaalde beslissingen genomen om de boel werkend te maken waar ik later spijt van gekregen heb. Want in de loop van de tijd hebben we met onze back-end developers "clean code" en "security by design" behandeld en daar komt ook een stukje "domain driven design" in voor. En dat raadt aan om met "business objecten" te werken. Want nu maakte ik soms in bepaalde Razor-template gebruik van de ViewBag, kreeg ik geen "object" binnen, maar bijvoorbeeld een record uit de database waar dan acties op uitgevoerd werden.

Is dat "een vieze oplossing"? Jazeker... als je terug kijkt denk je: hoe kon ik dat doen? Op dat moment was ik nog een beetje "groen" (nee, niet Team Groen, gewoon een "groentje") die zaken werkend kreeg. En nu denkt van: dat moet anders. Naast het feit dat je de code meer structuur wilt geven, ga ik kijken of dit naar .NET core overgezet kan worden (het is nu een Framework project) en volgens mij heb ik de "ViewBag" en "ViewData" of hoe het ook weet daar niet meer beschikbaar. En dat is ook goed dat het er niet meer is.

Ik heb al zalen opgeschoond, alleen liep ik nu tegen het punt aan dat het verwerken van taken eigenlijk anders moet. Want het was zo dat als een subtaak mislukte, automatisch de hoofdtaak ook op "mislukt" ingesteld werd. Maar omdat er nu een soort "ongedaan maken actie" uitgevoerd moet worden, is dat iets wat ik in de hoofdtaak geplaatst heb. Dit moet dan dus eigenlijk ook nog een keer uitgevoerd worden. Het probleem is dat sommige taken binnen de web-applicatie uitgevoerd kunnen worden, maar ook sommige taken op de webserver zelf uitgevoerd moet worden. En dan loop je aan tegen de plaats waar controles en acties plaats vinden: die heb ik in de API geplaatst.

En dat is dus niet de juiste plaats. Want zou ik iets met taken gaan doen op een andere plek dan de API, dan moeten ook daar die controles geïmplementeerd worden.

Tijd dus om alle taken in de API direct met een return ... statement te laten werken.

Waarom zou je dat willen doen? Nou, Sander Hoogendoorn kwam bij Techorama 2022 met dit punt, om je functies zo kort en "clean" mogelijk te houden start je met het return-statement. Dat is een hele andere insteek dan de meeste programmeurs (waaronder ik zelf) een functie starten. Want meestal bouw je wat objecten op, roep je wat functies aan, voer je daar filtering op uit en uiteindelijk geef je het resultaat terug. We "werken naar het resultaat". En daarmee ben je 60 regels verder. En inderdaad, zie je niet echt wat er nu onder de motorkap gebeurt.

Zo start je vaak met een "using" van de databaseconnectie (via het Entity Framework) om verbinding te maken met je database. Want je wilt je connectie "zo kort mogelijk open houden" en dat die zo snel mogelijk weer opgeruimd wordt. Met "using" zorg je dat, ook als er wat fout gaat, je databaseconnectie netjes afgesloten wordt. Want als zaken te lang (en te veel) opengezet worden, dan stijgt het risico op deadlock situaties: je wilt een actie op de database doen, maar alle connecties zijn in gebruik en na een bepaalde time-out wordt gezegd: ik kill jouw functie-aanroep, probeer het later nog maar eens.

De oplossing hiervoor is dat je in de class een eigen databaseconnectie opzet. Jouw class is een sub-class van IDisposable, en je implementeert de Dispose()-functie om je databaseconnectie weer op te ruimen. De "ceremonie" om via using een eigen database-connectie op te starten is dan in jouw functie niet meer nodig, dat handel je af in de constructor van de class.

Als je bang bent dat "je databaseconnectie dan te lang open blijft staan", dat is een ander punt. Je moet namelijk zorgen dat jouw functies "gewoon snel" hun werk doen, dan is dat helemaal geen probleem.

Daarbij kan het nog wel eens schelen hoe je door je data heen loopt. Vraag je eerst iets van 20.000 records op en ga je daar dan doorheen, waarbij je bij elk record 200 records opvraagt? Dat kan wel eens een trage actie zijn. Maar als blijkt dat die eerste 20.000 al anders gefilterd kan worden, waardoor er maar 150 over blijven en je daar per stuk 200 gerelateerde records op moet vragen, dat is wel te doen.

Ik kwam dit recent bij een eigen stuk code tegen. Ik moet voor een jaar (dus 365 records) gegevens opvragen, daar zaken aan koppelen en nog wat andere acties doen. En dat duurde wel lang. Je zit dus lang naar een "ladend scherm" te kijken en je denkt "wat gebeurt hier eigenlijk"?

Omdat ik de code lokaal had draaien heb ik aan het begin van de functie een System.Diagnostics.Stopwatch-object gemaakt. Na elke functie een System.Diagnostics.Debug.WriteLine($"Stap x: {stopWatch.ElapsedMilliseconds}"); stopWatch.Restart(); om te zien welke stap het langste duurde. Zo zag ik dat een aantal stappen 0 milliseconden duurden, maar andere sub-stappen 300 milliseconden. En als die 5 keer aangeroepen worden, dan is dat 1.5 seconde. Gaat dat 20x, dan zit je al op een halve minuut. Op basis daarvan heb ik mijn code aangepast, de SaveChangesAsync() om de wijzigingen naar database weg te schrijven uit te voeren pas op het moment dat alle wijzigingen uitgevoerd waren. Daarmee kon ik het hele proces binnen een redelijke tijd afronden.

Moraal van dit verhaal?

  • Zorg dat je naar je "View" een goed Object, met eigen properties en methoden doorgeeft, om de View "simpel" te houden, deze hoeft alleen data te tonen en moet zelf geen berekeningen meer doen!
  • Probeer je functies zoveel mogelijk met een "return"-statement te starten. Kun je redelijk snel het juiste resultaat terug geven? Beknopte code, leesbare code, fijn voor je collega's en voor jezelf als je naar een half jaar iets moet aanpassen in je eigen code!
  • Als je tijd hebt om je code op snelheid te controleren, doe het dan. Elke seconde extra die "iemand" moet wachten op resultaat is frustrerend. We zijn allemaal gewend aan "snelle sites, snelle reacties, snelle resultaten" en ook als je zelf zaken moet/wilt testen en je moet elke keer 5 seconden wachten voordat er "iets" gebeurt, daar ben je gauw klaar mee!