Manutenzione del codice: aggiungere commenti nel codice o lasciarlo semplicemente al controllo della versione?

42

Ci è stato chiesto di aggiungere commenti con tag di inizio, tag di fine, descrizione, soluzione ecc. per ogni modifica che apportiamo al codice come parte della correzione di un bug / implementazione di un CR.

La mia preoccupazione è che questo fornisce un valore aggiunto? Così com'è, abbiamo tutti i dettagli nella cronologia del controllo della versione, che ci aiuterà a tracciare ogni singolo cambiamento?

Ma i miei lead insistono nell'avere i commenti come una buona pratica di programmazione. Uno dei loro argomenti è quando un CR deve essere de-ambito / modificato, sarebbe ingombrante se i commenti non ci sono.

Considerando che i cambiamenti sarebbero in gran parte in mezzo al codice, sarebbe davvero utile aggiungere commenti per ogni singolo cambiamento che facciamo? Non dovremmo lasciarlo al controllo della versione?

    
posta Chillax 09.11.2012 - 19:32
fonte

9 risposte

43

Hai assolutamente ragione. Il monitoraggio delle modifiche è il lavoro per il tuo sistema di controllo della versione. Ogni volta che esegui un commit, dovresti scrivere un messaggio di commit che spiega cosa è stato fatto e fare riferimento al tuo sistema di tracciamento dei bug se si tratta di una correzione di bug. Mettendo un commento nel codice dicendo

// begin fix for bug XXXXX on 10/9/2012
...
// end fix for bug XXXXX

ogni volta che risolvi un bug, il tuo codice diventerà rapidamente illeggibile e non gestibile. Risulterà anche la duplicazione delle stesse informazioni in due punti, il che renderà il pasticcio ancora peggiore.

I commenti non dovrebbero essere usati per il tracciamento dei bug e inoltre non dovrebbero descrivere cosa sta facendo il tuo codice. Dovrebbero spiegare perché stai facendo X, o perché stai facendo X in questo modo particolare. Se senti la necessità di scrivere un commento che spiega cosa sta facendo un blocco di codice, questo è un odore di codice che indica che devi refactoring questo blocco in una funzione con un nome descrittivo.

Quindi invece di

// fixed bug XXXXX on 10/9/2012

potresti avere un commento che dice

// doing X, because otherwise Y will break.

o

// doing X, because doing Y is 10 times slower.
    
risposta data 09.11.2012 - 21:39
fonte
53

Utilizza lo strumento migliore per il lavoro. Il tuo sistema di controllo della versione dovrebbe essere lo strumento migliore per la registrazione quando vengono eseguite correzioni di bug e CR: registra automaticamente la data e chi fatto il cambiamento; non dimentica mai di aggiungere un messaggio (se lo hai configurato per richiedere messaggi di commit); non annota mai la riga di codice errata o cancella accidentalmente un commento. E se il tuo sistema di controllo delle versioni sta già facendo un lavoro migliore dei tuoi commenti, è stupido duplicare il lavoro aggiungendo commenti.

La leggibilità del codice sorgente è fondamentale. Una base di codice piena di commenti che fornisce la cronologia completa di ogni bugfix e CR creati non sarà affatto leggibile.

Ma non saltare completamente i commenti: I buoni commenti (che non documentano servilmente ogni start / stop / descrizione / soluzione di ogni bugfix e CR) migliorano la leggibilità del codice. Ad esempio, per un bit di codice difficile o poco chiaro che aggiungi per correggere un bug, un commento del modulo // fix ISSUE#413 che dice alla gente dove trovare più informazioni nel tuo tracker di problemi è un'idea eccellente.

    
risposta data 09.11.2012 - 19:50
fonte
7

I commenti nel codice riguardano ciò che il codice è in quel momento. Fare un'istantanea in qualsiasi momento non dovrebbe fare riferimento a versioni precedenti (o peggio future) del codice.

I commenti in VCS riguardano il modo in cui il codice è cambiato. Dovrebbero leggere come una storia sullo sviluppo.

Ora, ogni modifica dovrebbe includere commenti? nella maggior parte dei casi, sì. L'unica eccezione che immagino è quando il comportamento previsto è già stato documentato, ma non era quello che hai ottenuto, a causa di un bug. La correzione rende i commenti esistenti più precisi, quindi non devono essere modificati. Il bug stesso dovrebbe essere documentato nella cronologia del ticket e nel commento di commit, ma solo nel codice se il codice sembra strano. In tal caso, un // make sure <bad thing> doesn't happen dovrebbe essere sufficiente.

    
risposta data 09.11.2012 - 19:46
fonte
6

Un tipo di commento che apprezzo molto è:

// Questo è stato implementato per la regola aziendale 5 della proposta 2

o qualunque cosa diavolo usi per raccogliere i tuoi requisiti.

Questo ha due vantaggi, uno è che ti permette di trovare il motivo per cui hai implementato un determinato algoritmo senza cercare e un altro è che ti aiuterà a comunicare con ingegneri non software che lavorano / creano i documenti dei requisiti.

Questo potrebbe non essere d'aiuto con team più piccoli, ma se hai analisti che sviluppano i tuoi requisiti, può essere inestimabile.

    
risposta data 09.11.2012 - 23:29
fonte
4

I tuoi lead hanno ragione quando dicono che i commenti sono una buona pratica di programmazione, tuttavia ci sono delle eccezioni. Aggiungere un commento per ogni modifica apportata è uno di questi. E hai ragione dicendo che questo dovrebbe appartenere al sistema di controllo della versione. Se devi mantenere questi commenti in un posto solo, allora il VCS è la strada da percorrere. I commenti nel codice sorgente tendono a diventare vecchi e non mantenuti. Nessun commento è molto meglio dei commenti negativi. Quello che non vuoi è avere commenti in entrambe le posizioni (nel codice e VCS) che sono fuori sincrono. L'obiettivo è di mantenere le cose ASCIUTTE avendo un'unica fonte di verità per le modifiche al codice.

    
risposta data 09.11.2012 - 19:47
fonte
3

