Il codice commentato può essere una documentazione preziosa?

79

Ho scritto il seguente codice:

if (boutique == null) {
    boutique = new Boutique();

    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.persist(boutique);
} else {
    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    //boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.merge(boutique);
}

C'è una riga commentata qui. Ma penso che renda il codice più chiaro, rendendo evidente quale sia la differenza tra if e else . La differenza è ancora più evidente con l'evidenziazione dei colori.

È possibile che commentare un codice come questo sia sempre una buona idea?

    
posta Alexis Dufrenoy 11.03.2013 - 10:36
fonte

13 risposte

101

La maggior parte delle risposte si concentra su come refactoring questo caso specifico, ma mi permetta di offrire una risposta generale al perché il codice commentato è di solito cattivo:

Innanzitutto, il codice commentato non è compilato. Questo è ovvio, ma significa che:

  1. Il codice potrebbe anche non funzionare.

  2. Quando le dipendenze del commento cambiano, ovviamente non si romperà.

Il codice commentato è molto "codice morto". Più a lungo rimane lì, più marcisce e fornisce sempre meno valore al prossimo sviluppatore.

In secondo luogo, lo scopo non è chiaro. Hai davvero bisogno di un commento più lungo che fornisca un contesto per il motivo per cui ci sono linee commentate a caso. Quando vedo solo una riga di codice commentata, devo cercare come è arrivata solo per capire perché è arrivata lì. Chi lo ha scritto? Cosa commette? Qual è stato il messaggio di commit / contesto? Eccetera.

Considera alternative:

  • Se l'obiettivo è fornire esempi di utilizzo di una funzione / api, fornire un test unitario. I test unitari sono codice reale e si interromperanno quando non sono più corretti.
  • Se lo scopo è conservare una versione precedente del codice, utilizzare il controllo del codice sorgente. Preferirei eseguire il checkout di una versione precedente, quindi attivare i commenti in tutto il codice per "annullare" una modifica.
  • Se lo scopo è di mantenere una versione alternativa dello stesso codice, utilizzare il controllo del codice sorgente (di nuovo). Questo è ciò che i rami servono, dopotutto.
  • Se lo scopo è quello di chiarire la struttura, considera come puoi ristrutturare il codice per renderlo più ovvio. La maggior parte delle altre risposte sono buoni esempi di come potresti fare questo.
risposta data 12.03.2013 - 05:25
fonte
257

Il più grande problema con questo codice è che hai duplicato quelle 6 righe. Una volta eliminata la duplicazione, quel commento è inutile.

Se crei un metodo boutiqueDao.mergeOrPersist puoi riscriverlo come:

if (boutique == null) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

boutiqueDao.mergeOrPersist(boutique);

Il codice che crea o aggiorna un determinato oggetto è comune, quindi dovresti risolverlo una volta, ad esempio creando un metodo mergeOrPersist . Certamente non dovresti duplicare tutto il codice di assegnazione per questi due casi.

Molti ORM hanno supportato in questo modo in qualche modo. Ad esempio, potrebbero creare una nuova riga se id è zero e aggiornare una riga esistente se id non è zero. La forma esatta dipende dall'ORM in questione e, poiché non ho familiarità con la tecnologia che stai utilizzando, non posso aiutarti con questo.

Se non vuoi creare un metodo mergeOrPersist , dovresti eliminare la duplicazione in qualche altro modo, ad esempio introducendo un flag isNewBoutique . Potrebbe non essere bello, ma è comunque molto meglio che duplicare l'intera logica di assegnazione.

bool isNewBoutique = boutique == null;
if (isNewBoutique) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

if (isNewBoutique)
    boutiqueDao.persist(boutique);
else
    boutiqueDao.merge(boutique);
    
risposta data 11.03.2013 - 10:45
fonte
161

Questa è un'idea assolutamente terrificante . Non chiarisce quale sia l'intento. Lo sviluppatore ha commentato la linea per sbaglio? Per testare qualcosa? Cosa sta succedendo?!

A parte il fatto che vedo 6 righe che sono assolutamente uguali in entrambi i casi. Piuttosto, dovresti evitare questa duplicazione del codice. Quindi sarà più chiaro che in un caso si chiama anche setSelected.

    
risposta data 11.03.2013 - 10:41
fonte
117

No, è una pessima idea. Sulla base di quel pezzo di codice mi vengono in mente i seguenti pensieri:

  • Questa riga è commentata perché lo sviluppatore ha eseguito il debug e ha dimenticato di ripristinare la riga nel suo stato precedente
  • Questa riga è commentata perché una volta faceva parte della logica aziendale, ma non è più il caso
  • Questa riga è commentata perché ha causato problemi di prestazioni in fase di produzione e lo sviluppatore voleva vedere quale fosse l'impatto su un sistema di produzione

