Bad sign se nessuno può comprendere il proprio codice? [duplicare]

53

Se un programmatore scrive un codice che nessun altro può capire, e le revisioni del codice finiscono sempre con il recensore che si gratta la testa o si tiene la testa tra le mani, è un chiaro segno che il codificatore non è semplicemente tagliato per programmazione professionale? Questo sarebbe sufficiente per giustificare un cambio di carriera? Quanto è importante il codice comprensibile in questo settore?

Considera questi esempi in C, confronta:

if (b)
  return 42;
else
  return 7;

vs

return (b * 42) | (~(b - 1) * 7);

C'è mai una scusa per utilizzare l'ultimo esempio? se sì, quando e perché?

EDIT: Lasciando in considerazione lo snippet di quest'ultimo originale, aggiungendo una correzione:

return (b * 42) | ((b - 1) & 7);

Penso che l'altra osservazione interessante sia che richiede che b sia 1 per vero e 0 per falso, qualsiasi altro valore potrebbe dare risultati strani.

    
posta Jonathan Neufeld 28.02.2015 - 01:15
fonte

9 risposte

107

La prima regola di ogni ingegnere del software professionale è scrivere un codice comprensibile. Il secondo esempio sembra un esempio ottimizzato per un compilatore più vecchio, non ottimizzante o solo per qualcuno che si vuole esprimere con operatori bit a bit. È abbastanza chiaro cosa sta succedendo se abbiamo familiarità con le operazioni bit a bit, ma a meno che non ci si trovi in un dominio in cui questo è il caso, evitare il secondo esempio. Dovrei anche sottolineare che le parentesi graffe mancano nel primo esempio.

Il programmatore potrebbe insistere sul fatto che il suo codice sia efficiente, ma potrebbe non essere scritto, perché potrebbe generare un orrendo debito tecnico lungo la strada.

    
risposta data 28.02.2015 - 01:24
fonte
83

Ci sono diverse domande che tu sollevi.

1) È un chiaro segno che il codificatore non è tagliato per la programmazione professionale?

  • Nessun. Gli sviluppatori spesso passano attraverso fasi in cui apprendono un'idea e vogliono applicarla. Applicano sempre queste idee in modo efficiente e / o efficace. No. Gli errori sono fatti e fa parte del processo di apprendimento. Se scrivono coerentemente codice incomprensibile, è necessaria una comunicazione migliore. I revisori devono comunicare quali sono le aspettative per un codice accettabile e il codificatore deve comunicare che cosa è (dovrebbe essere) il codice e, se necessario, perché è stato fatto in un modo particolare.

2) Questo sarebbe sufficiente per giustificare un cambio di carriera?

  • Non toccare questo.

3) Quanto è importante il codice comprensibile in questo settore?

  • Estremamente. Se il codice non è comprensibile, non puoi essere sicuro di cosa sta facendo, né cosa NON sta facendo. In altre parole, non hai modo di sapere se funziona correttamente.

4a) Gli esempi C:

  • Gli esempi C citati non sono equivalenti. Il secondo caso darà gli stessi risultati del primo se e solo se b è limitato ai valori di 0 e 1. Tuttavia, se passi un valore di dire 173.406.926 per b , i risultati NON corrisponderanno.

