IMO, le risposte precedenti si concentrano su casi limite e codice che dovrebbero essere comunque riscritti. Ecco alcuni casi che vedo quasi ovunque in cui if/else avrebbe dovuto essere sostituito da if o da un altro costrutto linguistico.
Nota: creo la wiki della comunità di risposta. Sentiti libero di modificarlo e aggiungi altri esempi di casi in cui if/else di solito porta a codice errato.
Principianti, sii avvisato ! if/else ha il potenziale di creare un codice illeggibile se non usato con attenzione. Gli esempi seguenti mostrano i casi più comuni in cui if/else è facilmente utilizzato in modo improprio.
1. Clausola di protezione
Ho smesso di contare quante volte ho visto qualcosa di simile:
void Demo(...)
{
if (ok_to_continue)
{
... // 20 lines here.
}
else
{
throw new SomeException();
}
}
Nessuno ha bisogno di ulteriori indentazioni. Lo stesso codice avrebbe potuto essere scritto in questo modo:
void Demo(...)
{
if (!ok_to_continue)
{
throw new SomeException();
}
... // No indentation for the next 20 lines.
}
Regola: se puoi rimuovere un'istruzione else invertendo il controllo if , gestendo un errore ed uscendo immediatamente (altrimenti prosegui altrimenti), esegui tale operazione.
2. Duplicazione del codice
Anche questo è abbastanza frequente.
if (!this.useAlternatePanel)
{
currentInput = this.defaultTextBox.Text;
bool missingInput = currentInput.Length == 0;
this.defaultProcessButton.Enabled = !missingInput;
this.defaultMissingInputHelpLabel.Visible = missingInput;
}
else
{
currentInput = this.alternateTextBox.Text;
bool missingInput = currentInput.Length == 0;
this.alternateProcessButton.Enabled = !missingInput;
this.alternateMissingInputHelpLabel.Visible = missingInput;
}
Questo è un eccellente esempio di codice per principianti, perché un principiante non vedrebbe sempre come tale codice può essere rifatto. Uno dei modi è creare una mappa di controlli:
var defaultControls = new CountriesProcessingControls(
this.defaultTextBox,
this.defaultProcessButton,
this.defaultMissingInputHelpLabel);
var alternateControls = new CountriesProcessingControls(
this.alternateTextBox,
this.alternateProcessButton,
this.alternateMissingInputHelpLabel);
void RefreshCountriesControls(CountriesProcessingControls controls)
{
currentInput = controls.textBox.Text;
bool missingInput = currentInput.Length == 0;
controls.processButton.Enabled = !missingInput;
controls.missingInputHelpLabel.Visible = missingInput;
}
RefreshCountriesControls(this.useAlternatePanel ? alternateControls : defaultControls);
Regola: se il blocco in else è quasi identico al blocco in if , stai sbagliando e causando la duplicazione del codice.
3. Cercando di fare due cose
È anche vero il contrario:
if (...)
{
foreach (var dateInput = this.selectedDates)
{
var isConvertible = convertToDate(dateInput);
if (!isConvertible)
{
this.invalidDates.add(dateInput);
}
}
}
else
{
var languageService = this.initializeLanguageService();
this.Translated = languageService.translate(this.LastInput);
if (this.Translated != null)
{
this.NotificationScheduler.append(LAST_INPUT_TRANSLATED);
}
}
Il blocco in if non ha nulla a che fare con il blocco in else . La combinazione di due blocchi nello stesso metodo rende la logica difficile da comprendere e soggetta a errori. Trovare un nome per il metodo e anche commentarlo correttamente è estremamente difficile.
Regola: evita di combinare blocchi che non hanno nulla in comune insieme in un blocco if/else .
4. Programmazione orientata agli oggetti
Questo tipo di codifica è frequente anche tra i principianti:
// 'animal' is either a dog or a cat.
if (animal.IsDog)
{
animal.EatPedigree();
animal.Bark();
}
else // The animal is a cat.
{
animal.EatWhiskas();
animal.Meow();
}
Nella programmazione orientata agli oggetti, questo codice non dovrebbe esistere. Dovrebbe essere scritto in questo modo:
IAnimal animal = ...;
animal.Eat();
animal.MakeSound();
dato che ogni classe ( Cat : IAnimal e Dog : IAnimal ) avrà la propria implementazione di quei metodi.
Regola: quando si esegue la programmazione orientata agli oggetti, si generalizzano le funzioni (metodi) e si utilizzano l'ereditarietà e il polimorfismo.
5. Assegnazione di un valore
Vedo questo ovunque nelle codebase scritte dai principianti:
int price;
if (this.isDiscount)
{
price = -discount;
}
else
{
price = unitCost * quantity;
}
Questo è un codice terribile, dal momento che non è scritto per gli umani.
-
È fuorviante. Ti fa pensare che l'operazione principale qui è la scelta tra la modalità sconto e quella non scontata, mentre l'operazione principale effettiva è l'assegnazione di price .
-
È soggetto a errori. Cosa succede se in seguito, una condizione è cambiata? Cosa succede se uno dei due compiti viene rimosso?
-
Invita codice ancora peggiore. Ho visto un sacco di pezzi come quello in cui int price era invece int price = 0; , giusto per sbarazzarsi dell'avvertimento del compilatore se manca uno dei compiti. Nascondere gli avvertimenti invece di risolverli è una delle peggiori cose che si possa fare.
-
C'è una minuscola duplicazione del codice, che significa più caratteri da digitare in origine e più modifiche da fare in seguito.
-
È inutilmente lungo.
Lo stesso potrebbe essere scritto in questo modo:
int price = this.isDiscount ? -discount : unitCost * quantity;
Regola: usa l'operatore ternario invece dei blocchi if/else per assegnazioni di valore semplici.
