Writing Quality Code, Part 1

Read the first blog in this series: Don't skip, Hot take.

Alex werkt al geruime tijd in de wereld van softwareontwikkeling en heeft veel ervaring opgebouwd met codepatronen, best practices en andere aspecten van codekwaliteit. Vandaag begint Alex aan een nieuwe baan als lead software engineer bij Etrain, een bedrijf dat software ontwikkelt voor interne en online workshops. In eerste instantie sluit Alex zich aan bij een klein team van drie andere ontwikkelaars die werken aan het intern gebruikte boekingssysteem. Het team werkt grotendeels zonder toezicht. Naar verluidt gaat het niet goed.

Na de introducties en wat kennismaking bekijkt Alex de codebasis van de applicatie en vraagt het team wat hun grootste problemen zijn.

De teamleden zijn het allemaal eens. De codekwaliteit van de codebasis is verschrikkelijk. Alex, die de code inmiddels heeft gezien, kan dat moeilijk ontkennen. De volgende dag vragen ze Alex, als nieuwe en ervaren lead, om een aantal coderingsstandaarden en stijladviezen op te stellen voor de rest van het team. Ze noemen verschillende problemen.

De codebasis, zeggen ze, bevat meerdere implementaties van dezelfde functionaliteit (in de vorm van methoden of klassen die ongeveer hetzelfde doen, maar zich op verschillende plekken bevinden). “Dit schendt toch het DRY-principe, nietwaar?” vraagt een teamlid.
“Mijn grootste probleem is niet DRY,” zegt een andere collega. “Onze code is enorm inconsistent. Niet alleen qua toon en organisatie, maar ook in syntaxis en naamgevingsconventies.”
“En dat geldt dubbel voor onze git-commitberichten,” voegt de derde toe.
“Dat is niets vergeleken met onze codebloat.” Dat is het eerste teamlid weer. “Ik wed dat al die inconsistenties van jullie bijna helemaal verdwijnen als we alle verouderde hacks en ongebruikte klassen en methoden verwijderen.”
Een ander lacht. “Waar, maar dan verdubbelen we waarschijnlijk het aantal bugs, en die zijn al behoorlijk talrijk. Hoewel bugs kan ik nog verdragen. Het zijn de typfouten die me irriteren.”

Na wat verdere discussie stelt Alex het team een simpele vraag: “Welke schade veroorzaakt dit gebrek aan stijlen en standaarden eigenlijk?”

De argumenten vóór stijlen en standaarden

Alex’ vraag is, althans naar mijn mening, de enige relevante. De gewaagde stelling van deze blog is dat codeerstijlen en -standaarden sterk overschat worden. Je had dat vast al afgeleid, want het staat in de ondertitel, maar het is het waard om te herhalen. Voordat ik probeer die uitspraak te verdedigen, is het nodig om eerst te definiëren wat ermee bedoeld wordt.

Een codeerstijl is een reeks regels die voorschrijven hoe je namen van klassen, methoden en variabelen moet schrijven en/of afkorten, hoe je tabs, witruimte en inspringing gebruikt, wanneer en hoe je commentaar toevoegt, en andere elementen die de presentatie van code beïnvloeden. Het doel van het volgen van een codeerstijl is om de code beter leesbaar te maken en fouten te voorkomen. Een codeerstijl draait dus vooral om consistentie in de presentatie.

public class UserService
{
    readonly UserRepository userRepo;
    public UserService(UserRepository userRepo)
    {
        this.userRepo = userRepo;
    }
    public async Task Add(UserModel User)
    {
        await userRepo.CreateAsync(User);
        ///Commit the changes
        await userRepo.CommitAsync();
    }
}

Bij het beoordelen van de C#-code hierboven zou je op basis van stijladviezen de volgende wijzigingen kunnen aanbrengen:

public class UserService
{
    private readonly UserRepository _userRepository;
    public UserService(UserRepository userRepository)
    {
        _userRepository = userRepository;
    }
    public async Task AddAsync(UserModel user)
    {
        await _userRepository.CreateAsync(user);
        await _userRepository.CommitAsync();
    }
}

Zoals je kunt zien, verandert dat niets aan de functionaliteit, maar wel aan de manier waarop de code wordt gepresenteerd. Interessant genoeg zijn sommige van die veranderingen nogal discutabel. “Repo” schrijven is ongetwijfeld korter dan “repository”, en misschien zouden we dat kortere, niet-afgekorte alternatief moeten verkiezen. Het toevoegen van het achtervoegsel ‘Async’ aan de namen van asynchrone methoden is een goed idee als je duidelijk moet maken dat sommige methoden asynchroon zijn en andere niet, maar wat als bijna je hele applicatie asynchroon is? Dient het dan nog een doel, of vult het je codebasis alleen maar onnodig met modewoorden? En op dezelfde manier: moeten we die accessmodifier (private) echt toevoegen, of is dat slechts overbodige opvulling?

