Capitolo 4

Commenti
di Brian W. Kernighan e P. J. Plaugher

Non commentate il cattivo codice; piuttosto, riscrivetelo. ([KP78], p. 144)

Nulla può essere più utile di un buon commento ben posizionato. Nulla può rendere più disturbante un modulo di un ammasso di commenti frivoli e inutili. Nulla può essere più pericoloso di un vecchio commento che propaga falsità ed errate informazioni.

I commenti non sono come la “Lista di Schindler”. Non sono “pura bontà”. In effetti, i commenti sono, al massimo, un male necessario. Se i nostri linguaggi di programmazione fossero sufficientemente espressivi o se avessimo il talento necessario per impiegare tali linguaggi per esprimere il nostro intento, non dovremmo usare molti commenti (magari proprio nessun commento).

L’uso corretto dei commenti ha lo scopo di compensare la nostra incapacità di esprimerci con il solo codice. Notate che ho usato la parola “incapacità”. Lo ammetto: i commenti sono sempre dei fallimenti. Dobbiamo scriverli, perché senza di essi non sempre è possibile esprimere quanto dobbiamo dire, ma il loro uso non è certo una gioia.

Pertanto quando vi capiterà di dover scrivere un commento, pensateci bene e cercate di scoprire se non esiste un modo per esprimere la stessa cosa nel codice. Ogni volta che riuscite a esprimervi con il solo codice, datevi una “pacca sulla spalla”. Ogni volta che scrivete un commento, fate in modo di provare il senso di incapacità di esprimersi appieno con il solo codice.

Perché sono così ostile ai commenti? Perché mentono. Non sempre e non intenzionalmente, ma troppo spesso. Più è vecchio un commento e più si allontana dal codice che descrive, e quindi più è probabile che sia, semplicemente, sbagliato. Il motivo è semplice. I programmatori non possono, realisticamente, eseguirne la manutenzione.

Il codice cambia e subisce una sua evoluzione. Parti di esso si spostano da un punto a un altro. Tali parti si scindono e si riproducono e poi si rimettono insieme come chimere. Sfortunatamente i commenti non sempre seguono le loro evoluzioni, sarebbe impossibile. E fin troppo spesso i commenti finiscono per allontanarsi dal codice che descrivono e divengono come orfani, sempre meno accurati. Per esempio, osservate che cosa è accaduto a questo commento e alla riga che intendeva descrivere:

MockRequest request;
private final String HTTP_DATE REGEXP =
"[SMTWF][a-z]{2}\\,\\s[0-9]{2}\\s[JFMASOND][a-z]{2}\\s" +
"[0-9]{4}\\s[0-9]{2}\\:[0-9]{2}\\:[0-9]{2}\\sGMT";
private Response response;
private FitNesseContext context;
private FileResponder responder;
private Locale saveLocale;
// Esempio: "Tue, 02 Apr 2003 22:18:49 GMT"

Le variabili di istanza probabilmente sono state aggiunte successivamente fra la costante HTTP_DATE_REGEXP e quello che doveva essere il commento descrittivo.

È anche possibile ribadire che i programmatori dovrebbero essere disciplinati al punto da aggiornare i commenti, in termini di rilevanza e accuratezza. Sono d’accordo, dovrebbero. Ma penso che sia meglio dedicare le energie a rendere il codice talmente chiaro ed espressivo da non aver bisogno di commenti.

I commenti imprecisi sono molto peggiori dell’assenza di commenti: sono deludenti e fuorvianti. Stabiliscono aspettative che non verranno mai esaudite. Stabiliscono regole che non vigono più o addirittura che andrebbero evitate.

La verità si trova in un solo luogo: il codice. Solo il codice dice davvero quello che fa. È l’unica fonte di informazioni davvero accurate. Pertanto, sebbene i commenti possano talvolta essere necessari, cerchiamo in tutti i modi di ridurli al minimo.

I commenti non bastano a migliorare il codice cattivo

Una delle motivazioni che più spingono a scrivere commenti è la problematicità del codice. Scriviamo un modulo e sappiamo che è confuso e disorganizzato. Sappiamo che è un bel groviglio. Allora ci diciamo: “Ummm… sarà meglio che ci metta un commento qui!”. No! Sarà meglio che lo sistemiate!

Poter contare su codice chiaro ed espressivo con pochi commenti è molto meglio che avere codice complesso e intricato ma ricco di commenti. Invece di spendere il vostro tempo a scrivere commenti che spiegano il groviglio che avete fatto, dedicatelo a dipanare quel groviglio.

Spiegatevi nel codice

Vi sono certamente situazioni in cui il codice non riesce a spiegare se stesso. Sfortunatamente, molti programmatori lo addotano come scusa, sostenendo che il codice solo raramente, forse mai, può spiegare se stesso. Questo è palesemente falso. Che cosa preferireste trovare? Questo:

// Controlla se employee ha diritto a tutti i benefit
if ((employee.flags & HOURLY_FLAG) &&
    (employee.age > 65))

O questo?

if (employee.isEligibleForFullBenefits())

Pochi secondi sono sufficienti per comprendere l’intento del codice. In molti casi si tratta semplicemente di creare una funzione che dice la stessa cosa del commento che pensate di scrivere.

Buoni commenti