4b) C'è mai una scusa per utilizzare l'ultimo esempio?

  • Sì. Molti processori impiegano la previsione e le condotte delle filiali. Il costo di un ramo previsto in modo errato può introdurre ritardi inaccettabili. Per ottenere una risposta più deterministica, è possibile utilizzare il bit twiddling per appianare le cose.

  • Il mio esempio del secondo esempio è che è inutilmente complicato e dovrebbe essere rielaborato per chiarezza. Inoltre, non mi piace la moltiplicazione in quanto (a seconda dell'architettura) può essere lento rispetto ad altre operazioni. Molto probabilmente, preferirei vedere qualcosa del formato:

    return b ? 42 : 7;
    
  • Tuttavia, a seconda della circostanza (E se si può dimostrare che ha fornito risultati sostanzialmente migliori in categorie critiche), potrebbe accettare una macro con un nome descrittivo appropriato quando si esamina il codice. Ad esempio:

    /*
     * APPROPRIATE_MACRO_NAME - use (x) if (condition) is 1, (y) if (condition) is 0
     *
     * Parameter Restrictions:
     * (condition) - must be either 1 or 0
     * (x) and (y) are of the same integer type
     *
     * This macro generates code that avoids branching (and thus incorrectly
     * predicted branches).  Its use should be restricted to <such and such>
     * because <why and why>.
     */
    #define APPROPRIATE_MACRO_NAME(condition,x,y)  \
        (((condition - 1) & ((y) - (x))) + (x))
    

Spero che questo aiuti.

    
risposta data 28.02.2015 - 02:46
fonte
38

Il secondo codice non restituisce 42 o 7.

for b = 1:
  (1 * 42) | (~(1 - 1) * 7)
  42 | (~(0) * 7) 
  42 | (-1 * 7) 
  42 | -7
  -5

for b = 0:
  (0 * 42) | (~(0 - 1) * 7)
  0 | (~(-1) * 7) 
  0 | (0 * 7) 
  0 | 0
  0

Eppure, quando l'hai pubblicato, hai pensato che fosse così, ed è esattamente per questo che dovresti evitare il codice contorto.

Tuttavia, prendi del codice "corretto", ad esempio:

return ((b - 1) & (7 ^ 42)) ^ 42;

Ci sono due ragioni per cui posso pensare che potrebbe essere utile. - L'architettura per cui stai scrivendo non supporta la ramificazione o le istruzioni previste. - L'architettura per cui stai scrivendo ha una pipeline che non funziona oltre un'operazione di filiale o ha un costo proibitivo associato a una mancata previsione del ramo.

In questo caso dovresti scrivere qualcosa seguendo queste linee:

/* 
   This code is equivalent to:

   if (b)
      return 42;
   else
      return 7;

   when b=0 or b=1

   But does not include a branch instruction since a branch prediction
   miss here would cause an unacceptable impact to performance. 
*/

return ((b - 1) & (7 ^ 42)) ^ 42;

Se tuttavia vedi solo un codice del genere senza spiegazione o motivazione, allora probabilmente è un segno di offuscamento del codice sorgente. L'offuscamento del codice sorgente (al contrario dell'offuscamento del codice binario, che molti considererebbero avere scopi legittimi) tende ad essere nefasto. Alcuni programmatori possono essere territoriali per vari motivi, a volte per la sicurezza del lavoro e talvolta per un maggiore controllo su ciò che viene fatto e su come le cose vengono fatte a causa di ego, insicurezza o sfiducia. Tuttavia, è quasi sempre contrario agli interessi dei propri pari e del proprio datore di lavoro. È responsabilità di chi è responsabile della squadra eliminare questo comportamento prima o poi, sia costruendo la fiducia reciproca sia instillando la paura, a seconda dello stile di gestione del leader e dei metodi a cui il singolo programmatore risponde.

    
risposta data 28.02.2015 - 06:00
fonte
6

Le probabilità sono che qualcuno che scrive (b * 42) | (~(b - 1) * 7) è qualcuno che sa molto poco della programmazione cercando di far finta di essere esperto / esperto / ecc. oppure qualcuno sta cercando di sabotare un progetto (cioè sono troppo esperti / esperti / intelligenti e vogliono la sicurezza del lavoro).

Il primo tipo di persona vuole dimostrare di sapere come usare NOT, OR, l'ordine delle operazioni, ecc. Stanno mostrando le loro conoscenze, ma, ahimè, stanno scrivendo codice che è molto meno efficiente, perché questo richiede due moltiplicazioni, una sottrazione, una non, e una o, che è chiaramente meno efficiente di un confronto, un paio di salti e un ritorno.

Se è così, non meritano di essere nel settore (ancora), ma la loro dimostrazione dimostra di conoscere la logica di base del computer e potrebbe essere una risorsa preziosa un giorno, una volta superata la mostra e iniziare a scrivere codice efficiente . Inoltre, esiste la netta possibilità che b non sia necessariamente 0 o 1, il che comporterebbe la restituzione di un valore completamente diverso. Questo potrebbe introdurre bug sottili che potrebbero essere difficili da trovare.

Il secondo tipo di persona spera di inserire un codice come questo (o molti altri tipi di codice subdolo), in modo che le persone continuino a porre loro domande sul codice, quindi sono considerate troppo preziose da perdere. Questo tipo di sabotaggio finirà per danneggiare un progetto, e dovrebbero essere lasciati andare immediatamente finché non impareranno la lezione e scrivere un codice ottimizzato e di facile lettura. C'è anche la possibilità che b non sia 1 o 0, come prima, il che significa che restituirà un valore diverso dal previsto (42 o 7), che può funzionare correttamente fino a quando qualche programmatore sfortunato lo cambia in if(b) ... else ... e il il codice smette improvvisamente di funzionare. Ad esempio, forse questo è un generatore di pseudo-numeri mascherato da una dichiarazione molto costosa.

