Codice inutile nella tua fonte

Ho sentito sviluppatori che cercano di rendere i loro risultati di codifica più complessi di quanto non siano in realtà. Non ho mai sentito nessuno ammetterlo, ma ho visto codice che soddisfa i tuoi criteri che è stato creato intenzionalmente per fretta o cattive pratiche e non sabotaggio. Il codice che circonda il codice diffamato potrebbe essere stato alterato al punto in cui una particolare funzione non è più utile.

Qualcuno dovrebbe effettivamente vedere questo codice di prima mano prima di giungere alla conclusione che solo questo sviluppatore può gestire la complessità. La maggior parte dei manager e di altri uomini d'affari arrivano a questa conclusione perché non capiscono alcun tipo di codice e non vogliono riempire la posizione.

Non ho visto il codice come questo ma ho visto il codice che sembra inutile o inutile per gli altri motivi:

1. Compatibilità con le versioni precedenti. Hai trovato un modo molto migliore di fare le cose, ma devi mantenere l'API / funzione vecchia (e ormai non molto utile) perché alcuni moduli di terze parti potrebbero utilizzare questa API / funzione per qualcosa. Anche se la funzione non fa nulla di utile, l'assenza di esso potrebbe rompere il codice.

2. Codifica difensiva. Sai che i controlli in questo codice sono inutili perché questo è già stato controllato altrove. Ma cosa succede se qualcuno cambia questo codice altrove e rimuove o modifica i controlli in modo che non corrispondano più alle precondizioni?

3. Crescita organica. Nei grandi progetti, nel corso degli anni molte cose cambiano, e si scopre che alcuni metodi che sono stati usati prima non sono più usati, ma nessuno si è preoccupato di rimuoverli poiché nessuno ha tenuto traccia di se questo metodo specifico è usato o meno, hanno semplicemente refactored i loro pezzi di codice e per caso è successo che si sono fermati a usare questo metodo. O condizioni che un tempo avevano significato, ma l'applicazione è stata rifattorizzata in altri luoghi, in modo che la condizione diventasse sempre vera ma nessuno si prendeva la briga di rimuoverlo.

4. Over-progettazione. Le persone potrebbero codificare alcune cose "nel caso avessimo bisogno" e non ne hanno mai realmente bisogno. Come "facciamo un thread nel caso in cui dovessimo lavorare offline" e poi nessuno chiede di fare qualcosa offline e il programmatore si dimentica di esso e passa ad altri progetti (o forse anche a un'altra azienda) e quel codice rimane lì per sempre perché nessuno sa perché è lì o se è sicuro rimuoverlo.

Quindi, anche se non l'ho mai visto fatto per malizia o per approccio errato alla sicurezza del lavoro, ho visto un sacco di volte quando accade come risultato naturale dello sviluppo del software.

My question is. Has anyone else seen code like this? What was your conclusion was to why that code was there?

1) Sì.

2) Nei casi che ho visto, l'ho messo in vari modi per:

• inesperienza del programmatore
• Il programmatore non capisce un progetto particolarmente complicato e / o mal eseguito che sta tentando di modificare
• Il programmatore viene interrotto nel mezzo di (diciamo) un refactore.
• Disattenzione

Ora forse sono caritatevole a riguardo, ma il mio approccio generale è che è meglio essere indulgenti / non conflittuali riguardo a queste cose, piuttosto che puntare le dita e continuare a parlare di scarsa qualità. Ovviamente, le cose potrebbero diventare abbastanza brutte da dover fare qualcosa , ma una leggera spinta nella giusta direzione è di solito sufficiente.

Naturalmente, non è possibile adottare un approccio del tipo "laissez-faire" se la qualità / gli errori incideranno seriamente sul "business". Ma in quella situazione hai bisogno di obbligatorio e diligente recensione del codice di tutto, combinato con una procedura di test completa.