Alcuni commenti sono necessari o utili. Ne vedremo ora alcuni che considero meritevoli del tempo impiegato per scriverli. Tenete però sempre in considerazione che l’unico commento buono è quel commento che trovate il modo di non scrivere.

Commenti legali

Talvolta gli standard aziendali nell’ambito della programmazione ci obbligano a scrivere determinati commenti per motivi legali. Per esempio, le note di copyright e l’autore sono informazioni necessarie e ragionevoli per un commento posto all’inizio di ogni file di codice sorgente. Ecco, per esempio, il commento standard che poniamo all’inizio di ogni file di codice sorgente in FitNesse. Sono felice che il nostro IDE nasconda automaticamente questo commento.

// Copyright (C) 2003,2004,2005 by Object Mentor, Inc. All rights reserved.
// Released under the terms di the GNU General Public License version 2 o later.

Commenti come questo non dovrebbero essere contratti o pagine e pagine di “legalese”. Ove possibile, fate riferimento a una licenza standard o a un altro documento esterno invece di inserire nel commento tutti i termini e le condizioni.

Commenti informativi

Talvolta è utile fornire alcune informazioni tramite un commento. Per esempio, considerate questo commento che spiega il valore restituito da un metodo astratto:

// Restituisce un'istanza del Responder sottoposto a test.
protected abstract Responder responderInstance();

Un commento come questo talvolta può essere utile, ma ove possibile è meglio usare il nome della funzione per suggerire tale informazione. Per esempio, in questo caso il commento potrebbe essere reso ridondante rinominando la funzione: responderBeingTested.

Ecco un caso un po’ migliore:

// formato richiesto: kk:mm:ss EEE, MMM dd, yyyy 
Pattern timeMatcher = Pattern.compile(
  "\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*");

In questo caso il commento ci fa sapere che l’espressione regolare deve individuare un’ora e una data formattate con la funzione SimpleDateFormat.format usando la stringa di formattazione specificata. Ciononostante, tutto sarebbe più chiaro e pulito, se questo codice fosse stato spostato in un’apposita classe speciale che convertisse il formato delle date e delle ore. A questo punto, probabilmente il commento sarebbe diventato superfluo.

Descrizione dell’intento

Talvolta un commento va ben oltre la descrizione dell’implementazione e fornisce la motivazione di una decisione. Nel seguente caso vediamo un’interessante decisione documentata da un commento. Per confrontare due oggetti, l’autore ha deciso di voler ordinare gli oggetti della sua classe prima degli oggetti di ogni altra.

public int compareTo(Object o)
{
if(o instanceof WikiPagePath)
{
WikiPagePath p = (WikiPagePath) o;
String compressedName = StringUtil.join(names, "");
String compressedArgumentName = StringUtil.join(p.names, "");
return compressedName.compareTo(compressedArgumentName);
}
return 1; // siamo maggiori perché siamo del tipo corretto.
}

Ecco un esempio anche migliore. Potreste non concordare con la soluzione del problema scelta dal programmatore, ma almeno ora sapete che cosa tentava di fare.

public void testConcurrentAddWidgets() throws Exception {
WidgetBuilder widgetBuilder =
new WidgetBuilder(new Class[]{BoldWidget.class});
String text = "'''bold text'''";
ParentWidget parent =
new BoldWidget(new MockWidgetRoot(), "'''bold text'''");
AtomicBoolean failFlag = new AtomicBoolean();
failFlag.set(false);
//Questo è il nostro miglior tentativo di ottenere una competizione
//creando un gran numero di thread.
for (int i = 0; i < 25000; i++) {
WidgetBuilderThread widgetBuilderThread =
new WidgetBuilderThread(widgetBuilder, text, parent, failFlag);
Thread thread = new Thread(widgetBuilderThread);
thread.start();
}
assertEquals(false, failFlag.get());
}

Chiarimenti

Talvolta sembra opportuno tradurre il significato di uno strano argomento o valore restituito in qualcosa di leggibile. In generale è meglio trovare un modo per chiarire il significato di tale argomento o valore restituito; ma quando esso fa parte della libreria standard o di codice che non è possibile modificare, un commento chiarificatore può essere utile.

public void testCompareTo() throws Exception {
WikiPagePath a = PathParser.parse("PageA");
WikiPagePath ab = PathParser.parse("PageA.PageB");
WikiPagePath b = PathParser.parse("PageB");
WikiPagePath aa = PathParser.parse("PageA.PageA");
WikiPagePath bb = PathParser.parse("PageB.PageB");
WikiPagePath ba = PathParser.parse("PageB.PageA");
assertTrue(a.compareTo(a) == 0); // a == a
assertTrue(a.compareTo(b) != 0); // a != b
assertTrue(ab.compareTo(ab) == 0); // ab == ab
assertTrue(a.compareTo(b) == -1); // a < b
assertTrue(aa.compareTo(ab) == -1); // aa < ab
assertTrue(ba.compareTo(bb) == -1); // ba < bb
assertTrue(b.compareTo(a) == 1); // b > a
assertTrue(ab.compareTo(aa) == 1); // ab > aa
assertTrue(bb.compareTo(ba) == 1); // bb > ba
}