Il codice leggibile è importante, oltre che privo di codice (per quanto pratico) dai bug logici come questo. Chiunque abbia scritto un codice seriamente per un po 'sa quanto sia importante. Scrivi un gioco completamente funzionale di Tic Tac Toe. Ora, metti da parte questo codice per un anno, poi torna ad esso e prova a leggere il codice. Se l'hai scritto in modo superficiale, senza riguardo per gli standard di codifica, i commenti, la documentazione, ecc. Probabilmente non riconoscerai nemmeno che il codice che hai scritto è stato digitato da te, tanto meno come risolverlo o aggiungere una nuova funzionalità se qualcosa è stato rotto o doveva essere aggiornato.

Quando più sviluppatori lavorano insieme, è ancora più importante che il codice sia leggibile, perché le probabilità sono che non si manterrà quel codice. Ti sarai spostato su altre cose e qualcun altro dovrà mantenere il tuo codice. Al contrario, potresti ereditare un altro progetto e spero che gli sviluppatori prima di lasciare commenti e codice pulito con cui lavorare. I programmatori che lavorano su codice affidabile scrivono il codice per essere manutenibile, compresi la leggibilità e i commenti.

Le prestazioni, anche se importanti, non dovrebbero prevalere sulla leggibilità. Come minimo, se qualcuno ha usato il secondo blocco di codice qui, mi aspetterei un lungo blocco di commenti che spieghi chiaramente perché lo hanno fatto in questo modo invece che in un modo più convenzionale. A quel punto, probabilmente avrei deciso di sostituirlo con un codice più convenzionale se non ci fosse una buona ragione per farlo. Se, in effetti, fosse una bomba logica, l'avrei riscritta in un modo più lungo quindi è chiaro che la funzione deve essere quella per evitare i bug, insieme a una documentazione chiara di ciò che realmente fa.

A quanto pare, sono abbastanza sicuro che ci sia un uso legittimo per qualche problema specializzato che per coincidenza ha bisogno di questo preciso algoritmo. Se è così, però, mi aspetterei commenti che spieghino l'algoritmo che usa questa logica, e sarebbe meglio che fosse per qualcosa di meglio di un blocco if-else. Gli unici due se-else blocchi appropriati per l'esempio specifico sono: if(b) return 42; return 7; (altrimenti è facoltativo) e return b? 42: 7; (gli operatori ternari sono a posto per la logica di piccolo ramo, a meno che gli standard di codifica non lo proibiscano interamente). Qualsiasi altra cosa è eccessivamente complicata e dovrebbe essere ridotta a una dichiarazione più semplice.

Di tanto in tanto mi ritrovo a scrivere codice "difficile" che gli sviluppatori più giovani potrebbero non capire immediatamente, e di solito commento quel codice in modo che possano capire perché è stato scritto così com'era. A volte il codice ottimizzato può essere difficile da leggere e tuttavia è necessario, ma questi dovrebbero essere l'eccezione piuttosto che la regola.

C'è, per coincidenza, un posto perfettamente accettabile per codice come questo. Concorsi di offuscamento. In tal caso, riserverei il giudizio per la funzione fino a quando non avessi determinato che il codice era solo un branch davvero ingegnoso, spreco di CPU, o se era un dispositivo più subdolo per generare numeri pseudo-casuali, il valore per PI (o qualche altro numero magico), o forse anche un debole algoritmo di crittografia.

    
risposta data 28.02.2015 - 06:44
fonte
4

Se i rami in if / then / else rappresentano un problema, probabilmente è più semplice passare a qualcosa di simile:

static const int values[] = {6, 42};

return values[b!=0];

In realtà funziona e sebbene alcuni possano trovarlo marginalmente meno leggibile rispetto a if / then / else , certamente non dovrebbe essere un ostacolo evidente a chiunque conosca C o C ++.

Per tutti quelli che dovrebbero pensare che si tratti di un trucco sporco o che dipendono da controlli di tipo particolarmente lenti in una particolare lingua: sì e no. Come scritto, il codice dipende dalla conversione "false" in 0 e "true" in 1 in C e C ++.

L'idea di base, tuttavia, può essere applicata ugualmente bene anche in altri linguaggi, compresi quelli che hanno sistemi di tipo sostanzialmente più stretti. Ad esempio, in Pascal la stessa idea di base sarebbe:

var
    values : array [boolean] of Integer;

(* ... *)
values[false] = 6;
values[true] = 42;

(* ... *)
f = values[b<>0];

