Estrarre sempre casi comuni e diramazioni separatamente? [duplicare]

Ci sono due ragioni per le quali preferirei la seconda alla prima. Il primo è la ripetizione: ti ripeti che può portare a un codice più difficile da mantenere. Il secondo è che penso che il secondo sia più chiaro quando viene eseguito ogni blocco di codice. Se eseguo il debug e so che previouslyCalculatedIndex != -1 , quindi posso saltare immediatamente due interi blocchi di codice nella seconda semplicemente controllando uno condizionale. Nel primo, sono costretto a controllare due condizionali. Raddoppia il lavoro per lo stesso risultato.

Un'altra cosa da considerare è che se le affermazioni sono un grande spazio per causare bug. Più sono lunghe le condizioni (più è & e || usi), più possibilità hai di creare un bug facendolo male. Ridurre la complessità dei tuoi condizionali può renderlo più facile da capire anche in seguito.

Mentre sembra che i due blocchi di codice siano gli stessi, c'è la possibilità che non lo siano. In particolare, se stuff modifica previouslyCalculatedIndex i percorsi del codice per i due potrebbero essere diversi.

Probabilmente non è qualcosa che è destinato a succedere, ma potrebbe. Il primo esempio potrebbe esporre un errore se la variabile condizionale condivisa dovesse cambiare. Il secondo esempio impedisce che ciò accada (il condizionale viene testato solo una volta).

Per la leggibilità, trovo il secondo esempio più facile da leggere. Sono consapevole dello scopo che l'istruzione if esterna ha e può leggere le istruzioni meno complesse se più facilmente.

Poche regole sono "sempre" - dubito che chiunque, tranne il più pedante, faccia sempre qualcosa quando si tratta di stile del codice. Tuttavia, quando possibile, si dovrebbe tentare di codificare come difensivo (scrivendo codice che è difficile da modificare nel modo sbagliato) e comprensibile possibile.

Quando le cose raggiungono la complessità di 5-10 condizioni, questo è qualcosa che richiede un refactoring ancora più drastico - metodi di estrazione (sia per il codice condizionale che per il codice comune).

Considera come complex la classe è quando arrivi a 5-10 condizioni. La complessità aggiuntiva è correlata all'aumento dei conteggi degli errori (e alla più difficile testabilità).

Prendi le misure in anticipo per mantenere la complessità bassa.

Questo è piuttosto secondario, ma direi che ti sbagli. Il motivo principale per cui il suo codice è migliore è perché previouslyCalculatedIndex != -1 viene eseguito solo una volta. Questo non è un tipo di ottimizzazione delle prestazioni, ma se dovessi mai cambiare quella condizione per dire -2 o alcuni di questi, dovrai solo cambiarla in un posto invece che in due. E, se continui il modello (ad esempio hai 10 altre cose che si basano sull'indice -1), allora alla fine sarebbe un'enorme quantità di riferimenti da aggiornare.

Ancora una volta, piuttosto minore, ma il revisore ha ragione. La sua strada è più pulita e ha meno duplicazioni e, a lungo termine, sarà più facile da mantenere. Inoltre, a mio parere, sembra un po 'più chiaro

Inoltre, per le tue modifiche alle domande (credo). È un'opinione se questo dovrebbe essere fatto con l'istruzione 1 se (io tendo a non farlo), ma se ne hai due, allora dovresti farlo così. Che dire quando devi aggiungere alcune funzionalità tra un anno e ora e aggiungere una condizione che si basa sull'indice -1. Quindi, dovrai continuare la duplicazione del codice, o prenderti il tempo di rifattorizzarlo su come il tuo collega ha detto che dovrebbe essere in primo luogo

Penso che il contesto sia importante. Preferisco lasciare che le cose importanti risaltino e dovrebbero essere le migliori in caso di test.

Dal tuo esempio, se isGood / isBad è ciò che i tuoi if-check stanno davvero cercando di acquisire e il precedente CalculatedIndex è semplicemente un assegno per assicurarti di poter validamente intraprendere l'azione appropriata allora potrei andare con un terzo approccio e mettere il if (previousCalculatedIndex! = -1) all'interno delle parentesi isGood / isBad per ogni controllo, dal momento che se è possibile elaborare validamente la condizione isGood / IsBad è secondario rispetto a ciò che si stava tentando di acquisire (ovvero isGood / isBad).

Inoltre, cosa succede se si desidera creare una voce di registro se (previousCalculatedIndex == -1) e la condizione isGood / isBad è stata soddisfatta? Nessuno degli altri suggerimenti aiuta in questa situazione perché sta tentando di salvare una riga di codice invece di rendere il codice più comprensibile.

Vorrei anche cambiare -1 in un const con un nome significativo.

D'altra parte, se (previousCalculatedIndex! = -1) è il test importante, allora la seconda opzione è migliore.