Dopo aver visto migliaia di righe di codice commentato, ora sto facendo l'unica cosa sensata quando la vedo: la rimuovo immediatamente.

Non vi è alcun motivo ragionevole per archiviare il codice commentato in un repository.

Inoltre, il tuo codice utilizza molte duplicazioni. Ti suggerisco di ottimizzarlo al più presto possibile per renderlo leggibile.

    
risposta data 11.03.2013 - 10:47
fonte
49

Vorrei solo aggiungere alla risposta di CodesInChaos, sottolineando che puoi rifattalo ulteriormente in piccoli metodi . La condivisione delle funzionalità comuni per composizione evita i condizionali:

function fill(boutique) {    
  boutique.setSite(site);
  boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
  boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
  boutique.setNom(fluxBoutique.getNom());
  boutique.setIdWebSC(fluxBoutique.getId());
  boutique.setDateModification(new Date());
}    

function create() {
  boutique = new Boutique();      
  fill(boutique);
  boutique.setSelected(false);
  return boutiqueDao.persist(boutique);
}

function update(boutique) {
  fill(boutiquie);
  return boutiquieDao.merge(boutique); 
}

function createOrUpdate(boutique) {
  if (boutique == null) {
    return create();
  }
  return update(boutique);  
}
    
risposta data 11.03.2013 - 14:33
fonte
23

Anche se questo non è chiaramente un buon caso per il codice commentato, c'è una situazione che credo lo giustifichi:

// The following code is obvious but does not work because of <x>
// <offending code>
<uglier answer that actually does work>

È un avvertimento per chiunque lo veda dopo che l'ovvio miglioramento non lo è.

Modifica: sto parlando di qualcosa di piccolo. Se è grande, spiega invece.

    
risposta data 12.03.2013 - 04:43
fonte
13

In questo esempio specifico, trovo ambiguo il codice commentato molto , in gran parte per le ragioni delineate in la risposta di Dibkke . Altri hanno suggerito modi in cui è possibile rifattorizzare il codice per evitare anche la tentazione di farlo, tuttavia, se ciò non è possibile per qualche motivo (ad esempio, le linee sono simili, ma non abbastanza vicine), gradirei un commento del tipo:

// No need to unselect this boutique, because [WHATEVER]

Tuttavia, penso che ci siano alcune situazioni in cui lasciare (o addirittura aggiungere commenti) il codice non è riprovevole. Quando si utilizza qualcosa come MATLAB o NumPY, si può spesso scrivere codice equivalente che 1) itera su un array, elaborando un elemento alla volta o 2) gestisce l'intero array in una sola volta. In alcuni casi, quest'ultimo è molto più veloce, ma anche molto più difficile da leggere. Se sostituisco un codice con il suo equivalente vettoriale, incorporo il codice originale in un commento vicino, come questo:

%% The vectorized code below does this:

% for ii in 1:N
%    for jj in 1:N
%      etc.

% but the matrix version runs ~15x faster on typical input (MK, 03/10/2013)

Ovviamente, è necessario fare attenzione che le due versioni facciano effettivamente la stessa cosa e che il commento rimanga sincronizzato con il codice reale o venga rimosso se il codice cambia. Ovviamente, i soliti avvertimenti sull'ottimizzazione prematura si applicano anche ...

    
risposta data 11.03.2013 - 17:53
fonte
9

L'unica volta che ho visto il codice commentato che era utile era nei file di configurazione, dove è fornito il codice per ogni opzione, con molte opzioni commentate. Ciò semplifica l'attivazione e la disattivazione delle funzioni semplicemente rimuovendo i marcatori di commenti.

## import this list of modules
# config.imports = ['os', 'sys']
    
risposta data 13.03.2013 - 00:09
fonte
6

In generale, il codice è solo auto-documentante per la persona che ha scritto il codice. Se è richiesta la documentazione, scrivere la documentazione. È inaccettabile aspettarsi che uno sviluppatore nuovo di una base di codice sorgente siediti a leggere migliaia di righe di codice per tentare di capire da un alto livello ciò che sta accadendo.

In questo caso, lo scopo della riga di codice commentata è di mostrare la differenza tra due istanze di codice duplicato. Invece di tentare di documentare sottilmente la differenza con un commento, riscrivi il codice in modo che abbia senso. Quindi, se ritieni ancora necessario commentare il codice, scrivi un commento appropriato.

    
risposta data 11.03.2013 - 15:53
fonte
4

No, il codice commentato diventa obsoleto, ed è presto peggio che inutile, è spesso dannoso, poiché cementa alcuni aspetti dell'implementazione, insieme a tutti gli attuali presupposti.