Il Pascal User Manual and Report (2 nd Edition) di Kathleen Jensen e Niklaus Wirth mostra l'uso di un booleano come indice in un array in un numero di luoghi, come §6.2.1 e §7. Sebbene Pascal (come originariamente definito) non includa la nozione di variabili inizializzate, se lo facesse, gli inizializzatori finirebbero nello stesso ordine in cui fanno in C e C ++ (dato che definisce il fatto che false < true ).

Ada utilizza una sintassi leggermente diversa, ma offre la stessa funzionalità di base:

Temp    : array(Boolean) of Integer := (false => 7, true=>42);

-- ...

Return_Value = Temp( 0 /= i);

Quindi no, non abbiamo a che fare con qualcosa che è solo un accidente di una lingua che capita di usare controlli di tipo particolarmente lenti. Pascal e (soprattutto) Ada sono ben noti per essere particolarmente tipizzati, ma entrambi supportano lo stesso costrutto di base di C e C ++, e lo fanno essenzialmente nello stesso modo.

    
risposta data 28.02.2015 - 04:20
fonte
3

Se mi venisse presentato un codice come questo in una revisione del codice, avrei due domande da porre:

  • Perché abbiamo scelto di scriverlo in questo modo? Poiché la manipolazione di bit come questa viene utilizzata per aggirare una sorta di collo di bottiglia delle prestazioni, si presume che si abbia un collo di bottiglia che viene rettificato se si impiega invece questo approccio. Una domanda di follow-up immediata sarebbe: "Come abbiamo dimostrato che questo era un collo di bottiglia critico?"

  • Abbiamo qualche tipo di framework di accettazione (test unitari, ecc.) che provi che l'approccio ottimizzato è equivalente all'approccio non ottimizzato? Se non , significa che suonano gli allarmi, ma non sono così rumorosi come si potrebbe pensare.

In definitiva, il codice che scrivi dovrebbe essere mantenibile . Codice che è veloce ma difficile da gestire quando si verifica un errore in esso non è un buon codice. Se fossero in grado di soddisfare entrambe le mie preoccupazioni di cui sopra, probabilmente lascerei andare con una richiesta di aggiungere sia la giustificazione che la sua forma equivalente, non bit.

In generale, una revisione del codice dovrebbe identificare e chiarire queste carenze; se non ci fosse un motivo giustificabile per scrivere il codice in questo modo, o il codice semplicemente non è corretto (come alcuni altri hanno sottolineato qui), non c'è motivo di averlo scritto in quel modo in primo luogo, e deve essere corretto . Se questa è una tendenza continua; cioè, uno sviluppatore continua a scrivere codice che è efficiente ma orrendamente offuscato, a quel punto, il loro lead o senior management dovrebbe intervenire e reiterare l'importanza del codice chiaro.

    
risposta data 01.03.2015 - 21:41
fonte
3

Qualcosa di simile al seguente sarebbe un modo per rendere più evidente l'INTENTO del codice:

manifoldPressureFloor = (b * 42) | (~(b - 1) * 7);

return manifoldPressureFloor;

manifoldPressureFloor è totalmente composto, ovviamente non ho idea di cosa sia effettivamente il codice originale.

Ma senza un qualche tipo di spiegazione o giustificazione per il codice offuscato, il revisore del codice e / o il programmatore di manutenzione è nella posizione di non avere un'idea chiara di ciò che il programmatore originale intendeva davvero realizzare , che a sua volta rende quasi impossibile provare che il codice funziona effettivamente (che in realtà fa quello che è DOVUTO fare). E rende la programmazione della manutenzione sia dolorosa che molto più probabile introdurre bug.

Non credo (senza qualifiche) che il codice possa essere completamente auto-commentante. Però; Sono assolutamente convinto che sia possibile inclinarsi pesantemente in quella direzione.

Se c'è qualche rara (o edge, performance o altro fino ad ora non specificata) la ragione per cui la sintassi (b * 42) | (~(b - 1) * 7) è giustificata, il revisore del codice non è stato in grado di coglierlo facilmente perché l'INTENTO del codice non è facilmente decifrato e apparentemente non ci sono commenti che spiegano il motivo per cui è stato fatto.

Il codice che è intelligente solo per il gusto di essere intelligente dovrebbe essere evitato. Uno degli scopi principali della scrittura di un codice chiaro è quello di garantire la comprensione umana e prevenire proattivamente l'introduzione di bachi in futuro. Il codice intelligente che non è chiaramente comprensibile è una bomba ad orologeria. Anche se oggi funziona, smetterà di funzionare dopo la manutenzione del codice domani o tra un anno. E i bug saranno esoterici e difficili da trovare e risolvere.

