Kommentarer

I mitt förra inlägg tog jag upp duplicering och mitt exempel kunde kortas ner väsentligt genom att ta bort kommentaren i metodsignaturen. Kommentaren i det exemplet var ren duplicering av det som stod i metodsignaturen och där med reduant.

Så när, var och hur ska man kommentera. Svaret är enkelt. Det som inte kan uttryckas i kod skall kommenteras. Allt annat ska man låta bli att kommentera. Använd i stället lite mer beskrivande namn på saker och ting och framför allt förändra din kod så att den går att beskriva i kod. Det är i princip bara dålig kod som bör refaktoreras som behöver kommenterar. Trist men sant.

Så låt oss ta ett exempel med kommenter och se om vi kan klara oss utan.

/**
* Creates a employee
**/
public void CreateEmployee(Employee employee){}

Detta är samma exempel som jag hade i förra inlägget. Kommentaren tillför inget och kommer sannolikt bli felaktig vid första bästa förändring. Vi bygger ut exemplet lite med en body:

public void CreateEmployee(Employee employee{
// Check that name is not null
if(employee.name == null){
throw new Exception("Employee must have a name")
}

// Check that employeeNumber is not already set
if(employee.number != 0){
throw new Exception("Employee number is already set");
}

// Add into persistent storage
DataProvider.insert(employee);
}

Metoden ser rimligt ren ut men har tre kommentarer. Är dessa nödvändiga eller kan vi skriva metoden på ett annat sätt som skulle göra både koden mer lättläst och dessutom kunna ta bort kommentarerna. Mitt förslag är att dela upp metoden.

public void CreateEmployee(Employee employee){
ValidateNewEmployee(employee);
PersistEmployee(employee);
}

private void ValidateNewEmployee(Employee employee){
if(employee.name == null){
throw new Exception("Employee must have a name");
}

if(employee.number != 0){
throw new Exception("Employee number is already set");
}
}

private void persistEmployee(Employee employee){
DataProvider.insert(employee);
}

Det som har hänt är att jag har brutit ut funktionaliteten i CreateEmployee till två privata metoder som står för all action. CreateEmployee har en kort lista på de logiska steg som skall genomföras men gör inget själv. Vi har inte vunnit kortare kod men vi har vunnit enormt i läsbarhet. Direkt när man tittar på CreateEmployee ser man vad metoden tänker sig att den ska göra och man kan gå direkt till den del man är intresserad av.

Huvudregel: Alla former av duplicering är av ondo

Sedan man började programmera har man infört fler och fler begrepp för att undvika redunans i koden. Man har brutit ut metoder, skapat object, patterns etc.

Vi tittar på en metod definierad i ett interface och funderar på vilken information som är duplicerad.

/**
* Create employee
**/
public void createEmployee(Employee employee);

Det första vi kan stryka är kommentaren. Den tillför inget som inte metodsignaturern redan beskriver.

Sedan kan man fundera på om metodnamnet är lämpligt. Vi duplicerar ordet employee två gånger. Vi testar att ta bort ordet Employee från CreateEmployee och tittar på resultatet.

public void create(Employee employee);

Metodsignaturer beskriver precis samma sak som det första exemplet. Den innehåller ingen kommentar av flera anledningar

1. Kommentaren i exemplet tillför inget mervärde för koden. Det enda del tillför är mer text att läsa.
2. Kommentaren kommer med hög sannolikhet att bli felaktig.Tex om man använda automatiskt refaktorering (ändra på ett ställe och alla ställen som använder samma kod uppdateras i hela kodbasen) är det inte troligt att kommentaren kommer att uppdateras. Alltså kommer koden snart innehålla felaktiga kommentarer
   

Clean code

Sitter och läser en bok som heter clean code och tänke försöka att få ner lite anteckningar och då verkar ju en blogg vara ett lämpligt ställe att göra det på.