Come denominare le funzioni che utilizzano i condizionali nel refactoring [chiuso]

1

Considera questo bit di codice:

private Norf foo(Baz baz) {
    // ...
    // Logic on baz
    // ...

    if (baz.color == Baz.BLUE) {
        // Do this thing
    }

    // ...
    // More logic
    // ...

    return norf;
}

Supponiamo che il contenuto dell'istruzione if sia al livello di astrazione sbagliato per foo e quindi maturo per il refactoring. Spesso vado in situazioni come questa, ma non mi sento mai come se il mio refactoring fosse buono.

Un modo per fare il refact sarebbe estrarre una funzione doThisThingIfBazIsBlue . Però una funzione con un "se" nel suo nome mi fa male, quindi non va bene.

Un altro approccio è mantenere il condizionale in foo ed estrarre doThisThing . Questa soluzione si sente meglio, ma a volte non è propriamente l'attività di foo se il " thing " è fatto o meno. In questo caso, foo è troppo breve perché ciò contenga, ma in funzioni più lunghe può diventare la maggior parte della complessità.

Conosci uno schema di denominazione che funziona per questo tipo di situazione? Come si refactoring complessi if dichiarazioni e corpi?

    
posta Mobius 02.02.2016 - 18:23
fonte

4 risposte

6

Una delle soluzioni al tuo problema potrebbe essere il polimorfismo e la delega del // Do this thing , alla classe Baz .

Quindi modifica l'implementazione del metodo foo in qualcosa del genere:

private Norf foo(Baz baz)
{
    // ...
    // Logic on baz
    // ...

    baz.doThisThing();

    // ...
    // More logic
    // ...

    return norf;
}

E con più implementazioni della classe Baz.

class BlueBaz extends Baz
{
    public void doThisThing()
    {
        Console.WriteLine("Yay, this is right. Blue is the color of men!");
    }
}

class AnyOtherBaz extends Baz
{
    public void doThisThing()
    {
        throw new Exception("Sorry, no blue color in this one. Perhaps you like a different one.");
    }
}

E se ritieni che avresti ancora bisogno di un controllo if nei nuovi figli della classe Baz , nel nuovo metodo doThisThing , puoi delegare ulteriormente la decisione, magari a un Color classe.

Il modello che stai vedendo qui si chiama inversione del controllo . Significa fondamentalmente che esiste un contratto predefinito da un'API pubblica, determinata principalmente dalle interfacce, ma talvolta determinata da classi di base (astratte). Il contratto è impostato e può essere utilizzato, ma le implementazioni vengono scelte durante il runtime, quando stai costruendo il grafico degli oggetti .

Informazioni sulla convenzione di denominazione

Se deleghi la condizione più in basso (o le classi rispettivamente), non avrai davvero bisogno di avere un ifACondition nel nome del tuo metodo, perché il metodo stesso non conterrà il if che sei preoccupato di. Per gestire if , avrai una classe completamente separata, con il metodo doThisThing ma che fornisce un'implementazione diversa, una logica diversa.

Nel tuo caso verrebbe lasciato il nome doThisThing e decidere quale classe sarebbe scelta costruendo l'istanza desiderata (forse da una fabbrica o da un boostrap responsabile della costruzione del grafo degli oggetti).

Nota adizionale

Prendi in considerazione che questo è un approccio possibile, ma non l'unico. Se non vuoi avere condizionali nel tuo codice, il polimorfismo è la strada da percorrere nella programmazione OO. Ma se stai bene con il router alternativo quando il colore è blu e sei soddisfatto di come funziona il codice finora, puoi lasciarlo così com'è.

    
risposta data 02.02.2016 - 19:05
fonte
2

Anch'io ne ho abbastanza spesso.

doThisThingIfBazIsBlue ha un odore strano. doThisThing è OK, ma come dici tu, questa logica dovrebbe a volte essere incapsulata nel metodo.

Di solito provo a guardare l'operazione dalla vista di livello superiore e vedere cosa intende fare e nominarla di conseguenza (faccio fatica a produrre un esempio ora ...).

A volte lo chiamo doThisThing e considero "if" come un dettaglio di implementazione interno - l'azione viene eseguita solo se è necessaria, che semplicemente non ha bisogno di essere nota al codice client (ad esempio, i dati dal DB vengono caricati solo se non è nella cache: il client non si cura di dettagli di implementazione di questo tipo.

Se sopra non funziona, solitamente lo chiamo handleSomething , ad es. handleSaving - è la mia convenzione di codifica interna - non è così imperativo come "save ()" e potrebbe implicare che ci sia un po 'più di logica / pre-elaborazione.

    
risposta data 02.02.2016 - 18:35
fonte
1

Probabilmente una non risposta, ma non la rifarei. A meno che non accorci il codice o lo renda più leggibile, lo rende ancora più confuso.

La migliore lezione appresa sul refactoring è attendere fino a quando non si utilizza qualcosa due volte prima di renderlo una funzione. Se puoi utilizzare la funzione "doThisThingIfBazIsBlue ()" due volte, separala e in caso contrario.

Inoltre, non avere una metamorfosi di classe in qualcos'altro basato su di esso attributi. Se Baz.color = BLUE cambia completamente il comportamento, pensa di creare una classe BlueBaz. E poi muovi la "Logica su baz" e "Più logica" in subroutine.

    
risposta data 02.02.2016 - 19:19
fonte
0

Una buona funzione dovrebbe svolgere un compito specifico in un'astrazione di livello superiore, deve anche rispettare le regole di alta coesione e basso accoppiamento. Il nome della tua funzione deve essere l'attività sta eseguendo. Se non riesci a trovare un buon nome (un nome con "se" non è un buon nome), allora il blocco di codice che stai cercando di estrarre non è un buon candidato per una buona funzione. Forse dovresti includere più codice, o rimuovere il codice, o magari lasciare così com'è.

Ad esempio il seguente blocco di codice:

...

if (file.Exists) {
//code for removing the file and making any other related changes
...
}

...

Può essere estratto come funzione:

 removeFile(file);
    
risposta data 03.02.2016 - 17:44
fonte

Leggi altre domande sui tag