Quali elementi suonano all'istante le campane quando guardano il codice? [chiuso]

Codice C ++ con istruzioni di eliminazione esplicite (a meno che non stia guardando l'interno di un'implementazione di puntatore intelligente). 'delete' è il 'goto' della gestione della memoria IMHO .

Strettamente correlato a questo, codice come:

// Caller must take ownership
Thing* Foo::Bar()

(E ti consideri fortunato se c'è il commento!). Non è come puntatori intelligenti sono scienza missilistica. std::auto_ptr è fatto per questo genere di cose (documentando e rafforzando l'intento espresso nel commento) ed è stato parte di standard ormai da anni.

Insieme questi urlano codice legacy non amato, o manutentori con una mentalità bloccata da qualche parte nei primi anni '90.

Quando non ci sono commenti o documentazione di qualunque cosa faccia o faccia il codice (ad esempio "il codice è la documentazione").

Metodi o variabili con un numero come suffisso (ad es. Login2 ()).

ON ERROR RESUME NEXT

Il dover mantenere le applicazioni ASP classiche è purtroppo una necessità per la maggior parte degli sviluppatori ASP.NET, ma aprire un file di inclusione comune e vedere che sulla prima riga distrugge l'anima.

Codice che non può mai, immettere logicamente il percorso di esecuzione.

var product = repository.FindProduct(id);

log.Info("Found product " + product.Name);

if (product != null)
{
    // This will always happen
}
else
{
    // **This won't ever happen**
}

o

if (messages.Count > 0)
{
    // Do stuff with messages
}
else if (messages.Count == 1)
{
    // **I hope this code isn't important...**
}

try
{
//do something
}
catch{}

Dalla mia prospettiva Java-centrica:

• Stile di codifica non standard.
• Variabili non private.
• Manca final sui campi.
• Inutile o eccessivo uso dell'ereditarietà.
• Enormi classi o blocchi di codice.
• Troppi commenti (probabilmente sono solo un pio desiderio)
• Registrazione non strutturata.
• Getter e setter (le interfacce dovrebbero riguardare il comportamento)
• Dati duplicati.
• Strane dipendenze.
• Statics, incluse le globali dei thread.
• Per codice multi-thread, parti della stessa classe che dovrebbero essere eseguite in thread diversi (in particolare codice GUI).
• Codice guasto.
• Manipolazione delle stringhe combinata con altro codice.
• Generalmente miscelazione di livelli (materiale di livello superiore, combinato con iterazione su una matrice di primitive o gestione dei thread, per esempio).
• Qualsiasi uso della riflessione.
• catch di blocchi senza codice utile (non valido: commenti, printf, registrazione o semplicemente vuoto).

• Mettere ogni variabile locale nel prime linee di prova del blocco del metodo. Soprattutto in combinazione con lungo metodi.
• Uso delle variabili booleane per interrompere i loop / saltare le iterazioni invece di usare solo break / continue

Il nostro codice VB6 legacy, puoi aprire qualsiasi pagina di codice modulo o modulo e trovare uno schermo pieno di # pubblico o globale ! variabili dichiarate in alto, a cui si fa riferimento da tutto il @ &! ! (programma *! #. ARGH !!!!

(Wow, ho dovuto tirarlo fuori :-))

Per quanto riguarda i numeri magici: sono cattivi se vengono utilizzati in luoghi diversi e cambiandolo è necessario sincronizzarli in più punti. Ma un numero in un posto non è peggio che avere una costante per denotare un numero che è ancora usato in un posto.

Inoltre, le costanti potrebbero non avere molto spazio nella tua applicazione. In molte app di database, queste cose dovrebbero essere memorizzate nel database in base all'app o alle impostazioni dell'utente. E per completare la loro implementazione coinvolgono questa impostazione e un posto nell'iu e una nota nella documentazione dell'utente finale ... tutto questo è gentile, una sovrastampa e uno spreco di risorse se tutti sono perfettamente felici quando il numero è 5 ( e 5 è.)

Penso che sia possibile lasciare numeri e stringhe finché non sarà necessario utilizzare questo numero al di fuori di quel luogo. Quindi è il momento di ridefinire le cose con un design più flessibile.