Si corre però il rischio, naturalmente, che il commento sia impreciso. Scorrete l’esempio precedente e vedrete che è difficile verificare che tutto sia corretto. Questo spiega sia perché il chiarimento è necessario, sia perché è pericoloso. Pertanto, prima di scrivere dei commenti come questo, verificate che non vi sia un modo migliore di procedere e poi verificate assolutamente che i commenti siano precisi.

Avvertenze

Talvolta è utile mettere in guardia gli altri programmatori su determinate conseguenze. Per esempio, ecco un commento che spiega perché un determinato caso di test è stato disattivato:

// Non eseguite, a meno che
// abbiate del tempo da perdere.
public void _testWithReallyBigFile()
{
writeLinesToFile(10000000);
response.setBody(testFile);
response.readyToSend(this);
String responseString = output.toString();
assertSubString("Content-Length: 1000000000", responseString);
assertTrue(bytesSent > 1000000000);
}

Al giorno d’oggi, naturalmente, avremmo disattivato il caso di test usando l’attributo @Ignore con una stringa descrittiva appropriata: @Ignore("Takes too long to run"). Ma prima di JUnit 4, si era soliti porre un underscore davanti al nome del metodo. Il commento, pur vago, chiarisce la cosa.

Ecco un altro esempio, più pregnante:

public static SimpleDateFormat makeStandardHttpDateFormat()
{
//SimpleDateFormat non è thread safe,
//pertanto dobbiamo creare ciascuna istanza in modo indipendente.
SimpleDateFormat df = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z");
df.setTimeZone(TimeZone.getTimeZone("GMT"));
return df;
}

Potreste obiettare che vi sono modi migliori per risolvere questo problema. Io potrei concordare con voi. Ma il commento, così come è, è perfettamente ragionevole. Ha lo scopo di impedire a qualche programmatore di usare un inizializzatore statico nel nome dell’efficienza.

Commenti TODO

Talvolta è ragionevole lasciare degli appunti //TODO per le parti da finire. Nel caso seguente, il commento TODO spiega perché la funzione ha questa implementazione inconsistente e come dovrebbe essere la versione finale della funzione.

// TODO-MdM cose non necessarie
// Ci aspettiamo che sparisca nel modello finale
protected VersionInfo makeVersion() throws Exception {
return null;
}

I “TODO” sono operazioni che il programmatore pensa che dovrebbero essere fatte, ma che, per qualche motivo, al momento sono in sospeso. Può trattarsi di una nota per cancellare una funzionalità indesiderata o della richiesta a qualcun altro affinché consideri un problema. Potrebbe essere una richiesta affinché qualcun altro pensi a un nome migliore o a una nota che ricordi di fare una modifica che dipende da un evento già pianificato. Qualsiasi sia il significato del “todo”, non è una scusa per lasciare del cattivo codice nel sistema.

Al giorno d’oggi, la maggior parte degli IDE offre particolari simboli e funzionalità per individuare tutti i commenti TODO, pertanto è più difficile perderseli. Ciononostante, evitate che il vostro codice sia pieno di “TODO”. Controllateli regolarmente ed eliminate quelli che potete.

Amplificazione

Un commento può essere usato per amplificare l’importanza di qualcosa che altrimenti potrebbe sembrare illogico.

String listItemContent = match.group(3).trim();
// Il trim qui è importante. Elimina gli spazi
// iniziali che potrebbero fare in modo che l'item
// venga riconosciuto come un'altra lista.
new ListItemWidget(this, listItemContent, this.level + 1);
return buildList(text.substring(match.end()));

Javadoc nelle API pubbliche

Non vi è nulla di altrettanto utile e gradevole di un’API pubblica ben documentata. I javadoc della libreria standard Java ne sono un esempio. Sarebbe difficile, a dire poco, scrivere programmi Java senza di essi. Se state scrivendo un’API pubblica, certamente dovrete anche scrivere dei buoni javadoc per essa. Ma tenete in considerazione anche l’altro consiglio di questo capitolo. I javadoc possono essere altrettanto fuorvianti, fuori posto e disonesti di ogni altro genere di commenti.

Cattivi commenti

La maggior parte dei commenti rientra in questa categoria. Solitamente sono solo pretesti per programmare malamente o giustificazioni di decisioni scadenti, un po’ come se il programmatore parlasse a se stesso.

Pensieri

Scrivere un commento solo perché sentite di doverlo fare o perché il processo lo richiede, è del tutto inutile. Se decidete di scrivere un commento, dedicategli il tempo necessario per assicurarvi che sia il miglior commento che possiate scrivere.

Qui, per esempio, riporto un caso che ho trovato in FitNesse, dove un commento poteva anche essere utile. Ma l’autore aveva fretta o forse non vi ha dedicato particolare attenzione. I suoi pensieri generano un enigma:

public void loadProperties()
{
try
{
String propertiesPath = propertiesLocation + "/" + PROPERTIES_FILE;
FileInputStream propertiesStream = new FileInputStream(propertiesPath);
loadedProperties.load(propertiesStream);
}
catch(IOException e)
{
// Nessun file di proprietà? Carica i default
}
}

