vale la pena riorganizzare se le altre strutture sono più corte [duplicate]

In caso di dubbi, preferisci la brevità.
Se c'è meno codice da capire, sarà più facile sul lettore, assumendo approssimativamente la stessa qualità del codice. E la qualità non è stata sacrificata lì.

Ma tieni presente che facendo qualsiasi modifica rischia di introdurre nuovi difetti , quindi lo hai coperto con test automatizzati?
Migliorare la copertura dei test potrebbe essere un uso migliore del tuo tempo, come aggiungere nuove funzionalità o fare dei refatting più sostanziali.
Lavori ancora su quel codice, che include solo la lettura per capirlo, magari mentre cerchi i bug altrove?

A parità di tutti gli altri (non lo è mai), andrei con qualcosa come la seconda opzione, dopo aver pulito il blocco superfluo e averlo reso una catena condizionale appropriata.

Tuttavia, il refactoring mi porterebbe ad avere gli ultimi due blocchi in un ordine diverso. Mi chiedo perché li hai invertiti:

if(b) {
    doSomething1;
}
else if(a || !c) {
    doSomething2;
}
else {
    doSomething3;
}

Personalmente, metterei else sulla stessa riga della parentesi di chiusura, ma faccio come vuoi (o devi), la consistenza è fondamentale, e non serve a far partire ancora un altro flamewar sull'unico e solo vero stile.

Fai attenzione a valutare a , b o c che potrebbero avere effetti collaterali che devono essere considerati però.

Sì. Hai un caso. Refactor, assumendo che sia a basso rischio / è coperto con test unitari. Per essere onesti, cose simili dovrebbero essere prese durante la revisione del codice: un buon sviluppatore dovrebbe cercare un livello minimo di espressione ragionevole per un determinato problema ed è abbastanza ovvio che non è stato fatto qui.

Non penso che ci sia un nome diverso da quello che non è DRY (Non ripeterlo) - la stessa frase 'if' è scritta due volte.

Ovviamente non rifattalo per il suo bene - fallo alla prossima volta che fai quell'area di codice facendo qualcosa. In effetti, se vedessi un codice del genere sarei aggressivo:

• sposta le condizioni secondarie nelle loro funzioni.
• verifica se i doSomethings hanno un codice comune, questo è condiviso e riesegui l'eliminazione come appropriato.
• ricontrolla i requisiti del software che tutti i doSomethings sono effettivamente necessari.
• elimina il bisogno di ifs, definendo i doSomethings in classi di comportamento / usa la spedizione dinamica. Aggiungi test unitari per quelli.

In questo caso, non sono a favore di una tabella di verità in quanto hai solo due semplici condizioni. Se non è critico per le prestazioni, ciò che potrebbe funzionare per te è valutare le condizioni e archiviare i bool denominati, quindi utilizzare i bool in ifs più tardi. È un po 'più auto-documentante: per es.

bool is_enabled = ...;
if (is_enabled) {

Risposta breve "No".

Risposta lunga "Il refactoring per lo stile non vale mai la pena".

Ogni tuo cambiamento comporta il rischio di rompere qualcosa. Per valere il rischio, ci deve essere qualche potenziale guadagno monetario. Di solito questa è una nuova funzionalità, ma potrebbe essere un miglioramento delle prestazioni o qualche altra non funzionale.

Le modifiche allo stile non aggiungono funzionalità. L'argomento secondo cui accelera lo sviluppo futuro è molto debole. Sinceramente, se lo sviluppo che stai accelerando ha una grande percentuale di cambiamenti di stile.

PS Personalmente, penso che il primo esempio sia molto più leggibile del secondo. Anche se la linea duplicata è fastidiosa.

Direi un approccio completamente diverso che rimuove completamente il if .

Diciamo che abbiamo a=false , b=false , c=true , quale funzione verrà chiamata? Per sapere, dobbiamo eseguire faticosamente molte delle if-strutture annidate nelle nostre teste per arrivare alla risposta: doSomething1 . Per aiutare, potremmo inserire la tabella logica in un commento:

/*
  +-------+-------+-------+--------------+
  |   a   |   b   |   c   |   function   |
  +-------+-------+-------+--------------+
  | false | false | false | doSomething2 |
  | false | false | true  | doSomething3 |
  | false | true  | false | doSomething1 |
  | false | true  | true  | doSomething1 |
  | true  | false | false | doSomething2 |
  | true  | false | true  | doSomething2 |
  | true  | true  | false | doSomething1 |
  | true  | true  | true  | doSomething1 |
  +-------+-------+-------+--------------+
*/

ma a quel punto rischiamo che il codice e il commento non siano sincronizzati. Quindi metti la tabella logica nel codice. Ad esempio, in C #, potrei usare un dizionario:

private static readonly Dictionary<(bool, bool, bool), Action> FunctionChooser =
    new Dictionary<(bool, bool, bool), Action>
    {
        [(false, false, false)] = doSomething2,
        [(false, false, true)] = doSomething3,
        [(false, true, false)] = doSomething1,
        [(false, true, true)] = doSomething1,
        [(true, false, false)] = doSomething2,
        [(true, false, true)] = doSomething2,
        [(true, true, false)] = doSomething1,
        [(true, true, true)] = doSomething1
    };

Ora tutta la sequenza if - else può essere sostituita con una sola riga:

FunctionChooser[(a, b, c)]();

E, in futuro, se ho bisogno di aggiungere una condizione doSomething4 , ho appena modificato la tabella logica (dizionario) per aggiungerla, piuttosto che dover provare a modificare un carico di nidificati 'if senza rompere nulla.