Come posso suggerire con tatto miglioramenti al codice mal progettato di altri durante la revisione?

1. Controlla la tua motivazione. Se pensi che il codice debba essere cambiato, dovresti essere in grado di articolare un motivo per cui perché pensi che dovrebbe essere cambiato. E quella ragione dovrebbe essere più concreta di "avrei fatto diversamente" o "è brutta". Se non puoi indicare alcuni benefici derivanti dal tuo cambiamento proposto, allora non ha molto senso passare il tempo (a.k.a. soldi) nel cambiarlo.

2. Ogni riga di codice nel progetto è una linea che deve essere mantenuta. Il codice dovrebbe essere lungo quanto deve essere per portare a termine il lavoro ed essere facilmente comprensibile, e non più. Se riesci ad abbreviare il codice senza sacrificare la chiarezza, va bene. Se riesci a farlo mentre aumenti la chiarezza, è molto meglio.

3. Il codice è concreto: è più difficile cambiare dopo che è rimasto seduto per un po '. Suggerire le modifiche in anticipo se possibile, in modo che i costi e il rischio di modifiche siano entrambi ridotti al minimo.

4. Ogni cambiamento costa denaro. Riscrivere il codice che funziona ed è improbabile che debba essere modificato potrebbe essere uno sforzo inutile. Focalizza l'attenzione sulle sezioni che sono più soggette a modifiche o che sono più importanti per il progetto.

5. La forma segue la funzione, e talvolta viceversa. Se il codice è disordinato, c'è una maggiore probabilità che contenga anche bug. Cerca quegli errori e critica le funzionalità difettose piuttosto che l'aspetto estetico del codice. Suggerisci miglioramenti che facciano funzionare meglio il codice e per semplificare l'operazione del codice.

6. Distingue tra design e implementazione. Una classe importante con un'interfaccia schifosa può diffondersi attraverso un progetto come il cancro. Non solo diminuirà la qualità del resto del progetto, ma aumenterà anche la difficoltà di riparare il danno. D'altra parte, una classe con un'interfaccia ben progettata ma un'implementazione pessima non dovrebbe essere un grosso problema. È sempre possibile ri-implementare la classe per prestazioni migliori o affidabilità. Oppure, se funziona correttamente ed è abbastanza veloce, puoi lasciarlo da solo e sentirti sicuro sapendo che il suo cruft è ben incapsulato.

Per riepilogare tutti i punti precedenti: Assicurati che le modifiche proposte aggiungano valore.

C'è un punto debole per aggiungere valore attraverso il refactoring. Le modifiche devono portare a termine tre cose:

• migliora il codice che è probabile che cambi
• aumentare la chiarezza
• minimo sforzo

Considerazioni:

1. Sappiamo che il codice pulito è meno costoso da scrivere e mantenere ed è più divertente su cui lavorare. Il tuo compito è vendere quell'idea alle persone della tua azienda. Pensa come un venditore, non come un brontolone arrogante (cioè non come me).
2. Non puoi vincere, puoi solo perdere meno.
3. Concentrati sull'aggiunta di valore reale, non solo sulla bellezza. Mi piace che il mio codice abbia un bell'aspetto, ma a volte devo accettare questo aspetto più economico.
4. Un buon modo per trovare il punto giusto è seguire il Principio di Boy Scout - quando lavori su un'area del codice, lascialo sempre in una forma migliore di quella che hai trovato.
5. Un piccolo miglioramento è migliore di nessun miglioramento.
6. Fai buon uso degli strumenti automatici. Ad esempio, gli strumenti che eliminano un po 'di formattazione possono rendere un mondo di differenza.
7. Vendi altre idee che migliorano incidentalmente la chiarezza del codice. Ad esempio, i test unitari incoraggiano la decomposizione di grandi metodi in metodi più piccoli.

Se il codice funziona senza errori gravi e una scadenza importante (come negli effetti P & L o PR aziendale) è imminente, è troppo tardi per suggerire miglioramenti che richiedono importanti cambiamenti. Anche i miglioramenti apportati al codice possono creare rischi per l'implementazione del progetto. Il tempo per i miglioramenti era precedente nel progetto, quando c'era più tempo per investire nella robustezza futura della base di codice.

La revisione del codice ha 3 scopi:

1. Verifica bug

2. Controllo per vedere dove è possibile migliorare il codice

3. Strumento didattico per chiunque abbia scritto il codice.

La valutazione del design / della qualità del codice sono ovviamente circa # 2 e # 3.