Che cosa significa quel commento nel blocco catch? Chiaramente significava qualcosa per l’autore, ma il significato è piuttosto oscuro. Apparentemente, se otteniamo una IOException, significa che non vi era alcun file delle proprietà; e in tal caso verranno caricati tutti i default. Ma chi carica tutti i default? Sono già stati caricati prima della chiamata a loadProperties.load? O loadProperties.load raccoglie l’eccezione, carica i default e poi passa avanti l’eccezione, che dobbiamo ignorare? O loadProperties.load carica tutti i default prima di tentare di caricare il file? L’autore voleva solo rassicurarsi sul fatto che lasciava vuoto il blocco catch? O (e anche questa è una terribile possibilità) l’autore tentava di dirsi di tornare a lavorare su questa parte per scrivere il codice che doveva caricare i default? Saremo quindi costretti a esaminare il codice in altre parti del sistema per scoprire che cosa accade.

Ogni commento che vi costringe a cercare in un altro modulo il significato di quel commento fallisce nel suo tentativo di comunicare con voi e non vale nemmeno i bit che consuma.

Commenti ridondanti

Il Listato 4.1 presenta una semplice funzione con un commento iniziale completamente ridondante. Probabilmente ci vuole più tempo a leggere il commento che a leggere e comprendere il codice.

Listato 4.1 waitForClose.

// Metodo di servizio che termina se this.closed è true.
// Lancia un'eccezione se viene raggiunto il timeout.
public synchronized void waitForClose(final long timeoutMillis) throws Exception {
if(!closed)
{
wait(timeoutMillis);
if(!closed)
throw new Exception("MockResponseSender could not be closed");
}
}

Quale scopo ha questo commento? Certamente non è più informativo rispetto al codice. Non spiega il codice, né fornisce l’intento o la motivazione. Non è più facile da leggere rispetto al codice. In effetti, è anche meno preciso rispetto al codice e spinge chi legge ad accettare tale mancanza di precisione senza comprendere il vero significato del codice. È un po’ come se un affabile venditore di auto usate vi garantisse che non avete alcun bisogno di guardare sotto il cofano motore.

Ora considerate la gran quantità di javadoc inutili e ridondanti presentati nel Listato 4.2, tratto da Tomcat. Questi commenti servono solo a congestionare e nascondere il codice. Non hanno alcuno scopo documentale. A peggiorare le cose, vi ho mostrato solo i primi… ma ve ne sono parecchi in questo modulo.

Listato 4.2 ContainerBase.java (Tomcat).

