Pulisci i commenti del codice e la documentazione della classe

Come altri hanno già detto, c'è una differenza tra commenti di documentazione API e commenti in linea. Dal mio punto di vista, la differenza principale è che un commento in-line viene letto insieme al codice , mentre un commento della documentazione viene letto insieme alla firma di qualunque cosa si sta commentando.

Dato questo, possiamo applicare lo stesso principio DRY . Il commento dice la stessa cosa della firma? Diamo un'occhiata al tuo esempio:

Retrieves a product by its id

Questa parte ripete solo ciò che vediamo già dal nome GetById più il tipo di ritorno Product . Solleva anche la domanda su quale sia la differenza tra "ottenere" e "recuperare" e quale sia il codice che porta il commento rispetto a tale distinzione. Quindi è inutile e leggermente confuso. Semmai, sta intralciando la seconda parte del commento effettivamente utile:

returns null if no product was found.

Ah! Questo è qualcosa che sicuramente non possiamo sapere con certezza solo dalla firma e fornisce informazioni utili.

Ora fai un ulteriore passo avanti. Quando la gente parla di commenti come odori di codice, la domanda non è se il codice come è abbia bisogno di un commento, ma se il commento indica che il codice potrebbe essere scritto meglio, per esprimere le informazioni nel commento. Questo è ciò che significa "odore di codice" - non significa "non farlo!", Significa "se lo fai, potrebbe essere un segno che c'è un problema".

Quindi, se i tuoi colleghi ti dicono che questo commento su null è un odore di codice, dovresti semplicemente chiedere loro: "Okay, come dovrei esprimere questo allora?" Se hanno una risposta fattibile, hai imparato qualcosa. In caso contrario, probabilmente ucciderà i loro reclami morti.

Riguardo a questo caso specifico, in genere il problema nullo è ben noto per essere difficile. C'è una ragione per cui le basi di codice sono disseminate di clausole di salvaguardia, perché i controlli nulli sono una condizione preliminare popolare per i contratti di codice, perché l'esistenza di null è stata definita un "errore da un miliardo di dollari". Non ci sono molte opzioni valide. Uno dei più popolari, tuttavia, trovato in C # è la convenzione Try... :

public bool TryGetById(int productId, out Product product);

In altre lingue, potrebbe essere idiota usare un tipo (spesso chiamato qualcosa come Optional o Maybe ) per indicare un risultato che può essere o non essere lì:

public Optional<Product> GetById(int productId);

Quindi, in un certo senso, questa posizione anti-commento ci ha portato da qualche parte: abbiamo almeno pensato se questo commento rappresenta un odore e quali alternative potrebbero esistere per noi.

Se dovremmo in realtà preferire questi rispetto alla firma originale è un altro dibattito, ma abbiamo almeno delle opzioni per esprimere attraverso il codice piuttosto che i commenti cosa succede quando non viene trovato alcun prodotto. Dovresti discutere con i tuoi colleghi quali di queste opzioni ritengono sia meglio e perché, e speriamo che aiuti a superare le dichiarazioni dogmatiche generali sui commenti.

La citazione di Robert C. Martin è fuori dal contesto. Ecco la citazione con un po 'più di contesto:

Nothing can be quite so helpful as a well-placed comment. Nothing can clutter up a module more than frivolous dogmatic comments. Nothing can be quite so damaging as an old crufty comment that propagates lies and misinformation.

Comments are not like Schindler's List. They are not "pure good." Indeed, comments are, at best, a necessary evil. If our programming languages were expressive enough, or if we had the talent to subtly wield those languages to express our intent, we would not need comments very much -- perhaps not at all.

The proper use of comments is to compensate for our failure to express ourself in code. Note that I used the word failure. I meant it. Comments are always failures. We must have them because we cannot always figure out how to express ourselves without them, but their use is not a cause for celebration.

So when you find yourself in a position where you need to write a comment, think it through and see whether there isn't some way to turn the tables and express yourself in code. Every time you express yourself in code, you should pat yourself on the back. Every time you write a comment, you should grimace and feel the failure of your ability of expression.

(Copiato da qui , ma il la citazione originale proviene da Pulisci codice: un manuale di abilità software agile )