Fino a # 2:

• Rendi MOLTO chiaro quali sono i benefici derivanti dalle modifiche proposte rispetto ai costi da correggere. Come ogni decisione aziendale, questo dovrebbe riguardare l'analisi costi / benefici.

  es. "L'approccio X al design ridurrebbe in modo significativo la probabilità che il bug Y si verificasse quando si eseguono i cambiamenti Z, e sappiamo che questo pezzo di codice subisce cambiamenti di tipo Z ogni 2 settimane Il costo di gestire l'interruzione della produzione dall'insetto Y + trovare l'errore + fissare e rilasciare la correzione + costo opportunità dal mancato invio della prossima serie di imprese è $A , mentre il costo di ripulire il codice ora e il costo opportunità (ad es. prezzo di spedizione in ritardo o con meno funzioni) è $B . , valuta - o meglio il tuo team leader / manager - valuta $A vs $B e decidi.

  • Questo aiuterà il team intelligente a gestire efficacemente questo. Per esempio. prenderanno una decisione razionale usando le informazioni COMPLETE

  • Ciò avverrà (specialmente se pronunci bene questa frase) per aumentare il TUO stato - ad es. sei una persona abbastanza intelligente da vedere i benefici di un design migliore, E abbastanza intelligente da non richiederlo religiosamente senza pesare considerazioni di business.

  • E, nel probabile evento che si verifica l'errore Z, guadagni molto di più sulla prossima serie di suggerimenti.

Fino a # 3:

• MOLTO chiaramente delineato "deve correggere" bug / problemi da "Questa è una best practice e dovrebbe essere risolta SE possiamo risparmiare risorse - vedi allegati pro / con analisi" miglioramenti del design (allegare le cose descritte al punto 2 sopra ) vs "Queste sono linee guida generali che penso possano aiutarti a migliorare la robustezza del codice in modo da poter mantenere più facilmente il codice" modifiche facoltative. Si prega di notare la formulazione - non si tratta di "rendere il codice come quello che voglio" - è "se lo fai, ottieni benefici a, b, c". Il tono e l'approccio contano.

Scegli le tue battaglie, se arriva una scadenza quindi non fare nulla. La prossima volta che qualcun altro sta rivedendo o mantenendo il codice e continuano a riscontrare problemi con esso, gli si avvicina con l'idea che, come team, dovresti concentrarti maggiormente sulla qualità del codice durante la revisione del codice, in modo da non avere così tanti problemi in seguito.

Dovrebbero vedere il valore prima di eseguire il lavoro extra.

Inizio sempre il mio commento con "I would", che segnala che il mio è semplicemente una delle tante visualizzazioni.

Includo sempre anche un motivo.

" Vorrei estrapolare questo blocco in un metodo perché di leggibilità."

commento su tutto; grande e piccolo. A volte faccio più di un centinaio di commenti su un cambiamento, nel qual caso raccomando anche la programmazione delle coppie, e mi offro come wing-man.

Sto cercando di stabilire un linguaggio comune per ragioni; leggibilità, DRY, SRP, ecc.

Ho anche creato una presentazione su Clean Code e Refactoring spiegando perché e mostrando come, che ho tenuto ai miei colleghi. L'ho tenuto tre volte finora, e una società di consulenza che stiamo usando mi ha chiesto di tenerlo di nuovo per loro.

Ma alcune persone non ascolteranno comunque. Poi mi rimane il grado di tirare. Sono il capo del design. La qualità del codice è una mia responsabilità. Questo cambiamento non avverrà nel mio stato attuale.

Tieni presente che sono più che disposto a rinunciare a qualsiasi commento che faccio; per motivi tecnici, scadenze, prototipi, ecc. Ho ancora molto da imparare sulla programmazione e ascolterò sempre la ragione.

Oh, e di recente mi sono offerto di comprare il pranzo al primo della mia squadra che ha presentato una modifica non banale su cui non avevo nessun commento . (Ehi, devi divertirti anche tu.: -)