Secondo la mia esperienza, le persone tendono ad essere "tese" sul codice di scarsa qualità (almeno in parte) perché offende i loro standard personali. È molto bello (personalmente) aspirare alla perfezione, ma è un po 'irragionevole proiettare i tuoi standard personali su altre persone. Dai suoni delle cose (ad esempio dalla natura dei tuoi esempi), questo è quello che stai facendo.

IMO, questo non è produttivo e non favorisce un buon rapporto di lavoro con i tuoi colleghi.

Tutti questi sono spesso i sintomi di come un progetto invecchia.

1. Chiamate di metodo o di funzione che non fanno nulla di valore. Ci sono molte volte in cui un codice viene semplicemente lasciato così com'è (si spera con un grande avvertimento deprecato , ma poiché la maggior parte delle lingue non ha quell'avvertimento, non è sempre seguito ...) perché, ad un certo punto, è servito a uno scopo genuino e nessuno sapeva cosa sarebbe potuto accadere se le linee offensive fossero state rimosse.

Mi sembra di ricordare questo da un dailywtf:

@deprecated // he might have been crazy enough to use reflection...
boolean getTrue() {
    return false; 
}

2. Controlli ridondanti eseguiti in un file di classe, oggetto o metodo separati. I livelli di comunicazione sono anche imperfetti (mai letto il mese di Mythical Man? In caso contrario, cosa stai facendo sul computer !? Vai! LEGGI!). Spesso, una persona lavora su qualcosa e poi lascia il progetto, e poi il prossimo, trovando qualche bizzarro bug, lancia un controllo extra qua e là per cercare di eliminarlo. Quando il bug viene rimosso, i controlli non sono perché, beh, se non è rotto, non aggiustarlo.

3. se affermazioni che valgono sempre come vere. Oh, l'ho fatto. Ho avuto un progetto una volta, aveva una serie di probabilmente 10-15 se / else . Per cambiare il comportamento, ho semplicemente messo un true|| al primo blocco. Non è stato fino a mesi (anni?) Più tardi che sono tornato e ho detto, "Oh, wow, questo codice avrebbe dovuto essere distrutto ma non lo è mai stato"

4. Discussioni che si interrompono e non fanno nulla di importante. Posso immaginare una linea di pensiero che vada in questo modo:

1. Lo so! Posso gestire questi due problemi in modo asincrono! Creerò i thread foo e bar.
2. (due mesi dopo) Huh, sai, la funzionalità della barra è leggermente migliore in foo. Mi sposterò un po '.
3. (un anno dopo) Sapete, è sensato mettere quest'altra roba dal bar in foo.
4. (molti, molti anni dopo) "Ehi, questo thread bar non sembra che stia facendo qualcosa, possiamo rimuoverlo?" "Meglio di no, ci sono stati molti, molti anni ..."

Sono un po 'più ottimista. Penso che ciò che hai descritto spesso si verifica quando il codice viene refactored con noncuranza.

I vecchi amici mi hanno riferito di un periodo in cui i consulenti venivano pagati dal numero di linee di codice che producevano. E così massimizzano i profitti usando costrutti sorprendentemente prolissi.

Oggi presumo sempre che il ragazzo stia ancora imparando la lingua mentre fa il lavoro. E ha fretta.

La maggior parte delle risposte si riduce a questi due semplici fatti:
[1] Il codice riflette la cronologia del codice e
[2] Il codice riflette il futuro previsto del codice.

Ho scritto funzioni che non hanno alcun valore, NELL'AMBIENTE DI APPLICAZIONE ATTUALE date le SPECIFICHE ATTUALI, ma potrebbero essere necessarie in futuro.

Ho scritto if-statement che, ATTUALMENTE, valgono sempre come vere. Ma forse in passato potrebbe essere falso.