Oltre a ciò che altri hanno detto, considera cosa succede se un cambiamento ha effetti a catena su tutto il sistema. Supponete di refactoring una parte di un'interfaccia core nel processo di implementazione di una richiesta di modifica - quel tipo di modifica può facilmente toccare una grande percentuale dei file del codice sorgente in qualsiasi software non banale con ciò che equivale a cambiamenti banali (classe o modifiche al nome del metodo). Dovresti passare attraverso ogni singolo file toccato da tale operazione per annotarlo manualmente con tali commenti, piuttosto che fare affidamento sul VCS che fa tutto automaticamente? In un caso si sta osservando poco più di un lavoro di cinque minuti con uno strumento di refactoring decente seguito da una ricompilazione per assicurarsi che nulla abbia rotto la build, mentre l'altro può facilmente gonfiarsi nel lavoro di una giornata. Per quale vantaggio specifico?

Considerare anche cosa succede quando si spostano parti del codice. Uno degli sviluppatori di database con cui lavoro è in gran parte "ogni riga di SQL dovrebbe essere annotata con la revisione in cui è stata modificata, e faremo delle storie di revisione separate per ogni file, perché allora è più facile vedere chi ha cambiato cosa quando e perché ". Funziona benissimo quando la modifica è nell'ordine di modifica delle singole linee. Non funziona altrettanto bene quando, come ho fatto recentemente per risolvere un problema serio di prestazioni, si suddividono parti di una query più ampia introducendo tabelle temporanee, quindi si modifica l'ordine di alcune query per adattarsi meglio al nuovo flusso di codice. Certo, il diff contro la versione precedente era in gran parte privo di significato poiché diceva che circa i due terzi del file erano cambiati, ma il commento del check-in era anche simile a "una grande riorganizzazione per risolvere i problemi di prestazioni". Nel momento in cui hai guardato manualmente le due versioni, era abbastanza chiaro che le parti grandi erano davvero le stesse, solo spostate. (E ha impiegato la procedura memorizzata in questione impiegando regolarmente più di mezzo minuto per eseguire, in pochi secondi. A quel punto, era in gran parte legato all'I / O, con pochi punti di miglioramento significativo, almeno nel breve periodo. )

Con pochissime eccezioni, modifica il tracciamento e il riferimento ai problemi è il lavoro di VCS, IMNSHO.

    
risposta data 09.11.2012 - 22:36
fonte
3

Di solito seguo questa regola: se il cambiamento è ovvio e il codice risultante non solleva domande, non ripristina o modifica sostanzialmente qualsiasi comportamento precedente in modo sostanziale - quindi lascialo al VCS per tenere traccia dei numeri di bug e di altri cambiamenti informazioni.

Tuttavia, se c'è un cambiamento che non è ovvio, questo cambia la logica - in particolare cambia sostanzialmente la logica fatta da qualcun altro in un modo non ovvio - può essere molto utile aggiungere qualcosa come "questo cambiamento è fai questo e quello a causa del bug # 42742 ". In questo modo quando qualcuno guarda il codice e si chiede "perché questo è qui? Questo sembra strano" ha qualche guida proprio di fronte a lui e non deve fare indagini tramite VCS. Ciò impedisce anche le situazioni in cui le persone interrompono le modifiche altrui perché conoscono il vecchio stato del codice ma non si accorgono che sono state modificate da allora.

    
risposta data 10.11.2012 - 08:25
fonte
2

I commenti relativi al controllo della versione non appartengono al file sorgente. Aggiungono solo confusione. Dal momento che è probabile che debbano essere messi nello stesso posto (come un blocco di commenti nella parte superiore del file) causeranno dei conflitti di disturbo di solo commento quando i rami paralleli vengono uniti.

Qualsiasi informazione di tracciamento che può essere estratta dal controllo della versione non dovrebbe essere duplicata nel corpo del codice. Ciò vale per idee sciocche come le parole chiave di verifica di RCS come $Log$ e il suo ilk.

Se il codice viaggia sempre al di fuori dell'ambito del sistema di controllo della versione, quella scia di commenti sulla sua storia perde contesto e quindi gran parte del suo valore. Per comprendere correttamente la descrizione della modifica, abbiamo bisogno di accedere alla revisione, in modo che possiamo visualizzare il diff alla versione precedente.

Alcuni vecchi file nel kernel di Linux hanno grandi blocchi di commenti storici. Quelli risalgono a quando non esisteva il sistema di controllo della versione, solo tarball e patch.

    
risposta data 09.11.2012 - 23:52
fonte
2

I commenti nel codice dovrebbero essere minimi e precisi. Aggiunta di difetti / modifica delle informazioni non preziose. Dovresti usare il controllo della versione per questo. Qualche volta il controllo della versione fornisce un modo leggermente migliore di cambiare- Usiamo ClearCase UCM; Le attività UCM vengono create in base a numeri di difetto, area di modifica ecc. (Ad esempio defect29844_change_sql_to_handle_null).

I commenti dettagliati sono preferiti nei commenti del check-in.

Preferisco includere fornire dettagli sulle informazioni sul terreno posteriore, dettagli della soluzione NON implementati a causa di alcuni effetti collaterali.

Pramagic Programmer e CleanCode portano alle seguenti linee guida

Mantieni la conoscenza di basso livello nel codice, a cui appartiene e prenota i commenti per altre spiegazioni di alto livello.

    
risposta data 10.11.2012 - 12:53
fonte

Leggi altre domande sui tag