Verdergaand daarop is een codestandaard een reeks regels die voorschrijft hoe code geschreven moet worden. Dat is breder dan een codeerstijl, en sommige standaarden verwachten zelfs dat ontwikkelaars een bepaalde stijl gebruiken. Een codestandaard helpt of dwingt ontwikkelaars om keuzes te maken die passen bij de programmeertaal. In C# bevat een standaard bijvoorbeeld richtlijnen over hergebruik van code (zoals het DRY-principe), codeprincipes (zoals SOLID), de scope en het verbergen van implementatiedetails (oftewel encapsulatie), en nog veel meer. Een codestandaard is bedoeld om consistentie in ontwerp te waarborgen. Microsoft heeft bijvoorbeeld zijn eigen richtlijnen, die je daar kunt vinden.

Ter illustratie: na het toepassen van een bepaalde codestandaard zou de bovenstaande code er als volgt uitzien:

public class UserService : IUserService
{
    private readonly ILogger<UserService> _logger;
    private readonly IUserRepository _userRepository;
    public UserService(ILogger<UserService> logger, IUserRepository userRepository)
    {
        _logger = logger;
        _userRepository = userRepository;
    }

    public async Task<DataResult> AddAsync(UserModel user)
    {
        try        
        {
            await _userRepository.CreateAsync(user);
            await _userRepository.CommitAsync();
            return DataResult.Success;
        }
        catch (RepositoryException ex)
        {
            _logger.LogError(ex);
            return DataResult.Failure;
        }
    }
}

Een belangrijk punt waarop een codestandaard lijkt op een codeerstijl, is dat de waarde van zo’n standaard sterk afhangt van consistentie. Voorspelbaar loggen helpt bijvoorbeeld bij het debuggen en monitoren, en het gebruik van abstractie via interfaces helpt bij ontkoppeling, waardoor het eenvoudiger wordt om unittests te schrijven en later de implementatie van een klasse (en zijn afhankelijkheden) aan te passen. Dit is alleen effectief als het consequent in de hele code wordt toegepast. In dat geval verhoogt het de efficiëntie van een ontwikkelaar aanzienlijk, omdat er duidelijke regels zijn voor de eigen code en die van anderen, wat voorkomt dat je mentale energie verspilt aan keuzes tijdens het schrijven of aan het begrijpen van code tijdens het lezen.

Op basis van deze definities zorgen stijlen en standaarden dus voor consistente, leesbare code die overeenkomt met afgesproken best practices, waardoor ontwikkelaars efficiënter werken en tegelijkertijd bugs worden voorkomen. Klinkt goed, toch? Dat roept de vraag op: hoe kan het introduceren en onderhouden van codeerstijlen en standaarden dan ooit een slecht idee zijn?

De argumenten tégen stijlen en standaarden

Mijn eerste argument tegen stijlen en standaarden is gebaseerd op mijn eigen ervaringen in het werkveld. Stijlen en standaarden worden vaak een doel op zich, wat ik verbijsterend vind. Het zijn middelen, geen doelen. Althans, niet doelen waar iemand anders dan softwareontwikkelaars zelf zich druk om maakt. Een relevant doel zou zijn: we willen dat ons softwareontwikkelproces flexibel is.

En dat is een ambitieus doel. Aangezien de enige constante in softwareontwikkeling voortdurende verandering is, is het enorm belangrijk dat het schrijven van nieuwe code en het bijwerken van bestaande code eenvoudig en snel kan gebeuren. Je zou kunnen beargumenteren dat code zonder een consistente stijl en standaard moeilijk te onderhouden is, maar ik zou stellen dat in de meeste situaties juist het tegenovergestelde waar is. Strikt gehandhaafde codeerstijlen en -standaarden kunnen namelijk gemakkelijk leiden tot inflexibiliteit en het moeilijker maken om nieuwe, onverwachte functionaliteiten te implementeren. Wanneer je wordt beperkt door een vastgelegde stijl en dezelfde rigide regels, ongeacht de context, zal de efficiëntie afnemen, zeker wanneer die stijlen en standaarden verouderd raken of niet meer optimaal zijn voor het probleem dat je probeert op te lossen.

En wat dan met bugs? Stijlen en standaarden voorkomen toch bugs? Ik denk dat ze dat doen, ja - maar slechts indirect. En tegelijk ook niet. Aan de ene kant maakt consistente code het soms makkelijker om fouten in je logica op te merken en zo bugs op te sporen. Aan de andere kant: als je al je tijd besteedt aan stijlen en standaarden, houd je minder tijd over om daadwerkelijk bugs te vinden of te voorkomen. En mensen hebben nu eenmaal een beperkte hoeveelheid tijd en concentratie.