Per quanto riguarda i controlli ridondanti, hey, non mi fido di altro codice, non mi fido nemmeno del mio codice. Se un modulo dipende dal fatto che N sia 1 o 2 o 3, si è degnato meglio di assicurarlo, e si blocca in modo informale se non lo è. È illegittimo che il modulo B esploda perché il modulo A si è rovinato; è del tutto legittimo che il modulo B si lamenti del fatto che il modulo A si sia rovinato. E ricorda che, il mese prossimo, quel parametro potrebbe provenire dal modulo C non ancora scritto.

L'ho visto poche volte, infatti, proprio ieri, ho dovuto fondere parte del codice dei miei boss nella mia nuova app. Nel suo caso è solo una generale mancanza di abilità e comprensione e la convinzione che lui pensi di essere uno sviluppatore abbastanza abile.

'Chiamate di metodo o funzione che non fanno nulla di valore.' e "se le affermazioni che valgono sempre come vere" sono un grosso problema con il suo codice.

Ho il sospetto che, sebbene molti abbiano un codice visto che ha questi problemi, pochi potrebbero cercare di scrivere lo stesso. Con ogni probabilità, quello che vedi è un accumulo di software - qualcuno aggiunge qualcosa, che non funziona davvero, il prossimo manutentore aggiunge ulteriore codice protettivo lungo la catena per proteggersi dalla condizione che non è stata correttamente controllata nel primo posto; poi qualcuno riceve un rapporto sui problemi e aggiunge ancora più blindaggio contro un'istanza specifica di un problema; un'altra persona aggiunge un controllo più generale e dimentica di rimuovere parte del vecchio codice aggiunto in precedenza che trattava sintomi più specifici, ecc.

Poi c'è il problema della pulizia del codice: non c'è alcun incentivo particolare a rimuovere ciò che sembra essere un codice morto e un incredibile incentivo non a farlo, perché se non si Non capisco il codice completamente, la tua valutazione che il codice è "morto" sarà difettoso e finirai per rompere il sistema.

• Method or function calls that do nothing of value.

Non necessariamente male. I metodi in una classe base spesso chiamano metodi vuoti che sono intesi come punti override per le sottoclassi. Esempio: l'UIView di Cocoa Touch ha un metodo -didAddSubview: che è documentato come non fare nulla nella versione predefinita. Il metodo -addSubview: di UIView deve chiamare -didAddSubview: anche se non fa nulla perché le sottoclassi possono implementarlo per fare qualcosa. Metodi che non fanno nulla e le ragioni per cui dovrebbero essere documentati, naturalmente.

Se una funzione / metodo vuoto o inutile è ovviamente presente per ragioni storiche, dovrebbe essere cancellato. Dai un'occhiata alle versioni precedenti del codice nel tuo repository del codice sorgente, se non sei sicuro.

• Redundant checks done in a separate class file, object or method.

Difficile dire se va bene senza un contesto. Se i controlli sono chiaramente fatti per lo stesso motivo, ciò potrebbe significare che non c'è una chiara separazione delle responsabilità e che alcuni refactoring sono richiesti, specialmente quando entrambi i controlli portano alla stessa azione intrapresa. Se l'azione risultante da entrambi i controlli non è la stessa, probabilmente i due controlli vengono eseguiti per ragioni diverse anche se la condizione è la stessa, e probabilmente è ok.

• if statements that always evaluate to true.

C'è una grande differenza tra:

if (1) {
    // ...
}

e

if (foo() == true) {
    // ...
}

dove foo() succede sempre restituire true .