[Questa risposta è un po 'più ampia della domanda originariamente posta, poiché questo è il reindirizzamento per tante altre domande sulle revisioni del codice.]

Ecco alcuni principi che trovo utili:

Critica in privato, elogia pubblicamente. Fai conoscere a qualcuno un bug nel codice uno contro uno. Se hanno fatto qualcosa di geniale o hanno intrapreso un'attività che nessuno voleva, lodali in una riunione di gruppo o in una email inviata alla squadra.

Condividi i tuoi stessi errori. Condivido la storia della mia disastrosa prima revisione del codice (ricevuta) con studenti e colleghi junior. Ho anche fatto sapere agli studenti che ho preso il loro errore così velocemente perché l'ho fatto prima di me stesso. In una revisione del codice, questo potrebbe apparire come segue: "Penso che le variabili dell'indice siano sbagliate qui, lo controllo sempre a causa del tempo in cui ho sbagliato i miei indici e fatto cadere un data center". [Sì, questa è una storia vera.]

Ricordati di fare commenti positivi. Un breve "bello!" o "trucco pulito!" può rendere il giorno di un programmatore junior o insicuro.

Supponiamo che l'altra persona sia intelligente ma a volte incurante. Non dire: "Come prevedi che il chiamante ottenga il valore restituito se in realtà non lo restituisci ?!" Di ': "Sembra che tu abbia dimenticato la dichiarazione di reso". Ricorda che hai scritto un codice orribile nei tuoi primi giorni. Come qualcuno ha detto una volta, "Se non ti vergogni del tuo codice di un anno fa, non stai imparando."

Salva il sarcasmo / il ridicolo per gli amici non sul posto di lavoro. Se il codice è terribilmente terribile, scherza su di esso altrove. (Trovo comodo essere sposati con un collega programmatore.) Ad esempio, non condividerei i seguenti fumetti (o questo ) con i miei colleghi.

I sometimes find myself in a situation where a peer's code is riddled with messy design and very little concern for future maintenance, though it's functional and contains little to no bugs.

Questo codice è fatto. A un certo punto, le riprogettazioni diventano troppo costose per giustificare. Se il codice è già funzionante con pochi o nessun bug, allora questa sarà una vendita impossibile. Suggerisci alcuni modi per ripulirlo in futuro e andare avanti. Se / quando il codice si rompe in futuro, rivalutare il valore di una riprogettazione allora. Potrebbe non rompersi mai, il che sarebbe fantastico. Ad ogni modo, sei nel punto in cui ha senso scommettere che non si romperà, dal momento che il costo sarà lo stesso ora o più tardi: estenuante, terribile riprogettazione.

Ciò che devi fare in futuro è avere iterazioni di sviluppo più serrate. Se fosse stato possibile rivedere questo codice prima che tutto il lavoro di stirare i bug fosse stato investito, avrebbe avuto senso suggerire una riprogettazione. Verso la fine, non ha mai senso eseguire dei refactoring importanti a meno che il codice non sia scritto in modo fondamentalmente intrattabile e si è certi che il codice dovrà essere modificato subito dopo il rilascio.

Data la scelta tra le due opzioni (refactoring o no refactor), pensa a quali suoni come la vendita più intelligente:

Hey boss, we were on schedule and had everything working, but now we are going to rebuild a lot of stuff so that we can add feature X in the future.

o

Hey boss, we are ready to release. If we ever have to add on feature X, it might take us a couple extra days.

Se hai detto uno dei due, il tuo capo probabilmente direbbe:

Who said anything about feature X?

La linea di fondo è che a volte un po 'di debito tecnico ha senso, se non si è in grado di correggere alcuni difetti quando era economico (le iterazioni iniziali). Il design del codice di qualità ha rendimenti decrescenti man mano che ti avvicini a una funzione completata e alla scadenza.

Quando un cucchiaio di zucchero aiuta la medicina a cadere, e ciò che è sbagliato può essere espresso in modo succinto - non ci sono 20 cose sbagliate - ti condurrò con una forma che suggerisce che non ho alcun interesse, nessun ego investito in quello che voglio essere ascoltato Di solito è qualcosa di simile:

I wonder if it would be better to...

o

Does it make any sense to...

Se le ragioni sono abbastanza ovvie, non le dichiaro. Ciò dà ad altre persone la possibilità di assumere una proprietà intellettuale del suggerimento, come in:

"Sì, è una buona idea, perché < il tuo ovvio motivo qui >."

Se il miglioramento è abbastanza ovvio, ma non tanto da farmi sembrare un idiota per non pensarci, e il motivo per farlo riflette un valore condiviso con l'ascoltatore, quindi a volte non lo suggerisco nemmeno , invece:

Mi chiedo se c'è un modo per ... < dichiarazione di valore condiviso qui >

Questo è solo per trattare con persone davvero permalose - con la maggior parte dei miei colleghi, glielo faccio avere!

Le revisioni del codice non sempre mirano a migliorare.

Una recensione prossima alla fine di un progetto, come sembra essere il caso qui, è solo perché tutti sappiano da dove cominciare a guardare quando arrivano i bug (o per un progetto meglio progettato che può essere disponibile per un successivo riutilizzo). Qualunque sia il risultato della revisione, semplicemente non è il momento di cambiare nulla.

Per apportare effettivamente le modifiche devi discutere del codice e del design molto prima nel progetto - il codice è molto più facile da cambiare quando esiste solo come parlare di possibili approcci.

La tua domanda è "Come eseguire la revisione del codice codice mal progettato?":

La risposta IMO è semplice. Parlate del DESIGN del codice e di come il design è difettoso o non soddisfa i requisiti. Se fai notare un design difettoso o "non soddisfa il requisito", lo sviluppatore sarà costretto a cambiare il suo codice perché non fa ciò che deve fare.

Se il codice è "funzionalmente sufficiente" e / o "soddisfa le specifiche" e / o "soddisfa i requisiti":

Se sei un collega di questo sviluppatore, non hai alcun potere diretto che ti permetta di "dirgli" di apportare modifiche.

Ci sono un paio di opzioni a tua disposizione:

1. Devi usare la tua "influenza" personale (una forma di "potere" indiretto) e / o la tua capacità di essere "persuasivo"
2. entrare in contatto con il gruppo "codice in codice" dell'organizzazione e iniziare a rendere "la manutenzione del codice" una priorità più alta.
3. Morde il proiettile e impara a leggere il codice schifoso più veloce / più fluente, in modo da non rimanere agganciato (sembra che tu continui a rimanere agganciato o rallentato quando incontri un codice scadente) su un codice scadente.
   • Questo ti renderà anche un programmatore più strong.
   • E ti permetterà di correggere il codice schifoso quando lavori su codice scadente.
   • E questo ti permetterà anche di lavorare su più progetti perché molti progetti hanno un codice scadente che è funzionale ... ma un sacco di codice scadente.
4. Guida con l'esempio. Rendi il tuo codice migliore ... ma non cercare di essere un perfezionista.
   • Perché poi diventerai "il ragazzo lento che non può rispettare le scadenze, è sempre critico e pensa di essere migliore di chiunque altro".

Trovo che non ci sia un proiettile d'argento. Devi usare tutti e tre e devi essere creativo nell'uso di tutti e tre.

In caso di design rovinoso, il tuo obiettivo dovrebbe essere quello di massimizzare l'incapsulamento . In questo modo, diventa più facile sostituire le singole classi / file / subroutine con classi progettate meglio.

Concentrarsi sull'assicurare che le interfacce pubbliche dei componenti siano ben progettate e che i meccanismi interni siano accuratamente nascosti. Inoltre, i wrapper Data Storage sono essenziali. (Grandi quantità di dati memorizzati possono essere molto difficili da cambiare, quindi se ottieni "bleed di implementazione" in altre aree del sistema, sei nei guai).

Una volta che hai superato le barriere tra i componenti, concentrati sui componenti che hanno maggiori probabilità di causare problemi importanti.

Ripeti fino alla scadenza o finché il sistema non è "perfetto".

Invece di criticare in modo diretto e diretto il codice di qualcuno, è sempre meglio essere cosntructive nei nostri commenti durante la revisione del codice.

Un modo che seguo è

1. sarà ottimale se lo facciamo in questo modo.
2. Scriverlo in questo modo lo renderà più veloce.
3. Il tuo codice sarà molto più leggibile se fai "questo" "questo" e "questo"

Tali commenti saranno presi sul serio anche se le tue scadenze sono in arrivo. E probabilmente sarà implementato nel prossimo ciclo di sviluppo.

La revisione del codice deve essere integrata con il ciclo di cultura e sviluppo per funzionare. Non è probabile che la pianificazione di una grande revisione del codice alla fine dello sviluppo della feature X funzionerà. Prima di tutto, rendere le modifiche più difficili e qualcuno potrebbe sentirsi imbarazzato, creando resistenza all'attività.

Dovresti avere commit precoci e frequenti, insieme a recensioni a livello di commit. Con gli strumenti di analisi del codice in atto, la maggior parte delle recensioni sarà veloce. Strumenti di analisi / revisione del codice automatizzati come FindBugs e PMD ti aiuterà ad ottenere una grande classe di errori di progettazione fuori dalla porta. Tuttavia, non ti aiuteranno a risolvere i problemi a livello di architettura, quindi devi disporre di un solido design e valutare il sistema in generale rispetto a tale progetto.

Aumenta la qualità delle recensioni del codice.

Oltre alla qualità del codice in esame, esiste una qualità della stessa revisione del codice:

• Le modifiche proposte sono davvero un miglioramento rispetto a ciò che è presente?
• O solo una questione di opinione?
• A meno che qualcosa di molto ovvio, il recensore abbia spiegato correttamente, perché ?
• Quanto tempo ci è voluto? (Ho visto recensioni durate per mesi, con lo sviluppatore in fase di revisione incaricato di risolvere tutti i numerosi conflitti di fusione).
• Toni?

È molto più facile accettare una revisione del codice di buona qualità rispetto ad alcune tecniche di pulizia dell'ego per lo più discutibili.

Ci sono due problemi notevoli nella domanda, la parte con tatto e la parte in scadenza . Si tratta di problemi distinti: il primo è una questione di comunicazione e dinamiche di gruppo, il secondo è una questione di pianificazione e definizione delle priorità.

Con molto tatto . Presumo che tu voglia evitare l'ego spazzolato e il pushback negativo contro le recensioni. Alcuni suggerimenti:

• Avere una comprensione condivisa su standard di codifica e principi di progettazione.
• Non criticare o recensire mai lo sviluppatore , solo il codice . Evita la parola "tu" o "il tuo codice", parla solo del codice in esame, disconnesso da qualsiasi sviluppatore.
• Metti il tuo orgoglio nella qualità del codice completato , in modo tale che i commenti di revisione che aiutano a migliorare il risultato finale siano apprezzati.
• Suggerisci miglioramenti piuttosto che la domanda. Parla sempre se non sei d'accordo. Cerca di capire l'altro punto di vista quando non sei d'accordo.
• Le recensioni vanno in entrambe le direzioni. Cioè chiedi alla persona che hai recensito di rivedere il tuo codice. Non avere recensioni "a senso unico".

La seconda parte è la definizione delle priorità . Hai molti suggerimenti per miglioramenti, ma da quando si avvicina la scadenza c'è solo il tempo di applicarne alcuni.

Bene, in primo luogo vuoi evitare che ciò accada in primo luogo! Lo fai eseguendo revisioni continue e incrementali. Non lasciare che uno sviluppatore lavori per settimane su una funzione e poi riesamini tutto all'ultimo momento. In secondo luogo, le revisioni del codice e il tempo necessario per implementare i suggerimenti di revisione dovrebbero far parte della pianificazione e delle stime regolari per qualsiasi attività. Se non c'è abbastanza tempo per rivedere correttamente, qualcosa è andato storto nella pianificazione.

Ma ipotizziamo che qualcosa sia andato storto nel processo, e ora ti trovi di fronte a una serie di commenti di revisione, e non hai il tempo di implementarli tutti. Devi dare la priorità. Poi vai per le modifiche che saranno più difficili e più rischiose da modificare in seguito se la rimandi.

La denominazione di identificatori nel codice sorgente è incredibilmente importante per la leggibilità e la gestibilità, ma è anche piuttosto facile e a basso rischio di cambiare in futuro. Lo stesso con la formattazione del codice. Quindi non concentrarti su quella roba. D'altro canto, la sanità mentale delle interfacce esposte pubblicamente dovrebbe essere la massima priorità, dal momento che sono davvero difficili da cambiare in futuro. I dati persistenti sono difficili da cambiare: se in un primo momento si inizia a memorizzare dati inconsistenti o incompleti in un database, è molto difficile correggerli in futuro.

Le aree coperte dai test unitari sono a basso rischio. Puoi sempre sistemarli più tardi. Le aree che non lo sono, ma che potrebbero essere unità testate sono a rischio inferiore rispetto alle aree che non possono essere sottoposte a test dell'unità.

Supponiamo che tu abbia una grossa porzione di codice senza test unitari e tutti i tipi di problemi di qualità del codice, inclusa una dipendenza hardcoded su un servizio esterno. Invece, iniettando questa dipendenza, si rende il chunk testabile del codice. Ciò significa che in futuro potrai aggiungere test e quindi lavorare per risolvere il resto dei problemi. Con la dipendenza hardcoded non puoi nemmeno aggiungere test. Quindi, prima cerca questa soluzione.

Ma per favore cerca di evitare di finire in questo scenario in primo luogo!