È meglio chiamare una funzione più volte o assegnare una variabile più volte e chiamare la funzione una volta? [duplicare]

Usa quello che pensi sia più leggibile e manutenibile . A seconda della lingua, delle variabili e delle chiamate di funzione coinvolte, potrebbe essere preferibile una delle due. Se dovessi valutare i tuoi due esempi in isolamento, avrei una leggera preferenza per la seconda versione perché rende un po 'più ovvio che do_something () venga chiamato una sola volta.

Ma nel contesto del resto della tua base di codice, alcune domande di "tornasole" come queste possono aiutare a valutare quei criteri:

• È più intuitivo dire "se [condizione] è vera, [variabile] dovrebbe essere 1" o "se [condizione] è vera, dovremmo fare_qualcosa (1)"?
• Il valore di [variabile] ha un suo significato, o è solo significativo come argomento per fare_qualcosa ()?
• Quando questo blocco di codice deve cambiare in futuro, è più probabile che i rami finiscano per assegnare valori diversi alle variabili, o che i rami finiscano per chiamare diverse funzioni?
• Quali sono le probabilità che vorremmo chiamare do_something () più di una volta in questo blocco di codice?
• Avrà mai senso spostare questa logica di ramificazione all'interno di do_something (), rendendo inutile questa intera domanda?
• Quanto potrebbero essere complicate [condizioni] e [altre condizioni] potenzialmente?

Immagina un esempio concreto di un codice che, dato il prezzo di un articolo, dovrebbe chiamare un metodo specificando se l'articolo è un rimborso (prezzo inferiore a zero), un prodotto pagato (prezzo superiore a zero) o un valore gratuito prodotto (prezzo uguale a zero).

Le tue due parti di codice diventano:

Soluzione 1:

if (price < 0)
{
    this.DoSomething(PriceType.Rebate);
}
else if (price > 0)
{
    this.DoSomething(PriceType.Product);
}
else // price == 0
{
    this.DoSomething(PriceType.Free);
}

Soluzione 2:

priceType = PriceType.Free;

if (price < 0)
{
    priceType = PriceType.Rebate;
}
else if (price > 0)
{
    priceType = PriceType.Product;
}

this.DoSomething(priceType);

La seconda soluzione, così com'è, ha meno righe (10 contro 12), nonostante la tua affermazione che "occupa più righe di codice", ma la prima sembra più leggibile. D'altra parte, la seconda soluzione può essere rifattorizzata per diventare molto più leggibile:

Refactoring della soluzione 2:

private PriceType FindPriceType(price)
{
    if (price < 0)
    {
        return PriceType.Rebate;
    }
    else if (price > 0)
    {
        return PriceType.Product;
    }

    return PriceType.Free;
}

...

this.DoSomething(this.FindPriceType(price));

Separando il metodo originale in due metodi, il codice diventa più leggibile attraverso l'introduzione di un nome esplicito del metodo.

Esistono anche due casi specifici in cui la seconda soluzione offre ulteriori vantaggi:

Casi specifici

Il primo caso è quello in cui le condizioni tutte hanno la forma: if (something == value) . Ad esempio:

if (priceType == 1)
{
    return PriceType.Rebate;
}

if (priceType == 2)
{
    return PriceType.Product;
}

if (priceType == 3)
{
    return PriceType.Free;
}

throw new NotImplementedException();

può essere trasformato in molto più breve:

var map = {
    1: PriceType.Rebate,
    2: PriceType.Product,
    3: PriceType.Free
}[priceType];

Un secondo caso specifico è dove non ci sono tre rami, ma solo un if/else . Esempio:

Soluzione 1:

if (price < 0)
{
    this.DoSomething(PriceType.Rebate);
}
else
{
    this.DoSomething(PriceType.Product);
}

Soluzione 2:

priceType = PriceType.Product;

if (price < 0)
{
    return PriceType.Rebate;
}

this.DoSomething(priceType);

In questo caso, la seconda soluzione può (in molte lingue) essere trasformata in one-liner:

this.DoSomething(price < 0 ? PriceType.Rebate : PriceType.Product);

Raccomando di separare le preoccupazioni dal do_qualcosa e dalle condizioni. C'è una logica aziendale che dovrebbe essere ben denominata e incapsulata per determinare su cosa funzionerà do_qualcosa (ConditionType nel mio esempio di seguito). Usa il ConditionType risultante come parametro per fare qualcosa.

Ho letto molto codice (più di quanto scrivo) e non sono un grande fan di mettere più codice possibile in una riga. È troppo facile perdere alcuni passaggi cruciali durante la scansione del codice.

Inoltre, usa costanti, enumerazioni o il tuo stuc preferito per evitare i numeri magici (0,1,2 in questo caso).

Ecco un esempio di codice:

   const int ConditionTypeZero = 0;
   const int ConditionTypeOne = 1;
   const int ConditionTypeDefault = 2;

...

        var conditionType = GetConditionType(condition, otherCondition);
        do_something(conditionType);

...

    private int GetConditionType(bool condition, bool otherCondition )
    {
        if (condition)
        {
            return ConditionTypeZero;
        }
        if (otherCondition)
        {
            return ConditionTypeOne;
        }
        return ConditionTypeDefault;
    }