public abstract class ContainerBase
implements Container, Lifecycle, Pipeline,
MBeanRegistration, Serializable {
⁄**
* The processor delay for this component.
*/
protected int backgroundProcessorDelay = -1;
⁄**
* The lifecycle event support for this component.
*/
protected LifecycleSupport lifecycle = new LifecycleSupport(this);
⁄**
* The container event listeners for this Container.
*/
protected ArrayList listeners = new ArrayList();
⁄**
* The Loader implementation with which this Container is
* associated.
*/
protected Loader loader = null;
⁄**
* The Logger implementation with which this Container is
* associated.
*/
protected Log logger = null;
⁄**
* Associated logger name.
*/
protected String logName = null;
⁄**
* The Manager implementation with which this Container is
* associated.
*/
protected Manager manager = null;
⁄**
* The cluster with which this Container is associated.
*/
protected Cluster cluster = null;
⁄**
* The human-readable name of this Container.
*/
protected String name = null;
⁄**
* The parent Container to which this Container is a child.
*/
protected Container parent = null;
⁄**
* The parent class loader to be configured when we install a
* Loader.
*/
protected ClassLoader parentClassLoader = null;
⁄**
* The Pipeline object with which this Container is
* associated.
*/
protected Pipeline pipeline = new StandardPipeline(this);
⁄**
* The Realm with which this Container is associated.
*/
protected Realm realm = null;
⁄**
* The resources DirContext object with which this Container
* is associated.
*/
  protected DirContext resources = null;

Commenti fuorvianti

Talvolta, magari con tutte le migliori intenzioni, un programmatore fa un’affermazione nei propri commenti che è tutt’altro che accurata. Provate a tornare al ridondante, ma anche fuorviante, commento che abbiamo visto nel Listato 4.1.

In che senso il commento è fuorviante? Il metodo non esce quando this.closed diviene true. Esce se this.closed è true; in caso contrario, attende un certo time-out e poi lancia un’eccezione se this.closed non è ancora true.

Questa subdola imprecisione, inserita in un commento che è più difficile da leggere del corpo del codice, può far sì che un altro programmatore richiami inavvertitamente questa funzione aspettandosi che termini non appena this.closed diviene true. Il povero programmatore dovrà così svolgere una sessione di debug nel tentativo di scoprire perché il suo codice è così lento.

Commenti obbligati

Non è molto intelligente imporre una regola che dice che ogni funzione deve avere un javadoc o ogni variable deve avere un commento. Commenti come questo non fanno altro che congestionare il codice, propagare cose non vere e introdurre confusione e disorganizzazione.

Per esempio, l’imposizione di un javadoc per ogni funzione genera abomini come quello rappresentato nel Listato 4.3. Questa “roba” non aggiunge nulla e serve solo a offuscare il codice e a creare le condizioni per l’insorgere di falsità.

Listato 4.3

⁄**
*
* @param title Il titolo del CD
* @param author L'autore del CD
* @param tracks Il numero di tracce del CD
* @param durationInMinutes La durata del CD in minuti
*/
public void addCD(String title, String author,
int tracks, int durationInMinutes) {
CD cd = new CD();
cd.title = title;
cd.author = author;
cd.tracks = tracks;
cd.duration = duration;
cdList.add(cd);
}

Commenti a “log”

Talvolta i programmatori aggiungono un commento all’inizio di un modulo ogni volta che lo modificano. Questi commenti si accumulano come in un log, e registrano ogni modifica svolta. Mi è capitato di vedere moduli con decine di pagine di queste righe di log.

* Modifiche (dal 11-Ott-2001)
* --------------------------
* 11-Ott-2001 : Riorganizzato la classe e spostata in un nuovo package
com.jrefinery.date (DG);
* 05-Nov-2001 : Aggiunto un metodo getDescription() ed eliminata
la classe NotableDate;
* 12-Nov-2001 : IBD richiede il metodo setDescription(), ora che la classe
NotableDate non c'è più (DG); Modificate getPreviousDayOfWeek(),
getFollowingDayOfWeek() e getNearestDayOfWeek() per correggere
dei bug (DG);
* 05-Dic-2001 : Corretto bug nella classe SpreadsheetDate (DG);
* 29-Mag-2002 : Spostate le costanti dei mesi in un'interfaccia a parte
(MonthConstants) (DG);
* 27-Ago-2002 : Corretto bug nel metodo addMonths(), grazie a N???levka Petr (DG);
* 03-Ott-2002 : Corretti errori rilevati da Checkstyle (DG);
* 13-Mar-2003 : Implementato Serializable (DG);
* 29-Mag-2003 : Corretto bug nel metodo addMonths (DG);
* 04-Set-2003 : Implementato Comparable. Aggiornati i javadoc isInRange (DG);
* 05-Gen-2005 : Corretto bug nel metodo addYears() (1096282) (DG);

Molto tempo fa vi era un buon motivo per creare e gestire questi log all’inizio di ogni modulo. Non avevamo i sistemi di controllo del codice sorgente di oggi. Al giorno d’oggi, tuttavia, questi log non fanno altro che offuscare il modulo e quindi dovrebbero essere completamente rimossi.

Puro “rumore”

Talvolta si trovano commenti che non servono proprio a nulla. Ribadiscono l’ovvio e non forniscono alcuna informazione.

⁄**
* Costruttore di default.
*/
protected AnnualDateRule() {

Ma no?!?! Davvero?

E che dire di questo:

⁄** Il giorno del mese. */ 
private int dayOfMonth;

E poi vi è anche la ridondanza dell’inutilità:

⁄**
* Restituisce il giorno del mese.
*
* @return il giorno del mese.
*/
public int getDayOfMonth() {
return dayOfMonth;
}

Questi commenti sono talmente inutili che presto impariamo a ignorarli. Mentre leggiamo il codice, i nostri occhi semplicemente li saltano. E poi i commenti iniziano a mentire quando il codice attorno a loro cambia.

Il primo commento del Listato 4.4 sembra appropriato (l’attuale tendenza degli IDE di controllare l’ortografia nei commenti sarà un vero balsamo per tutti coloro che leggono molto codice). Spiega perché il blocco catch è stato ignorato. Ma il secondo commento è puro rumore. Apparentemente il programmatore era talmente frustrato dai blocchi try/catch di questa funzione che aveva bisogno di un po’ d’aria.

Listato 4.4 startSending.

private void startSending()
{
try
{
doSending();
}
catch(SocketException e)
{
// normale. qualcuno ha bloccato la richiesta.
}
catch(Exception e)
{
try
{
response.add(ErrorResponder.makeExceptionString(e));
response.closeAll();
}
catch(Exception e1)
{
//Basta, vi prego!
}
}
}

Invece di scrivere un inutile e liberatorio commento, il programmatore avrebbe dovuto riconoscere che la sua frustrazione si sarebbe potuta risolvere migliorando la struttura del suo codice. Avrebbe dovuto indirizzare le sue energie nell’estrazione dell’ultimo blocco try/catch in una funzione distinta, come si può vedere nel Listato 4.5.

Listato 4.5 startSending (dopo un refactoring).

private void startSending()
{
try
{
doSending();
}
catch(SocketException e)
{
// normale. qualcuno ha bloccato la richiesta.
}
catch(Exception e)
{
addExceptionAndCloseResponse(e);
}
}
private void addExceptionAndCloseResponse(Exception e)
{
try
{
response.add(ErrorResponder.makeExceptionString(e));
response.closeAll();
}
catch(Exception e1)
{
}
}

Resistite alla tentazione di creare puro “rumore” con la determinazione nel ripulire il vostro codice. Diventerete programmatori migliori e anche più felici.

Assoluto “rumore”

Anche i javadoc possono essere fastidiosamente “rumorosi”. Quale scopo hanno i seguenti javadoc (tratti da una nota libreria open-source)? Risposta: nessuno. Sono solo commenti ridondanti e rumorosi scritti con lo scopo di fornire chissà quale documentazione.

⁄** Il nome. */ 
private String name;
⁄** La versione. */
private String version;
⁄** Il licenceName. */
private String licenceName;
⁄** La versione. */
private String info;

Leggete questi commenti con attenzione. Lo vedete l’errore copia-incolla? Se gli autori non dedicano la necessaria attenzione ai commenti mentre li scrivono (o incollano), perché mai coloro che leggono il codice dovrebbero considerarli?

Non usate un commento al posto di una funzione o una variabile

Considerate il seguente frammento di codice:

// il modulo della lista globale <mod> dipende 
// dal sottosistema?
if (smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))

Si potrebbe riscrivere senza commento come:

ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees.contains(ourSubSystem))

