Eenvoudige tips voor clean code!

Dit artikel verscheen oorspronkelijk in Java Magazine #1-2024  en is met permissie van de NLJUG op deze plek gepubliceerd.

Iedereen wil nette code schrijven, toch? Nette code schrijven is jammer genoeg niet altijd even rechttoe rechtaan. Als je code smells niet weet te herkennen, is het erg makkelijk om deze te missen tijdens pull requests of wanneer je zelf aan het programmeren bent!

Dit artikel toont een aantal niet ongebruikelijke code smells, hoe je ze kunt identificeren en oplossen, om zo je codebase weer een stukje schoner te maken! Wist je bijvoorbeeld dat commentaar in code in de meeste gevallen gezien kan worden als een code smell? In dit artikel uit Java Magazine vertelt Michael Koers je waarom!

Introductie

Clean code schrijven is makkelijker gezegd dan gedaan. Het is niet altijd meteen duidelijk wat de mooiste oplossing is voor een bepaald probleem. Code smells kunnen het resultaat zijn van het programmeren volgens je denkproces, zonder vooraf na te denken over de structuur van je klassen of methoden.

Soms is het moeilijk om code smells te identificeren. Je kijkt naar een stuk code en je weet dat er iets mis  is, maar je kunt niet precies aanwijzen waar de fout vandaan komt, of misschien weet je niet hoe je het kunt verbeteren.

Dit artikel gaat in op enkele code smells die je kunnen verrassen als je niet met clean code in gedachten programmeert. De fouten die worden besproken zijn gekozen omdat ze gemakkelijk in je code kunnen glippen, en het oplossen ervan niet veel tijd kost, terwijl het je code ook enorm opschoont.

Door de code smells te laten zien via code snippets, laat dit artikel je zien hoe de code smells eruit zien en hoe je de code kunt opschonen. Er zijn veel meer code smells dan hier besproken worden, maar hopelijk raak je geïnteresseerd of zelfs enthousiast om meer  clean code te schrijven!

Geneste logica

Dit soort code smell is een goed voorbeeld van wat er gebeurt als je het programmeren laat leiden door je denkproces, zonder vooraf na te denken over de structuur. Stel dat je wat logica in een methode moet schrijven, maar deze logica hoeft alleen te worden uitgevoerd als een bepaalde boolean expressie waar is. Zonder na te denken, plaats je de logica mogelijk in een if-statement. Zie Listing 1 voor een voorbeeld.

Listing 1