Per quanto riguarda le stringhe ... Conosco gli argomenti, ma questo è un altro punto in cui non è necessario eseguire una conversione costante da una stringa all'altra. Soprattutto se le stringhe in atto provengono comunque da un'implementazione costante (ad esempio, le importazioni generate da un'applicazione esterna e hanno una stringa di stato breve e riconoscibile, proprio come "Overdue"). molto altro nel convertire il valore "Overdue" in STATUS_OVERDUE quando viene usato comunque in un solo posto.

Sono moltissimo per non aggiungere complessità se non crea effettivamente i benefici necessari in termini di flessibilità, riutilizzo o controllo degli errori. Quando hai bisogno della flessibilità, codificalo correttamente (il refactory thingy). Ma se non è necessario ...

Qualcosa di simile

x.y.z.a[b].c

Questo odora di bio-hazard. Questo riferimento ai membri non è mai un buon segno. E sì, questa è un'espressione tipica del codice con cui lavoro.

Utilizzare un oggetto nascosto nell'interfaccia utente (ad es. una casella di testo) per memorizzare un valore piuttosto che definire una variabile con ambito appropriato e -tipo.

Codice che mostra che il programmatore non si è mai adattato, anni fa, a Java 5:

• Uso di Iterators anziché "per ogni"
• Non usare i farmaci generici nelle raccolte e il cast degli oggetti recuperati nel tipo previsto
• Uso di classi antiche come Vector e Hashtable

Non conoscendo i moderni modi con multithreading.

Ogni volta che leggo quanto segue:

//Don't put in negative numbers or it will crash the program.

O qualcosa di simile. Se questo è il caso, allora metti un asserito! Fai sapere al debugger che durante l'esecuzione non vuoi quei valori e assicurati che il codice spieghi il contratto con il chiamante.

qualsiasi cosa con qualcosa di simile

// TODO: anything could be here!

Modifica : dovrei qualificarmi per ciò che intendevo nel codice di produzione. Ma anche nel codice dedicato al controllo del codice sorgente questo non è ancora valido. Ma quella potrebbe essere una cosa personale in quanto mi piace finire il giorno libero dopo avermi legato tutte le mie forze:)

Modifica 2 : dovrei ulteriormente qualificare che intendevo quando vedo questo nel codice stabilito. Come qualcosa che ha diversi anni e che sto correggendo un bug. Vedo un TODO e questo è quando le campane di allarme iniziano a squillare. TODO (per me) implica che il codice non è mai stato completato per qualche motivo.

Per SQL:

Il primo grande indicatore è l'uso di join impliciti.

Il prossimo è l'uso di un join sinistro sulla tabellaB combinato con una clausola WHERE come:

WHERE TableB.myField = 'test'

Se non sai che darà risultati errati, non posso fidarmi che qualsiasi query scritta fornirà risultati corretti.

Codice strettamente accoppiato. Soprattutto quando vedi un sacco di cose hardcoded (nomi di stampanti di rete, indirizzi IP, ecc.) Nel mezzo del codice. Questi dovrebbero essere in un file di configurazione o costanti, ma il seguente causerà problemi alla fine:

if (host_ip == '192.168.1.5'){
   printer = '192.168.1.123';
} else
  printer = 'prntrBob';

Un giorno, Bob sta per uscire e la sua stampante verrà rinominata. Un giorno la stampante riceverà un nuovo indirizzo IP. Un giorno 192.168.1.5 vorrà stampare sulla stampante di Bob.

il mio mantra preferito: scrivi sempre codice come uno psicopatico omicida che sa dove vivi dovrai mantenerlo!

Questo tipo di codice:

        if (pflayerdef.DefinitionExpression == "NM_FIELD = '  '" || One.Two.nmEA == "" || 
            One.Two.nmEA == " " || One.Two.nmEA == null ||
            One.Two.nmEA == "  ")
        {                
            MessageBox.Show("BAD CODE");
            return;
        }

Questo deriva da una vera base di codice di produzione dal vivo!

Quando il codice sembra un disastro: commenti con "todo" e "note to self" e battute zoppicanti. Codice che è stato ovviamente utilizzato in qualche punto esclusivamente per scopi di debug, ma non è stato quindi rimosso. Variabili o funzioni con nomi che suggeriscono che lo scrittore non ha ritenuto che qualcuno avrebbe letto in seguito. Spesso questi nomi saranno molto lunghi e poco maneggevoli.

Inoltre: se il codice non ha ritmo. Funzioni di lunghezza selvaggiamente divergente. Funzioni che non aderiscono agli stessi schemi di denominazione.

Leggermente correlato: mi rende sempre nervoso se un programmatore ha una calligrafia sbiadita. O se sono cattivi scrittori o cattivi comunicatori.

Tutto ciò che viola principi che sono importanti. Ad esempio, sono stati suggeriti alcuni anti-pattern (numero magico - vedi link ). Gli anti-schemi sono chiamati così perché causano problemi (anche già citati - fragilità, incubi di manutenzione, ecc.). Inoltre, fai attenzione alla violazione dei principi SOLID - vedi link Inoltre, codice che non considera la separazione dei livelli (dati che accedono a cose all'interno dell'interfaccia utente, ecc.). Avere standard di codifica e recensioni del codice aiutano a combattere questo.

Una volta ho lavorato a un progetto in cui l'appaltatore aveva eliminato tutti i tipi di dati standard da int a string compresi i puntatori a nomi oscuri. Il suo approccio ha reso la comprensione del codice davvero difficile. Un altro stile che mi avvisa è la flessibilità prematura; un prodotto su cui ho lavorato una volta aveva il codice completo in DLL che non venivano caricate in un ordine prevedibile. Tutto questo per accogliere l'evoluzione futura. Alcune DLL utilizzavano wrapper di thread per la portabilità. È stato un incubo eseguire il debug del programma o prevedere il flusso leggendo il codice sorgente. Le definizioni erano sparse tra i file di intestazione. Non sorprende che il codice non sia sopravvissuto oltre la seconda versione.