L’autore del codice potrebbe aver scritto prima il commento (improbabile) e poi aver scritto il codice in base al commento. Tuttavia, l’autore avrebbe dovuto eseguire il refactoring del codice, nel modo descritto, così da rimuovere il commento.

Contrassegni di posizione

Talvolta i programmatori amano contrassegnare una determinata posizione in un file di codice sorgente. Per esempio, recentemente ho trovato questa cosa in un programma che esaminavo:

// Actions //////////////////////////////////

Vi sono rare situazioni in cui è sensato raccogliere determinate funzioni sotto un banner come questo. Ma in generale queste cose dovrebbero essere eliminate, in particolare la lunga sequenza di slash alla fine.

Pensatela in questo modo. Un banner è evidente e ovvio solo se non si ripresenta troppo spesso. Pertanto usateli con molta parsimonia e solo quando il vantaggio è significativo. Se eccederete con l’uso, finiranno nel rumore di fondo e verranno ignorati.

Commenti per le parentesi graffe chiuse

Talvolta i programmatori inseriscono dei commenti alle parentesi graffe chiuse, come nel Listato 4.6. Questo può avere senso nelle funzioni molto lunghe con strutture profondamente annidate, ma non servono a nulla nelle piccole funzioni incapsulate che prediligiamo. Pertanto, se pensate di dover commentare le parentesi graffe chiuse, cercate piuttosto di accorciare le vostre funzioni.

Listato 4.6 wc.java.

public class wc {
public static void main(String[] args) {
BufferedReader in = new BufferedReader(new InputStreamReader(System.in));
String line;
int lineCount = 0;
int charCount = 0;
int wordCount = 0;
try {
while ((line = in.readLine()) != null) {
lineCount++;
charCount += line.length();
String words[] = line.split("\\W");
wordCount += words.length;
} //while
System.out.println("wordCount = " + wordCount);
System.out.println("lineCount = " + lineCount);
System.out.println("charCount = " + charCount);
} // try
catch (IOException e) {
System.err.println("Error:" + e.getMessage());
} //catch
} //main
}

Attribuzioni

⁄* Aggiunto da Rick */

I sistemi di controllo del codice sorgente sono precisissimi nel ricordare chi ha aggiunto cosa e quando. Non vi è alcuna necessità di inquinare il codice con queste cose. Potreste pensare che tali commenti possano essere utili per aiutare gli altri a sapere a chi chiedere informazioni sul codice. Ma la realtà è che questi commenti tendono a rimanere in giro per anni e anni, divenendo sempre meno accurati e rilevanti.

Di nuovo, il sistema di controllo del codice sorgente è un luogo migliore per questo genere di informazioni.

Codice commentato

Poche abitudini sono altrettanto fastisione del porre il codice in commenti. Non fatelo!

    InputStreamResponse response = new InputStreamResponse();
response.setBody(formatter.getResultStream(), formatter.getByteCount());
// InputStream resultsStream = formatter.getResultStream();
// StreamReader reader = new StreamReader(resultsStream);
// response.setContent(reader.read(formatter.getByteCount()));

Coloro che vedranno quel codice commentato non avranno il coraggio di cancellarlo. Penseranno che sia lì per un motivo e che sia troppo importante per cancellarlo. Pertanto il codice commentato finisce per accumularsi come il fondo in una pessima bottiglia di vino.

Considerate il seguente esempio tratto da apache commons:

this.bytePos = writeBytes(pngIdBytes, 0);
//hdrPos = bytePos;
writeHeader();
writeResolution();
//dataPos = bytePos;
if (writeImageData()) {
writeEnd();
this.pngBytes = resizeByteArray(this.pngBytes, this.maxPos);
} else {
this.pngBytes = null;
}
return this.pngBytes;

Perché queste due righe di codice sono commentate? Sono importanti? Sono state lasciate lì come annotazione per una modifica imminente? O sono solo residui che qualcuno ha commentato anni fa e, semplicemente, non si è preoccupato di eliminare?

Un tempo, parliamo degli anni Sessanta, il fatto di trasformare in commenti il codice poteva essere utile. Ma oggi contiamo su ottimi sistemi di controllo del codice sorgente. Tali sistemi possono ricordarci il codice impiegato. Non abbiamo più bisogno di usare questi tipi di commenti. Cancellate il codice. Non lo perderete. Fidatevi.

Commenti HTML

La presenza di codice HTML nei commenti al codice sorgente è un abominio, come potete sincerarvi leggendo il codice sottostante. Complica la leggibilità dei commenti nel posto in cui dovrebbero essere più facili da leggere: l’editor/IDE. Se i commenti dovranno essere estratti da qualche strumento (come Javadoc) per comparire in una pagina web, allora l’aggiunta del codice HTML rientra nelle responsabilità di tale strumento, non del programmatore.