Il primo caso si verifica molto quando le persone eseguono il debug. È facile utilizzare if (0) {... per rimuovere temporaneamente un blocco di codice mentre stai cercando di isolare un bug, quindi modificare il 0 in 1 per ripristinare quel codice. Il if dovrebbe essere rimosso una volta che hai finito, ovviamente, ma è facile dimenticarlo, o perdere uno o due se lo hai fatto in più punti. (È una buona idea identificare tali condizionali con un commento che è possibile cercare successivamente.) L'unico danno è la confusione che potrebbe causare in futuro; se il compilatore può determinare il valore della condizione al momento della compilazione, lo rimuoverà completamente.

Il secondo caso può essere accettabile. Se la condizione rappresentata da foo() deve essere testata da più punti del codice, il factoring in una funzione o un metodo separati è spesso la cosa giusta da fare anche se foo() è sempre vero in questo momento. Se è ipotizzabile che foo() possa alla fine restituire false , isolare quella condizione in un metodo o funzione è un modo per identificare tutte le posizioni in cui il codice si basa su tale condizione. Tuttavia , facendo ciò si rischia che la condizione foo() == false non venga testata e potrebbe causare problemi in seguito; la soluzione è assicurarsi di aggiungere test unitari che testino esplicitamente il caso false .

• Threads that spin off and do nothing of note.

Questo sembra un artefatto della storia e qualcosa che potrebbe essere identificato durante una revisione del codice o attraverso la profilazione periodica del software. Suppongo che potrebbe essere creato intenzionalmente, ma ho difficoltà a immaginare che qualcuno lo faccia di proposito.

Succede. Abbastanza spesso in realtà.

A volte questi vicoli ciechi di codifica sono più simili a vecchi sentieri di capra che sono caduti in rovina quando è stata installata una superstrada più efficiente / moderna / veloce intorno a loro.

In altre occasioni (e probabilmente ne sono colpevole), sono le basi per l'estensione del software quando viene richiesto un insieme di caratteristiche / funzioni, ma non confermato. (A volte mettere un po 'di lavoro nella build iniziale fornendo maniglie e simili per il lavoro successivo che si prevede di "imbullonare" può rendere la vita più facile, quando accade, o più difficile / disordinata se il lavoro ulteriore non eventuate.)

E, molto è direttamente dovuto al vecchio "Se non è rotto, non adattarlo". A volte strappare il codice che non capisci, o credere che non sia utilizzato, può causare la versione di programmazione di "The Butterfly Effect". È successo anche una o due volte.

A volte avrò un booleano globale impostato su true, e più avanti nel mio codice un if (bool), quindi durante il runtime, potrei impostare un punto di interruzione sull'istruzione if, e cambiare il booleano per testare qualcosa.

Mi oppongo alle dichiarazioni if true per essere classificate indiscriminatamente come "codice senza senso".

C'è un caso legittimo per l'utilizzo di un blocco if (1) { ... } nel codice C che vuole essere compatibile con lo standard C che ha insistito sul fatto che le definizioni di variabili siano all'inizio di una funzione, o vuole solo che siano definite le variabili locali il più localmente possibile.

switch (i) {
    case 23:
        if (1) {
            /* I can declare a local var here! */
        }
        break;
}

Un mio professore ci raccontò una storia un giorno che un precedente datore di lavoro li avrebbe pagati in base al numero di linee che completavano. Così, hanno scritto diverse dozzine di funzioni allineate che non sono mai state chiamate. Sembra una buona ragione per scrivere codice senza senso.

Come menzionato @Andy, un grande componente che ho visto si sta rompendo YAGNI .

Qualcuno inizia con una supposizione su ciò che tutti gli elementi saranno invece di ciò che potrebbe avere bisogno di molti elementi , che è una confusione del "è un " e " ha un " relazioni.

Questa confusione porta a una rigida struttura di ereditarietà e quindi sono metodi che non vengono implementati perché non vengono mai chiamati, logica ripetuta in cui le parti devono essere ottimizzate e solo in generale flussi di lavoro strani che non si allineano al business modello.

Come molti altri qui, non ho visto questo fatto maliziosamente, ma per mancanza di conoscenza del buon design e il tentativo di farlo sembrare così agli altri. Stranamente, sembra che gli sviluppatori meno esperti sappiano fare meglio a questo proposito, perché almeno il loro codice non finisce con un naseum troppo elaborato. ( principio KISS )