Laten we eerlijk zijn: veel zogenoemde bugs richten in de praktijk geen echte schade aan. Ze zijn hooguit irritant, of het zijn theoretische problemen met randgevallen die zich in werkelijkheid nooit voordoen. Maar zelfs als we het hebben over bugs die wél echte, tastbare problemen veroorzaken voor echte gebruikers… hoe vaak zouden die werkelijk voorkomen zijn dankzij stijlen en standaarden?

En ten slotte, als je kijkt naar de hoeveelheid tijd die sommige teams besteden aan deze onderwerpen, zou je bijna denken dat stijlen en standaarden het allerbelangrijkste zijn voor de waarde van je code. Hevige discussies over het wat, hoe en waarom van hoofdletters; eindeloze documentatiepagina’s met gekoppelde richtlijnen die tot stand kwamen na urenlange vergaderingen; refactoren van (vrijwel probleemloze) code puur omwille van consistentie; en een van mijn grootste ergernissen: pull requests waarin de enige opmerkingen gaan over tabs en ‘naked ifs. Alsjeblieft, als je een deel van je kostbare tijd vrijmaakt om iemands code te reviewen, besteed die dan niet aan grammatica of theoretische checklistjes van patronen. Richt je op de echte risico’s en voordelen van de code.

Laat me afsluiten met een metafoor. Artsen hebben naar verluidt een vreselijk handschrift. Maar dat recept dat je na een consult meekrijgt? Dat bevat nog steeds waardevol medisch inzicht. Is dat onleesbare handschrift irritant? Absoluut. Maar verwar een rommelige presentatie niet met een gebrek aan kwaliteit of juistheid.

De schade meten

Als het gebruiken van stijlen en standaarden problemen veroorzaakt, maar het níet gebruiken ervan óók problemen oplevert, wat moeten we dan doen? Waarom niet het midden zoeken, zou je kunnen zeggen. Wees pragmatisch en pas stijlen en standaarden alleen toe als ze bij het probleem passen. Sommige delen van de code zijn belangrijker dan andere, dus het slaat nergens op om ze allemaal aan dezelfde eisen te laten voldoen, toch?

Wanneer de verwachting van een bepaalde stijl of standaard botst met de afwezigheid daarvan, kan dat juist grotere problemen veroorzaken. Het is vergelijkbaar met verouderde comments of tests die onterecht slagen: als ontwikkelaars aannemen dat een bepaalde stijl of standaard wordt gevolgd, maar die onverwacht of onduidelijk ontbreekt… dan kan die aanname leiden tot ernstige fouten. In plaats van pragmatisch te zijn, kunnen zulke opportunistische stijlen en standaarden juist het slechtste van beide werelden combineren.

Dus, noch A, noch Z, noch iets daartussenin lijkt de juiste keuze. Waar laat dat ons, software engineers? Ik zou zeggen dat je eerste reflex altijd moet zijn om te meten voordat je handelt, iets wat goed aansluit bij de Agile, en DevOps-gedachte waar ik veel ervaring mee heb.

Meet de tijd die nodig is om nieuwe functionaliteiten aan te passen. Meet de Mean Time to Change (MTTC), de Mean Time Between Failure (MTBF) en de Mean Time To Repair (MTTR). Meet gebruikerswaarderingen en de juistheid van de software. En, het belangrijkste, bepaal wat acceptabele scores voor die metingen zijn. Dáár zouden urenlange codekwaliteitsvergaderingen over moeten gaan.

Stel dat jij en je team al die metingen hebben verzameld. Dán kun je de vraag van Alex aan het begin van deze blog beantwoorden: Welke schade wordt er nu eigenlijk aangericht? En wat kunnen we doen om dat te herstellen?

Mijn oplossingen

Op basis van de voorgaande alinea’s zou je misschien denken dat ik helemaal niet geloof in codestandaarden. En dat is - enigszins voorspelbaar - niet het geval.

Ten eerste geloof ik in standaarden binnen processen. De manier waarop een software engineer de kwaliteit van code beoordeelt, moet op de een of andere manier gestandaardiseerd zijn (bijvoorbeeld via code reviews met pull requests). Maar tijdens code reviews moet je niet de code zelf beoordelen, beoordeel de functionaliteit. Denk aan de randgevallen. Benader een code review als een samenwerking tussen gelijken (want dat ben je). Je probeert samen de code te verbeteren. De slechtste manier om een code review te benaderen, is alsof je een docent bent die een werk nakijkt, of een auditor met een checklist.