⁄**
* Task per i test di correttezza.
* Questo task esegue dei test di Fitnesse e pubblica i risultati.
* <p/>
* <pre>
* Uso:
* &lt;taskdef name=&quot;execute-fitnesse-tests&quot;
* classname=&quot;fitnesse.ant.ExecuteFitnesseTestsTask&quot;
* classpathref=&quot;classpath&quot; /&gt;
* OPPURE
* &lt;taskdef classpathref=&quot;classpath&quot;
* resource=&quot;tasks.properties&quot; /&gt;
* <p/>
* &lt;execute-fitnesse-tests
* suitepage=&quot;FitNesse.SuiteAcceptanceTests&quot;
* fitnesseport=&quot;8082&quot;
* resultsdir=&quot;${results.dir}&quot;
* resultshtmlpage=&quot;fit-results.html&quot;
* classpathref=&quot;classpath&quot; /&gt;
* </pre>
*/

Informazioni fuori posizione

Se dovete scrivere un commento, allora assicuratevi che descriva il codice circostante. Non offrite informazioni di sistema nel contesto di un commento locale. Considerate, per esempio, il commento javadoc sottostante. A parte il fatto che è del tutto ridondante, offre anche informazioni sulla porta di default. E comunque la funzione non ha assolutamente alcun controllo su tale default. Il commento non sta descrivendo la funzione, ma una qualche altra parte del sistema, molto molto lontana. Naturalmente non vi è alcuna garanzia che questo commento verrà modificato quando cambierà il codice che contiene il default.

⁄**
* Porta utilizzata da fitnesse. Default = <b>8082</b>.
*
* @param fitnessePort
*/
public void setFitnessePort(int fitnessePort)
{
this.fitnessePort = fitnessePort;
}

Eccesso di informazioni

Non infilate nei commenti i dettagli di interessanti discussioni storiche o di descrizioni irrilevanti. Il commento sottostante è stato estratto da un modulo progettato per verificare che una funzione potesse eseguire la codifica e decodifica base64. A parte il numero di RFC, chi leggerà questo codice non ha alcun bisogno delle esotiche informazioni contenute nel commento.

⁄*
RFC 2045 - Multipurpose Internet Mail Estensioni (MIME)
Parte 1: Formato degli Internet Message Bodies
sezione 6.8. Base64 Content-Transfer-Encoding
Il processo di codifica rappresenta i gruppi di bit di input (24 bit) come
stringhe di output di quattro caratteri codificati. Procedendo da sinistra a
destra, un gruppo di input da 24 bit viene formato concatenando tre gruppi di
input a 8 bit.
Questi 24 bit vengono poi trattati come quattro gruppi concatenati da 6 bit,
ognuno dei quali viene tradotto in un'unica cifra nell'alfabeto base64.
Nella codifica base64 di uno stream di bit, tale stream si presume ordinato
a partire dal bit più significativo.
In pratica, il primo bit dello stream sarà nel bit di ordine più elevato nel
primo byte di 8 bit e l'ottavo bit sarà nel bit di ordine inferiore del primo
byte di 8 bit e così via.
*/

Scarso legame con il codice

La connessione fra il commento e il codice che descrive dovrebbe essere evidente. Se vi date il disturbo di scrivere un commento, quanto vorreste che chi legge sia in grado di vedere il commento e il codice e di capire il motivo del commento.

Considerate, per esempio, questo commento tratto da apache commons:

⁄*
* iniziate con un array di dimensioni sufficienti per contenere tutti i pixel
* (più i byte del filtro) e altri 200 byte per l'header
*/
this.pngBytes = new byte[((this.width + 1) * this.height * 3) + 200];

Che cos’è un byte del filtro? Ha a che fare con il +1? O con il * 3? O con entrambi? Un pixel è di un byte? Perché 200? Lo scopo di un commento è quello di spiegare il codice che non si spiega da solo. È un peccato quando un commento richiede… spiegazioni.

Intestazioni di funzioni

Le funzioni brevi non hanno bisogno di troppe descrizioni. Un nome “parlante” per una piccola funzione che faccia una cosa sola è solitamente meglio di un commento di intestazione.

Javadoc nel codice non pubblico

Per quanto possano essere utili i javadoc per le API pubbliche, essi rappresentano un anatema per il codice che non è rivolto all’uso pubblico. Produrre pagine javadoc per classi e funzioni interne di un sistema non è utile a nessuno e la maggiore formalità dei commenti javadoc è di valore sostanzialmente nullo.

Esempio

Ho scritto il modulo presentato nel Listato 4.7 per il primo XP Immersion. Aveva lo scopo di rappresentare un cattivo esempio di programmazione e scelta dei commenti. Kent Beck ha poi eseguito il refactoring di questo codice in una forma molto più sensata davanti ad alcune decine di allievi entusiasti. Successivamente ho adattato l’esempio per il mio libro Agile Software Development, Principles, Patterns, and Practices e il primo dei miei articoli Craftsman pubblicati sul “Software Development Magazine”.

Ciò che trovo affascinante di questo modulo è che vi fu un tempo in cui molti di noi l’avrebbero considerato “ben documentato”. Oggi lo consideriamo negativamente. Provate a vedere quanti problemi riuscite a individuarvi.

Listato 4.7 GeneratePrimes.java.