// Smelly, all logic is in if-statement
public String transform(String someString) {
if (!someString.isEmpty()) {
someString = someString.strip();
someString = someString.toUpperCase();

/* More logic */
}
return someString;

In de minder nette variant van de transform-methode bevindt alle logica zich in een if-statement. Deze manier van code schrijven komt voort uit je denkproces: “Oké, voer deze logica alleen uit als de gegeven String niet leeg is”. Dit leidt er echter snel toe dat je al je logica in een if-statement plaatst, wat minder mooi is. Waarom? Omdat ingesprongen code moeilijker te lezen is. Overmatig inspringen maakt het moeilijker om de code gemakkelijk te scannen, omdat de instructies verspreid staan over de pagina in plaats van in een compact formaat [1].

De oplossing voor dit probleem is heel eenvoudig: je keert de if-statement om. In plaats van alle logica te nesten in een if-statement, wordt het if-statement omgedraaid – waardoor de uitvoering van je methode stopt als die niet voldoet aan de boolean expressie. Zie de verbeterde versie in Listing 2.

Nu hoeft je logica niet meer genest te zijn. Dit soort oplossing wordt een “Guard Clause” genoemd.

Listing 2

// Refactored by inverting if-statement, applying a guard-clause
public String transform(String someString) {
if (someString.isEmpty()) {
return someString;
}

someString = someString.strip();
someString = someString.toUpperCase();

/* More logic */

return someString;
}

Een extremere versie van dit soort code smell is wanneer je meerdere geneste if-statements hebt, wat resulteert in Listing 3. Al deze nesting laat de code er zeer ingewikkeld uitzien en aanvoelen.

Listing 3

// Smelly, all logic is nested
public static String transform(String someString) {
if (!someString.isEmpty()) {
if (someString.length() < 5) { someString = someString.toUpperCase(); } else { if (someString.length() > 10) {
someString = someString.toUpperCase();
} else {
someString = someString.toLowerCase();
}
}
}
return someString;
}

Dit probleem is vrij eenvoudig op te lossen, als je weet hoe je het moet aanpakken. Zoek de buitenste conditie en maak daar een guard clause van. In het geval van dit voorbeeld is de buitenste conditie de .isEmpty() check. Door dit om te zetten in een guard clause kunnen we één laag van de geneste logica verwijderen. Herhaal dit proces nu voor alle lagen die in aanmerking komen. Het resultaat is een methode die een veel duidelijkere flow heeft. Zie Listing 4 voor de verbeterde versie.

Listing 4

// Refactored by inverting if-statements using guard clauses
public static String transform(String someString) {
if (someString.isEmpty()) return someString;
// consolidated conditional expression
if (someString.length() < 5 || someString.length() > 10) return someString.toUpperCase();

return someString.toLowerCase();
}

Als je merkt dat meerdere condities hetzelfde resultaat opleveren, kun je de conditionele controles samenvoegen tot een enkele conditionele check. Dit bereik je met behulp van de ‘and’ of ‘or’ operator, waardoor de cyclomatische complexiteit een beetje wordt verminderd.

Staat en gedrag splitsen

Deze code smell is interessant omdat het tegen het idee van Object Oriented design ingaat. Hier wordt de staat en het gedrag van een object gescheiden in twee of meer klassen, terwijl ze onderdeel van hetzelfde object hadden kunnen zijn. Zie Listing 5. In het voorbeeld moeten de velden van User toegankelijk zijn omdat de UserManager informatie over de User moet wijzigen, wat het verbergen van informatie verhindert.

Deze code smell kan moeilijker te detecteren zijn. Een manier om de fout op te sporen, is door te controleren of een methode binnen een klasse uitsluitend werkt op de methode parameters. Dit geeft aan dat de klasse geen gegevens bevat die door de methode worden gebruikt, wat betekent dat staat en gedrag hier niet synchroon zijn.

Listing 5

// Smelly, state (User) and behaviour (UserManager) are seperated
class User {
int securityLevel;
boolean admin;
}

class UserManager {
boolean hasSecurityClearance(User user, int req) {
return user.admin || user.securityLevel >= req;
}

int determineClearanceLevel(User user) {
return user.admin ? MAX_LEVEL : user.securityLevel;
}

Bij dit probleem wil je staat en gedrag weer samenbrengen. In het geval van het voorbeeld betekent dit dat de methoden van UserManager naar het User-object zelf worden verplaatst. De velden van User hoeven niet langer toegankelijk te zijn en kunnen verborgen worden achter de methoden van de klasse. Nu kun je het beveiligingsniveau van een User controleren op het User-object zelf, wat veel cleaner aanvoelt en ook één parameter minder vereist. Een ander voordeel is dat het User-object nu meer controle heeft over wat er met zijn staat gebeurt. Dit maakt de weg vrijnaar immutable objecten. Zie Listing 6 voor de gerefactorde versie.

Listing 6

// Refactored to combine state and behaviour
class User {
    private int securityLevel;
    private boolean admin;

    boolean hasSecurityClearance(int req) {
        return admin || securityLevel >= req;
    }

    int determineClearanceLevel() {
        return admin ? MAX_LEVEL : securityLevel;
    }
}

In mijn ervaring is de kans groter dat je deze code smell aantreft in utility-achtige klassen, omdat die de neiging hebben andere objecten te “beheren”.

Commentaar

Een onderdeel van SonarQube’s statistieken verzameling is de “Comments (%)”, die meet hoeveel commentaarregels je in een bestand schrijft in vergelijking tot code [2], waarop je bepaalde drempels kunt instellen voor je quality gates om op te reageren. Maar als het je lukt om clean code te schrijven met duidelijke naamgeving en beknopte methoden, zou het schrijven van commentaar onnodig moeten zijn. Dus waarom zou je in dat geval commentaar willen schrijven?

Afhankelijk van wie je het vraagt, krijg je verschillende antwoorden op de vraag of- en hoeveel commentaar men in een applicatie moet schrijven. Als je op internet zoekt naar wanneer je commentaar schrijft, kom je waarschijnlijk iemand tegen die die vraag beantwoordt met “code zou zichzelf moeten verklaren”. Dit is natuurlijk waar,  maar zo’n kort antwoord maakt dat het moeilijk te begrijpen is wat dat inhoudt. Er zijn bepaalde dingen die gewoon niet vastgelegd kunnen worden door clean code te schrijven, zoals beslissingen die voor bepaalde oplossingen zijn genomen.

Commentaar kan dus goed zijn, maar kan ook regelmatig als slecht worden gezien. Voorbeelden van slecht commentaar zijn:

Overbodig commentaar: Commentaar dat geen nieuwe informatie toevoegt in tegenstelling tot wat je al uit de code zelf kunt lezen of begrijpen. Dit soort commentaar moet gewoon worden verwijderd.

Code in commentaar: Code die ooit deel uitmaakte van het systeem, maar op een bepaald moment in commentaar is gezet. Code in commentaar voegen rommel toe en moeten  worden verwijderd. Tegenwoordig zouden alle projecten (hopelijk) in een soort versiebeheer moeten staan. Wanneer je die code ooit nodig hebt, kun je die terughalen uit je versiebeheer. Je kunt een opmerking achterlaten over het eerdere bestaan van de code in commentaar, voor het geval andere ontwikkelaars de verwijderde code opnieuw moeten bekijken.

Commentaar die variabelen/methoden uitlegt: Commentaar die uitlegt wat methoden doen of wat variabelen betekenen, maskeren een code smell. Het betekent dat de methode of variabele hernoemd moet worden om duidelijker weer te geven wat ze doen of betekenen. Eenmaal aangepast, kan het commentaar worden verwijderd.

Maar niet al het commentaar hoeft slecht te zijn. Enkele voorbeelden van goed commentaar zijn:

Implementatiebeslissingen documenteren: Implementatiebeslissingen kunnen niet worden afgeleid uit methode- of variabele namen. Deze kunnen gedocumenteerd worden met behulp van commentaar in je code. Door ze te documenteren wordt het voor andere ontwikkelaars duidelijk waarom een bepaalde beslissing is genomen. Geldige redenen zijn onder andere compatibiliteit, een bug of andere beperkingen.

JavaDoc: Het documenteren van je packages, klassen, interfaces, methoden en/of constructors met JavaDoc is over het algemeen een goed idee. Vooral als het code betreft, zoals een API, die gedeeld wordt tussen teams. Het schrijven van goede JavaDoc stelt andere ontwikkelaars in staat om je code te begrijpen en correct te gebruiken.

Zie Listing 7 voor een voorbeeld met nutteloos commentaar, en Listing 8 voor dezelfde code, maar deze keer voorzien van zinvoller commentaar.

Onthoud dat commentaar, als het niet goed wordt toegepast, meer rommel aan de code toevoegt. Het zijn meer regels voor ontwikkelaars om te lezen. Schrijf alleen commentaar als het echt waarde toevoegt en houd het beknopt om je collega-ontwikkelaars blij te maken.

Listing 7

// Smelly, lots of comments that don't add any value or mask a code smell
class Order {

    // List of items in the order                   <- Superfluous
    private List<String> list = new ArrayList<>();
    private double total;

    // Add item to order                            <- Superfluous
    void add(String item, double price) {
        list.add(item);
        total += price * 1.08d; // Add sales tax    <- Explains constant
        Collections.sort(list);
        // list.sort((o1, o2) -> o1.compareTo(o2)); <- Commented out code
    }

    // See if order contains some other item        <- Explains method
    boolean isInOrder(String o) {
        // Fast search                              <- Poorly documented decision
        return Collections.binarySearch(list, o) != -1;
    }
}

Listing 8

// Refactored, commented out code and superfluous comments have been removed
// Implementation decision has been documented more clearly, sales tax has been
// turned into a constant and comment explaining method has had its method 
// renamed
class Order {
    private List<String> items = new ArrayList<>();
    private double total;

    /**
     * Adds given item to the order
     * @param item      Item name, to add to the order
     * @param price     Item price, must be excluding sales tax
     */
    void add(String item, double price) {
        items.add(item);
        total += price * SALES_TAX;
        // Important to keep collection sorted, see containsItem() method
        Collections.sort(items);
    }

    /** {...}
     */
    boolean containsItem(String other) {
        /*
         * We use Binary Search on the items collection because
         * we noticed performance issue when the size of the order
         * grew to be > 1000, this drastically improved search performance
         * but requires us to keep the collection sorted at all times.
         */
        return Collections.binarySearch(items, other) != -1;
    }
}

Conclusie

Dit artikel wijst slechts op enkele code smells, maar er zijn veel meer soorten code smells die in je codebase kunnen sluipen. Als je na het lezen van dit artikel geïnteresseerd bent in het lezen van meer tips voor clean code op een vergelijkbare manier als in dit artikel, bekijk dan het boek “Java by Comparison” van Simon Harrer, Jörg Lenhard en Linus Dietz [3]. Het behandelt 70 verschillende tips voor clean code. Deze zijn allemaal voorzien van voorbeelden van code voor en na, waardoor het probleem en de oplossing zo duidelijk mogelijk worden. Een ander goed boek over refactoring is “Refactoring” van Martin Fowler, dat hetzelfde onderwerp behandelt [4].

Hopelijk heb je enkele nieuwe tips voor clean code geleerd die je kunt gaan toepassen in je dagelijkse werk. Vergeet niet om je collega’s/ontwikkelaars op te leiden zodat we allemaal clean code leren schrijven!

Referenties

[1] https://www.cs.umd.edu/~ben/papers/Miara1983Program.pdf

[2] https://docs.sonarsource.com/sonarqube/latest/user-guide/metric-definitions/#size

[3] https://java.by-comparison.com/

[4] https://martinfowler.com/books/refactoring.html

BIO

Michael Koers is een softwareconsultant bij Info Support, met de meeste kennis in Java en Azure. Hij deelt graag zijn kennis, leert en daagt anderen uit en raakt enthousiast over domotica.