Come può qualcuno diverso dal programmatore originale dimostrare che il codice funzioni se l'intento e la sintassi non sono chiari? Come può anche il programmatore originale dimostrare che il codice funzioni in quel caso?

I commenti dovrebbero essere richiesti in un codice caso vizioso e intelligente. Ma è meglio se il codice è abbastanza semplice da non richiedere i commenti. Questi commenti finiscono anche per essere obsoleti o irrilevanti se il codice viene aggiornato e il programmatore di manutenzione non aggiorna o rimuove i commenti. I programmatori aggiornano regolarmente il codice senza aggiornare i commenti. Loro non dovrebbero , sono solo fanno .

Tutti questi fattori portano alla conclusione che questo programmatore deve affrontare i problemi di essere in grado di dimostrare che il codice funziona, rendere il codice comprensibile agli altri esseri umani e rendere robusto il codice . Il codice che è molto suscettibile all'introduzione di nuovi bug durante la manutenzione è fragile. Nessun datore di lavoro vuole pagare un sacco di soldi per lo sviluppo di un codice fragile e soggetto a bug.

Il codice fragile, il cui comportamento è indimostrabile, è molto più costoso di un codice pulito, chiaro, dimostrabile (testabile), di facile comprensione e di facile manutenzione. Il programmatore viene pagato da qualcun altro per essere un professionista e produrre un prodotto ragionevolmente pulito, robusto e mantenibile ad un costo ragionevole. Giusto?

Quindi, in conclusione, il programmatore che ha inventato quel codice è ovviamente intelligente e quindi pieno di potenziale. Ma il problema di essere in grado di provare la correttezza del codice, rende il codice robusto e comprensibile, da prendere sul serio. Se questo cambiamento può aver luogo, è probabile che il programmatore valga la pena continuare. Altrimenti, e il programmatore insiste a continuare a fare le cose in questo modo, potrebbe diventare difficile giustificare il fatto di mantenerle nella squadra, perché quasi certamente ti costeranno di più di quanto non riescano a farti a lungo andare. E questo è il vero problema, vero?

IMHO.

    
risposta data 28.02.2015 - 06:46
fonte
2

Sostituire if-s con espressioni aritmetiche / logiche è talvolta necessario laddove sono richieste lunghe piping del processore. Ciò fa sì che il codice esegua sempre le stesse istruzioni indipendentemente dalla condizione, rendendolo più adatto alla parallelizzazione.

Detto questo, il campione fornito è sbagliato, poiché i due campioni non sono equivalenti:

if (b)
  return 42;
else
  return 7;

può essere return ((b&1)*42)|((1-(b&1)))*7

Il programmatore del campione OP potrebbe aver sbagliato se l'eliminazione, o l'OP stesso potrebbe aver interpretato erroneamente le intenzioni del programmatore quando ha indovinato l'interruttore tradizionale if in modo errato. Dire quale delle due sia corretta è difficile non sapere quale fosse il requisito.

Si noti che l'espressione non è così "oscura": è solo una combinazione lineare nella forma P*(t)+Q*(1-t) (con t = 0..1) di cui ogni programmatore dovrebbe essere a conoscenza, poiché è la base della maggior parte di algebra lineare.

L'unica cosa che posso capire è che tra il coder e il revisore c'è una diversa base culturale sull'elaborazione parallela e l'algebra lineare. Questo IMHO, non rende l'uno superiore all'altro, se la correttezza è dimostrata.

    
risposta data 28.02.2015 - 22:51
fonte
1

Oltre agli eccellenti punti fatti da Sparky (articoli 1 e 3) e Donscarletti, questo post mi porta a un altro a cui ho risposto molto tempo fa: Come posso documentare il mio codice? .

Molte persone sono o si definiscono programmatori, alcune sono buone, non molte sono eccellenti. Proprio come in molti altri ambiti della vita. Puoi decidere di giudicare quelli che appaiono meno buoni di te o meno buoni di quelli che ti aspetteresti (non molto utili, perdita di tempo), puoi provare ad aiutarli (bene) o costringerli a fare meglio ( potresti non avere scelta), o ignorarli e fare semplicemente del tuo meglio (potresti non avere scelta, a volte questo è il percorso migliore). Qualunque sia la tua scelta di azione, generalmente dipende da molti fattori ben oltre la semplice abilità tecnica.

    
risposta data 02.03.2015 - 03:58
fonte

Leggi altre domande sui tag