Ten tweede denk ik dat stijlen en standaarden nuttig kunnen zijn, zolang we er geen handmatig werk aan verspillen. Gebruik automatisering. Als je een set stijlen automatisch kunt toepassen bij het opslaan van een bestand, doe dat dan. De specifieke stijl die je kiest is bijna irrelevant; met automatisering kun je stijlen consequent toepassen, en dat is de grootste winst. Voor standaarden ligt dat wat moeilijker, maar er zijn genoeg tools die statische code-analyse uitvoeren (zoals SonarQube en SonarCloud). Deze tools markeren automatisch probleemgebieden, niet op basis van de ideeën van jouw team, maar van de bredere community. En meestal biedt je IDE - of dat nu Visual Studio, Rider of Eclipse is - een snelle, halfautomatische manier om zulke problemen te refactoren. Dit proces heeft geen reviewer nodig en zou idealiter vóór de handmatige code review moeten plaatsvinden, om inspanning te minimaliseren. Als je een standaard niet volledig of deels kunt automatiseren, zou ik die niet als standaard bestempelen. Beschouw het dan hooguit als een richtlijn. Want uiteindelijk wegen de nadelen van standaarden die veel handmatig werk vereisen, zwaarder dan de voordelen.

En tenslotte, en misschien wel het belangrijkst, mijn enige echte standaard voor codekwaliteit: elke code die onderdeel is van een bedrijfsapplicatie moet makkelijk te debuggen zijn (handmatig) en testbaar zijn (geautomatiseerd, bijvoorbeeld met unit tests). Als je in staat bent om aan die standaard te voldoen, dan zijn andere stijlen en standaarden van veel minder belang. Code die moeilijk te debuggen is, heeft wellicht meer toegankelijke ingangen nodig (en hé, een unit test kan precies zo’n ingang zijn, handig, toch?). Aan de andere kant is code die moeilijk te testen is vaak óf te simpel óf niet abstract genoeg. En trouwens, de makkelijkste manier om ervoor te zorgen dat je code testbaar is, is door hem te testen. Ik zeg niet dat je moet streven naar 100% testdekking (dat leidt alleen maar tot nieuwe starheid), en ja, slechte of verouderde tests kunnen gevaarlijker zijn dan helemaal geen tests… maar unit testing is de meest efficiënte manier om kwaliteit in je code uit te nodigen én te behouden. Het is zelfs zo belangrijk dat geautomatiseerde tests de hoofdrol zullen spelen in het volgende deel van deze reeks. Dus, vergeet stijlen en standaarden, zolang je maar tests hebt. Hoe vind je dát als gewaagde stelling?

Het boekingssysteem verbeteren

Na een periode van analyse en metingen identificeert het team van Alex een aantal probleemgebieden waar de starheid, het aantal fouten en de ernst van de bugs onacceptabel zijn voor zowel het team als het bedrijf. Allereerst veroorzaken sommige bugs echte schade, zowel voor eindgebruikers als voor de ontwikkeltijd. Deze krijgen de hoogste prioriteit om op te lossen, terwijl andere bugs voorlopig mogen blijven bestaan. Het team refactort de probleemgebieden zodat ze eenvoudiger te debuggen zijn, waarbij telkens oplossingen worden gekozen die passen bij de context van het specifieke geval. Alex configureert een stijlset die ook door andere teams bij Etrain wordt gebruikt. De stijl wordt automatisch toegepast bij het opslaan van een codebestand. Als ze ooit willen overstappen op een andere stijlset, kost dat nauwelijks moeite. Daarnaast configureert Alex een statische code-analysetool voor het boekingssysteem, die automatisch code markeert met potentiële risico’s. De tool wordt niet gebruikt als poortwachter, maar als een geautomatiseerde bron van reflectie voor de auteur van de code, een soort kunstmatig intelligente rubberen eend, als je wilt.

“Nu moeten we beginnen met het schrijven van unittests,” zegt Alex tijdens een online vergadering.
“Makkelijker gezegd dan gedaan,” werpt een ander teamlid tegen.
“Geen zorgen,” antwoordt Alex terwijl hij op de knop ‘scherm delen’ klikt.
Terwijl de rest van het team meekijkt, maakt Alex een nieuw testproject aan. “Bijna alles wat een software engineer doet, is makkelijker gezegd dan gedaan. Maar dat heeft me nog nooit tegengehouden.

Een overzicht van deze hele blog serie door Jelle.

• Don't Skip, Hot Take
• Writing Quality Code (part 2)
• Building Quality Software (part 1)
• Building Quality Software (part 2)
• Creating Ethical Software (part 1)
• Creating Ethical Software (part 2)
• The Development Process (part 1)
• The Development Process (part 2)
• The Software Engineer Oath