A volte vedo parti di un metodo che ha fornito tutti gli input possibili, non funzionerebbe MAI in alcun modo, quindi non dovrebbe essere lì a confondere le persone in primo luogo. Un esempio potrebbe essere ...
Se il metodo può essere chiamato solo nel contesto di un superutente Admin e vedo qualcosa che verifica se l'utente della richiesta non è un superutente amministratore ...: /

Ottimizzazione degli spioncini sul codice che potrebbe essere ottimizzata con una struttura migliore, ad es. ricerche lineari implementate in assembly inline quando una ricerca binaria in plain C / C ++ / Java / C # sarebbe appropriata (e più veloce).

O alla persona mancano alcuni concetti fondamentali o non ha alcun senso di priorità. Quest'ultimo è molto più preoccupante.

L'uso della parola chiave synchronized in Java.

Non c'è niente di sbagliato nell'usare synchronized quando viene usato correttamente. Ma nel codice con cui lavoro, sembra che ogni volta che qualcuno cerca di scrivere codice thread-safe, sbagliano. Quindi so che se vedo questa parola chiave, devo fare molta attenzione al resto del codice ...

• $ data - È come la pubblicità "Oggetti fisici, ora a un minimo ridicolo 100 per 5 ! "
• TODO articoli nel codice: utilizza un tracker bug / ticket / issue in modo che le persone sappiano cosa è necessario a livello di prodotto anziché a livello di file.
• Codice di accesso al lavoro - Ecco a cosa serve il controllo della versione.

La maggior parte di questi proviene dall'esperienza Java:

• Digitazione delle stringhe. Solo ... no.
• Il typecasting punta spesso a un odore di codice nella moderna Java.
• L'anti-pattern di Pokemon Exception (quando devi prenderli tutti).
• Tentativi di caricamento del carico nella programmazione funzionale in cui non è appropriato.
• Non usare un costrutto simile a funzioni (chiamabile, funzione, ecc.) dove sarebbe appropriato.
• Fallimenti per sfruttare il polimorfismo.

Commenti insoddisfatti che dimostrano la mancanza di moderazione:

//I CAN'T GET THIS F^#*&^ING PIECE OF S$^* TO COMPILE ON M$VC

O sono di cattivo umore o non hanno esperienza sufficiente per aver imparato che le battute d'arresto sono inevitabili nella programmazione.

Non voglio lavorare con persone del genere.

@FinnNk, sono d'accordo con te riguardo al codice commentato. Quello che veramente mi infastidisce è roba del genere:

for (var i = 0; i < num; i++) {
    //alert(i);
}

o tutto ciò che era lì per test o debugging, ed è stato commentato e poi commesso. Se è solo occasionale, non è un grosso problema, ma se è ovunque, ingombra il codice e rende difficile vedere cosa sta succedendo.

Questo è un sintomo un po 'minore rispetto alle cose che altri hanno elencato, ma io sono un programmatore Python e spesso lo vedo nella nostra base di codice:

try:
    do_something()
except:
    pass

Che di solito mi segnala che il programmatore non sapeva davvero (o si preoccupava) perché do_something potesse fallire (o quello che fa) - semplicemente continuava a "armeggiare" fino a quando il codice non si arrestava più.

Per quelli che provengono da uno sfondo più java, quel blocco è l'equivalente Python di

try {
    doSomething();
} catch (Exception e) {
    // Don't do anything. Don't even log the error.
}

Il fatto è che Python usa le eccezioni per il codice "non eccezionale", come le interruzioni di tastiera o l'interruzione di un ciclo for.

e una cosa speciale: i getter che delegano ad altri getter.

questo non va bene perché significa che la persona che ha scritto non capisce la base di object-oriented, che è incapsulamento, il che significa dove sono i dati, dovrebbero essere i metodi che agiscono su quei dati.

la delega è per un metodo non tutti i getter. il principio è "raccontare, non chiedere"; Dì una cosa a un oggetto da fare, non chiedermelo per mille cose e poi fallo da solo.

I Getters mi fanno impazzire, perché significa che altri principi di Oop saranno violati.

Informazioni sul tipo mancante.

Dai un'occhiata a queste firme dei metodi:

1. public List<Invoice> GetInvoices(Customer c, Date d1, Date d2)

2. public GetInvoices(c, d1, d2)

In (1) c'è chiarezza. Sai esattamente con quali parametri hai bisogno di chiamare la funzione ed è chiaro cosa restituisce la funzione.

In (2) c'è solo incertezza. Non hai idea di quali parametri utilizzare e non sai cosa restituisce la funzione se qualcosa. Sei effettivamente costretto a utilizzare un approccio di prova ed errore inefficiente alla programmazione.