⁄**
* Questa classe genera dei numeri primi fino a un massimo specificato
* dall'utente. L'algoritmo usato è il Setaccio di Eratostene.
* <p>
* Eratosthenes di Cirene, a. c. 276 AC, Cirene, Libia --
* d. c. 194, Alessandria. Il primo uomo a calcolare la
* circonferenza della terra. Noto anche per aver lavorato sui
* calendari con anni bisestili e aver gestito la libreria di Alessandria.
* <p>
* L'algoritmo è semplice. Dato un array di interi
* che partono da 2. Elimina tutti i multipli di 2. Trova il
* prossimo intero disponibile ed elimina tutti i suoi multipli.
* Ripeti fino ad aver superato la radice quadrata del valore
* massimo.
*
* @author Alphonse
* @version 13 Feb 2002 atp
*/
import java.util.*;
public class GeneratePrimes
{
⁄**
* @param maxValue è il limite della generazione.
*/
public static int[] generatePrimes(int maxValue)
{
if (maxValue >= 2) // l’unico caso valido
{
// dichiarazioni
int s = maxValue + 1; // dimensioni array
boolean[] f = new boolean[s];
int i;
// inizializza l'array a true.
for (i = 0; i < s; i++)
f[i] = true;
// elimina i non-primi
f[0] = f[1] = false;
// setaccio
int j;
for (i = 2; i < Math.sqrt(s) + 1; i++)
{
if (f[i]) // se i è primo, elimina i suoi multipli.
{
for (j = 2 * i; j < s; j += i)
f[j] = false; // un multiplo non può essere primo
}
}
// quanti primi ci sono?
int count = 0;
for (i = 0; i < s; i++)
{
if (f[i]) count++; // conteggio.
}
int[] primes = new int[count];
// sposta i primi nel risultato
for (i = 0, j = 0; i < s; i++)
{
if (f[i]) // se è primo
primes[j++] = i;
}
return primes; // restituisci i primi
}
else // maxValue < 2
return new int[0]; // restituisce un array nullo se l'input è errato.
}
}

Nel Listato 4.8 trovate una versione dello stesso modulo dopo un refactoring. Notate che l’uso dei commenti è molto limitato. Vi sono solo due commenti nell’intero modulo. Entrambi sono descrittivi.

Listato 4.8 PrimeGenerator.java (dopo un refactoring).

⁄**
* Questa classe Genera dei numeri primi fino a un massimo specificato
* dall'utente. L'algoritmo usato è il Setaccio di Eratostene.
* Dato un array di interi a partire da 2:
* trova il primo intero non eliminato ed elimina tutti i suoi
* multipli. Ripeti finché non vi sono più multipli
* nell'array.
*/
public class PrimeGenerator {
private static boolean[] crossedOut;
private static int[] result;
public static int[] generatePrimes(int maxValue)
{
if (maxValue < 2)
return new int[0];
else
{
uncrossIntegersUpTo(maxValue);
crossOutMultiples();
putUncrossedIntegersIntoResult();
return result;
}
}
private static void uncrossIntegersUpTo(int maxValue)
{
crossedOut = new boolean[maxValue + 1];
for (int i = 2; i < crossedOut.length; i++)
crossedOut[i] = false;
}
private static void crossOutMultiples()
{
int limit = determineIterationLimit();
for (int i = 2; i <= limit; i++)
if (notCrossed(i))
crossOutMultiplesOf(i);
}
private static int determineIterationLimit()
{
// Ogni multiplo nell'array ha un fattore primo che è minore
// o uguale alla radice delle dimensioni dell'array,
// così che possiamo evitare di eliminare i multipli
// dei numeri maggiori di tale radice.
double iterationLimit = Math.sqrt(crossedOut.length);
return (int) iterationLimit;
}
private static void crossOutMultiplesOf(int i)
{
for (int multiple = 2*i;
multiple < crossedOut.length;
multiple += i)
crossedOut[multiple] = true;
}
private static boolean notCrossed(int i)
{
return crossedOut[i] == false;
}
private static void putUncrossedIntegersIntoResult()
{
result = new int[numberOfUncrossedIntegers()];
for (int j = 0, i = 2; i < crossedOut.length; i++)
if (notCrossed(i))
result[j++] = i;
}
private static int numberOfUncrossedIntegers()
{
int count = 0;
for (int i = 2; i < crossedOut.length; i++)
if (notCrossed(i))
count++;
return count;
}
}

È facile notare che il primo commento è ridondante, perché è un po’ come quello della funzione generatePrimes. Ciononostante, penso che il commento sia utile per facilitare chi legge l’algoritmo, quindi ho preferito lasciarlo.

Il secondo argomento è quasi certamente necessario. Spiega l’uso della radice quadrata come limite del ciclo. Non sono riuscito a trovare alcun nome di variabile o struttura del programma che chiarisse a sufficienza le cose. D’altra parte, l’uso della radice quadrata pone dei dubbi. Risparmio davvero del tempo limitando l’iterazione alla radice quadrata? Non è che il calcolo della radice quadrata richiede più tempo di quanto ne faccia risparmiare?

Vale la pena di rifletterci. L’uso della radice quadrata come limite dell’iterazione soddisfa il vecchio programmatore C e Assembly che abita in me, ma non sono sicuro che valga il tempo e l’impegno che chiedo agli altri per comprenderne la motivazione.

Bibliografia