I commenti dovrebbero includere dettagli dell'interfaccia e funzioni previste; "Funzione prevista": può includere, prima proviamo questo, poi ci proviamo, quindi falliamo in questo modo.

I programmatori che ho visto cercano di lasciare le cose nei commenti sono semplicemente innamorati di ciò che hanno scritto, non vogliono perderlo, anche se non aggiungono nulla al prodotto finito.

    
risposta data 11.03.2013 - 19:05
fonte
2

Può essere in casi molto rari, ma non come hai fatto tu. Le altre risposte hanno piuttosto cancellato le ragioni per questo.

Uno dei rari casi è una specifica RPM del modello che utilizziamo nel mio negozio come punto di partenza per tutti i nuovi pacchetti, in gran parte per assicurarsi che nulla di importante sia lasciato fuori. La maggior parte, ma non tutti i nostri pacchetti hanno un archivio contenente sorgenti che ha un nome standard e viene specificato con un tag:

Name:           foomatic
Version:        3.14
 ...
Source0:        %{name}-%{version}.tar.gz

Per i pacchetti senza fonti, commentiamo il tag e mettiamo un altro commento sopra per mantenere il formato standard e indicare che qualcuno si è fermato e ha pensato al problema come parte del processo di sviluppo:

Name:           barmatic
Version:        2.71
 ...
# This package has no sources.
# Source0:        %{name}-%{version}.tar.gz

Non aggiungi codice che sai non verrà utilizzato perché, come altri hanno già detto, potrebbe essere scambiato per qualcosa che appartiene a questo. Può. tuttavia, può essere utile aggiungere un commento che spieghi perché il codice uno potrebbe aspettarsi di essere mancante:

if ( condition ) {
  foo();
  // Under most other circumstances, we would do a bar() here, but
  // we can't because the quux isn't activated yet.  We might call
  // bletch() later to rectify the situation.
  baz();
}
    
risposta data 11.03.2013 - 16:05
fonte
0

Il codice commentato non viene utilizzato dall'applicazione, quindi deve essere accompagnato da ulteriori commenti che affermano perché non viene utilizzato. Ma in quel contesto, ci sono sono situazioni in cui il codice commentato può essere utile.

Ciò che mi viene in mente è un caso in cui risolvi un problema usando un approccio comune e accattivante, ma poi si scopre che i requisiti del tuo problema reale sono leggermente diversi da quel problema. Soprattutto se le tue esigenze richiedono molto più codice, la tentazione per i manutentori di "ottimizzare" il codice usando il vecchio approccio sarà probabilmente strong, ma farlo ripristinerà solo il bug. Mantenere l'implementazione "sbagliata" nei commenti aiuterà a dissipare questo, perché è possibile utilizzarlo per illustrare esattamente perché quell'approccio è sbagliato in questa situazione.

Questa non è una situazione che posso immaginare accadere molto spesso. Di solito, dovrebbe essere sufficiente spiegare le cose senza includere un'implementazione "errata" di esempio. Ma io posso immaginare un caso in cui non è abbastanza, quindi dato che la domanda riguarda se può essere utile, sì, può. Solo non la maggior parte del tempo.

    
risposta data 12.02.2014 - 17:07
fonte
-2

Questo non sembra un buon amico.

Il codice commentato è ... non solo codice. Il codice è per l'implementazione della logica. Rendere un codice più leggibile di per sé è un'arte. Poiché @CodesInChaos ha suggerito già che le righe di codice ripetitive non sono un'ottima implementazione della logica .

Pensi davvero che un vero programmatore preferirà la leggibilità rispetto all'implementazione logica. (dal modo in cui abbiamo commenti e "complementi" da inserire nella nostra rappresentazione logica).

Per quanto mi riguarda, si dovrebbe scrivere un codice per il compilatore e va bene - se "capisce" quel codice. Per la leggibilità umana I commenti sono buoni, per gli sviluppatori (a lungo termine), per le persone che riutilizzano quel codice (ad esempio i tester).

Altrimenti puoi provare qualcosa di più flessibile qui, qualcosa come

boutique.setSite (sito) può essere sostituito con

setsiteof.boutique (sito). Ci sono diversi aspetti e prospettive di OOP attraverso i quali è possibile aumentare la leggibilità.

Anche se all'inizio questo codice sembra molto allettante e si può pensare di aver trovato un modo per la leggibilità umana, mentre il compilatore fa anche il suo lavoro perfettamente, e iniziamo tutti seguendo questa pratica che porterà a un file fuzzy che diventa meno leggibile nel tempo e più complesso in quanto si espanderà verso il basso.

    
risposta data 11.03.2013 - 13:36
fonte

Leggi altre domande sui tag