Come questa citazione è ridotta in "I commenti sono sempre falliti" è un buon esempio di come alcune persone prenderanno una citazione sensata fuori dal contesto e trasformandola in stupido dogma.

La documentazione dell'API (come javadoc) dovrebbe documentare l'API in modo che l'utente possa usarlo senza dover leggere il codice sorgente . Quindi in questo caso la documentazione dovrebbe spiegare che cosa fa il metodo. Ora si può affermare che "Recupera un prodotto in base al suo id" è ridondante perché è già indicato dal nome del metodo, ma l'informazione che null può essere restituita è sicuramente importante per documentare, poiché ciò non è in alcun modo ovvio.

Se vuoi evitare la necessità del commento, devi rimuovere il problema sottostante (che è l'uso di null come valore di ritorno valido), rendendo l'API più esplicita. Ad esempio, potresti restituire un tipo di tipo Option<Product> , quindi la firma del tipo comunica chiaramente ciò che verrà restituito nel caso in cui il prodotto non venga trovato.

Ma in ogni caso non è realistico documentare completamente un'API solo attraverso i nomi dei metodi e le firme dei tipi. Utilizza i commenti del doc per eventuali informazioni aggiuntive non ovvie che l'utente dovrebbe conoscere. Dì la documentazione dell'API da DateTime.AddMonths() nel BCL:

The AddMonths method calculates the resulting month and year, taking into account leap years and the number of days in a month, then adjusts the day part of the resulting DateTime object. If the resulting day is not a valid day in the resulting month, the last valid day of the resulting month is used. For example, March 31st + 1 month = April 30th. The time-of-day part of the resulting DateTime object remains the same as this instance.

Non è possibile esprimere ciò utilizzando solo il nome e la firma del metodo! Naturalmente la tua documentazione di classe potrebbe non richiedere questo livello di dettaglio, è solo un esempio.

I commenti in linea non sono male neanche.

non validi sono negativi. Per esempio commenti che spiegano solo cosa può essere visto banalmente dal codice, il classico esempio è:

// increment x by one
x++;

I commenti che spiegano qualcosa che potrebbe essere chiarito rinominando una variabile o un metodo o altrimenti ristrutturando il codice, è un odore di codice:

// data1 is the collection of tasks which failed during execution
var data1 = getData1();

Questi sono il tipo di commenti contro cui Martin si oppone. Il commento è un sintomo di una mancata scrittura del codice chiaro - in questo caso per usare nomi autoesplicativi per variabili e metodi. Il commento in sé non è ovviamente il problema, il problema è che abbiamo bisogno del commento per capire il codice.

Tuttavia i commenti dovrebbero essere usati per spiegare tutto ciò che non è ovvio dal codice, ad es. perché il codice è scritto in un modo non ovvio:

// need to reset foo before calling bar due to a bug in the foo component.
foo.reset()
foo.bar();

Commenti che spiegano cos'è un pezzo di codice eccessivamente contorto, ma la correzione non è di mettere fuori legge i commenti, la correzione è la correzione del codice! Nella realtà, il codice contorto si verifica (si spera solo temporaneamente fino a un refactoring) ma nessuno sviluppatore ordinario scrive il codice pulito perfetto la prima volta. Quando si verifica un codice convoluto, è molto meglio scrivere un commento che spieghi cosa fa che non scrivere un commento. Questo commento renderà più semplice il refactoring più tardi.

A volte il codice è inevitabilmente complesso. Potrebbe essere un algoritmo complicato, o potrebbe essere un codice che sacrifica la chiarezza per motivi di prestazioni. Di nuovo i commenti sono necessari.

C'è una differenza tra commentare il tuo codice e documentare il tuo codice .

• Commenti sono necessari per mantenere il codice in un secondo momento, ovvero modificare il codice stesso.

  I commenti possono infatti essere percepiti come problematici. Il punto estremo sarebbe dire che sempre indicano un problema, sia all'interno del codice (codice troppo difficile da capire) o all'interno della lingua (il linguaggio non è sufficientemente espressivo, ad esempio il fatto che il metodo non restituisce mai null potrebbe essere espresso tramite i contratti di codice in C #, ma non c'è modo di esprimerlo tramite codice, per esempio, in PHP).

• La documentazione è necessaria per poter utilizzare gli oggetti (classi, interfacce) che hai sviluppato. Il pubblico di destinazione è diverso: non sono le persone che manterranno il tuo codice e lo cambieranno di cui stiamo parlando qui, ma le persone che hanno a malapena bisogno di usarlo .

  Rimuovere la documentazione perché il codice è abbastanza chiaro è folle, poiché la documentazione è qui specificatamente per rendere possibile l'uso di classi e interfacce senza dover leggere migliaia di righe di codice .

Bene, sembra che il tuo collega legga libri, prenda in considerazione ciò che dicono e applica ciò che ha appreso senza pensare e senza alcuna considerazione del contesto.

Il tuo commento su ciò che la funzione dovrebbe essere tale da poter buttare via il codice di implementazione, ho letto il commento della funzione, e posso scrivere il sostituto per l'implementazione, senza che qualcosa vada storto.

Se il commento non mi dice se viene lanciata un'eccezione o se viene restituito nulla, non posso farlo. Inoltre, se il commento non dice a tu se viene lanciata un'eccezione o se viene restituito nil, allora ovunque tu chiami la funzione, devi assicurarti che il tuo codice funzioni correttamente se un'eccezione è lanciata o nulla viene restituito.

Quindi il tuo collega ha torto. E vai avanti e leggi tutti i libri, ma pensa tu stesso.

PS. Ho visto la tua linea "a volte fa parte di un'interfaccia, quindi non sai nemmeno quale codice è in esecuzione." Anche solo con le funzioni virtuali, non sai quale codice è in esecuzione. Peggio ancora, se hai scritto una classe astratta, non c'è nemmeno un codice! Quindi se hai una classe astratta con una funzione astratta, i commenti che aggiungi alla funzione astratta sono la cosa solo che un implementatore di una classe concreta deve guidarli. Questi commenti possono anche essere l'unica cosa che può guidare un utente della classe, ad esempio se tutto ciò che si ha è una classe astratta e una fabbrica che restituisce un'implementazione concreta, ma non si vede mai alcun codice sorgente dell'implementazione. (E ovviamente non dovrei guardare il codice sorgente di one implementazione).

Ci sono due tipi di commenti da considerare: quelli visibili alle persone con il codice e quelli che usano per generare documentazione.

Il tipo di commento a cui Zio Bob si riferisce è il tipo che è visibile solo alle persone con il codice. Quello che sta sostenendo è una forma di ASCIUTTA . Per una persona che sta guardando il codice sorgente, il codice sorgente dovrebbe essere la documentazione di cui hanno bisogno. Anche nel caso in cui le persone abbiano accesso al codice sorgente, i commenti non sono sempre negativi. A volte gli algoritmi sono complicati o devi capire perché stai adottando un approccio non ovvio in modo che gli altri non finiscano per violare il codice se provano a correggere un bug o aggiungere una nuova funzionalità.

I commenti che stai descrivendo sono documenti API. Queste sono cose che sono visibili alle persone che usano la tua implementazione, ma che potrebbero non avere accesso al tuo codice sorgente. Anche se hanno accesso al tuo codice sorgente, potrebbero lavorare su altri moduli e non guardare il tuo codice sorgente. Queste persone troverebbero utile avere questa documentazione disponibile nel loro IDE mentre stanno scrivendo il loro codice.

Il valore di un commento è misurato nel valore delle informazioni fornite meno lo sforzo necessario per leggerlo e / o ignorarlo. Quindi se analizziamo il commento

/// <summary>
/// Retrieves a product by its id, returns null if no product was found.
/// </summary>

per valore e costo, vediamo tre cose:

1. Retrieves a product by its id ripete il nome della funzione, quindi costa senza valore. Dovrebbe essere cancellato.

2. returns null if no product was found è un'informazione molto preziosa. Probabilmente riduce i tempi in cui altri programmatori dovranno esaminare l'implementazione della funzione. Sono abbastanza sicuro che si risparmia più lettura rispetto al costo di lettura che si presenta. Dovrebbe rimanere.

3. Le linee

   /// <summary>
   /// </summary>
   

   non portare alcuna informazione. Sono costo puro per il lettore del commento. Possono essere giustificati se il tuo generatore di documentazione ne ha bisogno, ma in tal caso dovresti probabilmente pensare a un altro generatore di documentazione.

   Questo è il motivo per cui l'uso di generatori di documentazione è un'idea discutibile: in genere richiedono molti commenti aggiuntivi che non portano alcuna informazione o ripetono cose ovvie, solo per il piacere di un documento lucido.

Un'osservazione che non ho trovato in nessuna delle altre risposte:

Anche i commenti non necessari per comprendere / utilizzare il codice possono essere molto preziosi. Ecco uno di questi esempi:

//XXX: The obvious way to do this would have been ...
//     However, since we need this functionality primarily for ...
//     doing this the non-obvious way of ...
//     gives us the advantage of ...

Probabilmente un sacco di testo, completamente inutile per capire / utilizzare il codice. Tuttavia, spiega un motivo per cui il codice ha lo stesso aspetto. Ciò impedirà alle persone di dare un'occhiata al codice, si chiederà il motivo per cui non è stato fatto in modo ovvio e inizierà a refactoring del codice fino a quando non si renderanno conto del motivo per cui il codice è stato scritto in questo modo. E anche se il lettore è abbastanza intelligente da non passare direttamente al refactoring, devono ancora capire perché il codice ha l'aspetto che fa prima che si rendano conto che è meglio che rimanga così com'è. Questo commento potrebbe letteralmente risparmiare ore di lavoro. Quindi il valore è più alto del costo.

Allo stesso modo, i commenti possono comunicare le intenzioni di un codice, invece di come funziona. E possono dipingere il quadro generale che di solito si perde nei minimi dettagli del codice stesso. Come tale, hai ragione ad essere a favore dei commenti di classe. Apprezzo di più i commenti di classe se spiegano l'intenzione della classe, come interagisce con altre classi, come deve essere usato, ecc. Ahimè, non sono un grande autore di tali commenti ...

Il codice non modificato è un codice errato. È un mito diffuso (se non universale) che il codice possa essere letto più o meno allo stesso modo, ad esempio, in inglese. Deve essere interpretato, e per qualsiasi ma il più banale codice che richiede tempo e fatica. Inoltre, ognuno ha un diverso livello di capacità sia nella lettura che nella scrittura di una determinata lingua. Le differenze tra l'autore e gli stili e le capacità di codifica del lettore sono forti ostacoli all'interpretazione accurata. È anche un mito che tu possa derivare l'intenzione dell'autore dall'implementazione del codice. Nella mia esperienza, è raramente sbagliato aggiungere commenti extra.

Robert Martin et al. consideralo un "cattivo odore" perché potrebbe essere che il codice sia cambiato e i commenti no. Dico che è una buona cosa (esattamente nello stesso modo in cui un "cattivo odore" viene aggiunto al gas domestico per avvisare l'utente di perdite). Leggere i commenti ti dà un utile punto di partenza per interpretare il codice attuale. Se corrispondono, avrai maggiore fiducia nel codice. Se differiscono, hai rilevato un odore di avvertimento e devi investigare ulteriormente. La cura per un "cattivo odore" non è rimuovere l'odore ma sigillare la perdita.

In alcune lingue (per esempio F #) questo intero commento / documentazione può essere effettivamente espresso nella firma del metodo. Questo perché in F #, null non è generalmente un valore consentito se non specificamente consentito.

Ciò che è comune in F # (e anche in molti altri linguaggi funzionali) è che invece di null si usa un tipo di opzione Option<T> che potrebbe essere None o Some(T) . La lingua quindi capirebbe questo e ti costringerà a (o avvisarti se non lo fai) abbinare su entrambi i casi quando tenti di usarlo.

Quindi ad esempio in F # potresti avere una firma simile a questa

val GetProductById: int -> Option<Product>

Quindi questa sarebbe una funzione che accetta un parametro (un int) e quindi restituisce un prodotto o il valore None.

E quindi potresti usarlo in questo modo

let product = GetProduct 42
match product with
| None -> printfn "No product found!"
| Some p -> DoThingWithProduct p

E si otterrebbero avvisi del compilatore se non si corrispondono entrambi i casi possibili. Quindi non c'è modo di ottenere eccezioni di riferimento null lì (a meno che non si ignorino gli avvertimenti del compilatore, ovviamente) e si conosce tutto ciò che è necessario semplicemente osservando la firma della funzione.

Ovviamente, ciò richiede che la tua lingua sia progettata in questo modo - che molti linguaggi comuni come C #, Java, C ++ ecc. non lo sono. Quindi questo potrebbe non esserti d'aiuto nella tua situazione attuale. Ma è (si spera) bello sapere che ci sono lingue là fuori che ti permettono di esprimere questo tipo di informazioni in modo tipicamente statico senza ricorrere a commenti, ecc.)

Ci sono alcune risposte eccellenti qui e non voglio ripetere ciò che dicono. Ma lasciatemi aggiungere alcuni commenti. (Nessun gioco di parole previsto.)

Ci sono molte affermazioni che le persone intelligenti fanno - sullo sviluppo del software e su molti altri argomenti, suppongo - che siano consigli molto buoni se capiti nel contesto, ma quali persone sciocchezze estraggono dal contesto o si applicano in situazioni inappropriate o prendere per estremi ridicoli.

L'idea che il codice dovrebbe essere autodocumentato è un'idea così eccellente. Ma nella vita reale, ci sono limitazioni sulla praticità di questa idea.

Un problema è che la lingua potrebbe non fornire funzionalità per documentare ciò che deve essere documentato in modo chiaro e conciso. Con il miglioramento delle lingue informatiche, questo diventa sempre meno un problema. Ma non penso che sia andato completamente via. Ai tempi in cui scrivevo in assembler, era molto sensato includere un commento come "prezzo totale = prezzo di articoli non tassabili più il prezzo di articoli tassabili più il prezzo di articoli tassabili * aliquota fiscale". Esattamente ciò che era in un registro in un dato momento non era necessariamente ovvio. Ci sono voluti molti passi per fare una semplice operazione. Ecc. Ma se stai scrivendo in un linguaggio moderno, un commento del genere sarebbe solo una riformulazione di una riga di codice.

Mi infastidisco sempre quando vedo commenti come "x = x + 7; // aggiungi 7 a x". Come, wow, grazie, se avessi dimenticato cosa significa un segno più che potrebbe essere stato molto utile. Dove potrei davvero essere confuso è sapere cosa sia "x" o perché è stato necessario aggiungerne 7 in questo particolare momento. Questo codice potrebbe essere reso auto-documentante dando un nome più significativo a "x" e usando una costante simbolica invece di 7. Come se tu avessi scritto "total_price = total_price + MEMBERSHIP_FEE;", probabilmente non è necessario un commento .

Sembra fantastico dire che il nome di una funzione dovrebbe dirti esattamente cosa fa una funzione in modo che eventuali commenti aggiuntivi siano ridondanti. Ho bei ricordi del tempo in cui ho scritto una funzione che verificava se un numero di articolo fosse nella nostra tabella Item del database, restituendo true o false, e che io chiamassi "ValidateItemNumber". Sembrava un nome decente. Poi arrivò qualcun altro e modificò quella funzione per creare anche un ordine per l'oggetto e aggiornare il database, ma non cambiò mai il nome. Ora il nome era molto fuorviante. Sembrava che facesse una piccola cosa, quando davvero faceva molto di più. Più tardi qualcuno ha anche preso la parte sulla convalida del numero dell'articolo e lo ha fatto da qualche altra parte, ma ancora non ha cambiato il nome. Quindi, anche se ora la funzione non aveva nulla a che fare con la convalida dei numeri di articolo, era ancora chiamata ValidateItemNumber.

Ma in pratica, è spesso impossibile per un nome di funzione descrivere completamente ciò che fa la funzione senza che il nome della funzione sia lungo quanto il codice che compone quella funzione. Il nome ci dirà esattamente quali sono le validazioni eseguite su tutti i parametri? Cosa succede in condizioni di eccezione? Spiegare ogni possibile ambiguità? Ad un certo punto il nome sarebbe diventato così lungo da rendere semplicemente confusionario. Accetterei String BuildFullName (String firstname, String lastname) come firma di funzione decente. Anche se ciò non chiarisce se il nome è espresso "first last" o "last, first" o qualche altra variazione, cosa fa se una o entrambe le parti del nome sono vuote, se impone limiti sulla lunghezza combinata e cosa fa se questo è superato, e dozzine di altre domande sui dettagli che si potrebbero chiedere.