Devo ripetere il codice di controllo delle condizioni o inserirlo in una funzione? [duplicare]

Di solito quando qualcosa si sente così imbarazzante, è perché hai bisogno di rifattorizzare i tuoi oggetti per allocare più logicamente le responsabilità. Se ne hai una manciata, la mia preferenza personale sarebbe metterli in una lista e chiamarli così:

for feature in enabledFeatures:
    feature.process()

In questo modo non stai nemmeno perdendo un controllo condizionale sulle funzionalità disabilitate, che è particolarmente utile quando il rapporto tra le funzioni disabilitate e quelle abilitate è elevato.

Potresti non avere una scelta qui: se valutando la proprietà ReallyLongFeatureZSettingAName quando FeatureSettings.FeatureZIsEnalbed equivale a false si traduce nel lancio di un'eccezione, l'idea di passare il "guarding" FeatureSettings.FeatureZIsEnalbed a TheMethodImCalling attiverà un eccezione:

this.TheMethodImCallingIfEnabled(
    FeatureSettings.FeatureZIsEnalbed,
    FeatureSettings.ReallyLongFeatureZSettingAName,
    FeatureSettings.ReallyLongFeatureZSettingBName,
    ...);

A seconda della lingua di destinazione, potresti essere in grado di abbreviare il codice "invertendo" la sequenza di chiamate: ad esempio, in C # potresti aggiungere questo metodo a FeatureSettings

class FeatureSettings {
    ...
    public void RunIfEnabledZ(Func<string,string> callback) {
        if(FeatureZIsEnalbed) {
            callback(ReallyLongFeatureZSettingAName, ReallyLongFeatureZSettingBName);
        }
    }
}
...
var theCallback = (a, b) =>  this.TheMethodImCalling(a, b, ...);
FeatureSettings.RunIfEnabledX(theMethod);
FeatureSettings.RunIfEnabledY(theMethod);
FeatureSettings.RunIfEnabledZ(theMethod);

Non c'è nulla di sbagliato in una condizione di guardia nella parte superiore di una funzione, ma prima considererei due cose: (1) qual è il caso d'uso normale - viene normalmente chiamato con sì o no? E poi (2) è esternamente visibile? Possono fare lo stesso tipo di controllo?

Chiamare un metodo che non fa il 99% delle volte, è una cattiva idea - è fuorviante.

Se la tua funzione è esposta esternamente e chiamata da altrove, la tua condizione potrebbe non avere senso, quindi passeranno sempre true.

Sarebbe meglio se la tua funzione potesse calcolare FeatureIsEnabled stessa.

In parole semplici:

• Il codice mi sembra a posto.
• Ha una complessità ciclomatica molto piccola.
• Non vedo alcun rumore nel dover chiudere le parentesi graffe di if .
• Avere semplice sequenziale "se" blocchi * non è un odore di codice *.

Ma se pensi che la lista dei processi condizionali crescerà in futuro dovresti considerare i modelli di "Command" o "Chain of responsability" . In questo modo non avrai bisogno del ifs con le parentesi graffe corrispondenti che ti infastidiscono così tanto.