L'invio di parametri all'interno di una classe suggerisce che dovrei rifattore in una nuova classe?

1

Ho notato uno schema durante il refactoring del mio codice per la leggibilità. Quando rifatto un metodo in diversi metodi più piccoli, spesso devo introdurre dei parametri che a volte (troppo spesso) rendono il codice meno leggibile di prima. Mi chiedo se questo indica che dovrei refactoring per una classe diversa, invece di solo metodi più piccoli.

Penso che sia una situazione molto comune, ma non ricordo di aver letto da nessuna parte come gestirlo.

Lei è un semplice esempio di questo da uno scenario inventato.

Prima di refactoring

public class ATM
{
    private double cashBalance = 5000;

    public double WithDraw(CreditCard card, string pin, double amount)
    {
        if(card.Pin != pin)
            throw new Exception("Wrong pin");
        if(balance < amount)
            throw new Exception("ATM out of money");
        if(card.Balance < amount)
            throw new Exception("Credit card out of money");
        cashBalance -= amount;
        return amount;
    }
}

A prima vista, questo non è male, ma voglio rimuovere la logica dai miei condizionali e dargli i nomi propri.

Primo refactoring

public class ATM
{
    private double cashBalance = 5000;

    public double WithDraw(CreditCard card, string pin, double amount)
    {
        Validate(card, pin, amount);
        cashBalance -= amount;
        return amount;
    }

    private void Validate(CreditCard card, string pin, double amount)
    {
        if(!VerifyPin(card, pin))
            throw new Exception("Wrong pin");
        if(!HasEnoughCashBalance(amount))
            throw new Exception("ATM out of money");
        if(!CardHasEnoughMoney(card, amount))
            throw new Exception("Credit card out of money");
    }

    private bool VerifyPin(CreditCard card, string pin)
    {
        return card.Pin == pin;
    }

    private bool HasEnoughCashBalance(double amount)
    {
        return cashBalance >= amount;
    }

    private bool CardHasEnoughMoney(CreditCard card, double amount)
    {
        return card.Balance >= amount;
    }
}

Separo la convalida in base al suo metodo, quindi aggiusta ulteriormente ogni condizionale al proprio metodo assegnandogli un nome più descrittivo. (nota che non ho passato il tempo a inventare effettivamente dei bei nomi) Questo però ha introdotto molti parametri, e mentre il metodo WithDraw è più leggibile, la parte di validazione isn ' davvero.

Questa è la situazione di un avviso più e più volte durante il refactoring dei metodi in metodi più piccoli.

Secondo refactoring

public class ATM
{
    private double cashBalance = 5000;
    private CreditCard card;
    private string pin;
    private double amount;

    public double WithDraw(CreditCard card, string pin, double amount)
    {
        this.card = card;
        this.pin = pin;
        this.amount = amount;
        Validate();
        cashBalance -= amount;
        return amount;
    }

    private void Validate()
    {
        if(!VerifyPin())
            throw new Exception("Wrong pin");
        if(!HasEnoughCashBalance())
            throw new Exception("ATM out of money");
        if(!CardHasEnoughMoney())
            throw new Exception("Credit card out of money");
    }

    private bool VerifyPin()
    {
        return card.Pin == pin;
    }

    private bool HasEnoughCashBalance()
    {
        return cashBalance >= amount;
    }

    private bool CardHasEnoughMoney()
    {
        return card.Balance >= amount;
    }
}

A volte finisco con questa correzione , ma non ne sono un grande fan. Sembra rischioso borderline, o almeno non si sente coesivo.

Terzo refactoring

public class ATM
{
    private double cashBalance = 5000;

    public double WithDraw(CreditCard card, string pin, double amount)
    {
        var validator = new ATMValidator(cashBalance, card, pin, amount);
        validator.Validate();
        cashBalance -= amount;
        return amount;
    }
}

public class ATMValidator
{
    private double amount;
    private CreditCard card;
    private string pin;
    private double cashBalance;

    public ATMValidator(double cashBalance, CreditCard card, string pin, double amount)
    {
        this.cashBalance = cashBalance;
        this.card = card;
        this.pin = pin;
        this.amount = amount;
    }

    public void Validate()
    {
        if(!VerifyPin())
            throw new Exception("Wrong pin");
        if(!HasEnoughCashBalance())
            throw new Exception("ATM out of money");
        if(!CardHasEnoughMoney())
            throw new Exception("Credit card out of money");
    }

    private bool VerifyPin()
    {
        return card.Pin == pin;
    }

    private bool HasEnoughCashBalance()
    {
        return cashBalance >= amount;
    }

    private bool CardHasEnoughMoney()
    {
        return card.Balance >= amount;
    }
}

E questo è dove finisco, avendo una classe separata. In questo esempio mi sembra abbastanza ragionevole, e vorrei sostenere che il ritiro e la convalida sono due responsabilità separate.

Ciò che voglio davvero sapere però è se questo modello di invio di parametri all'interno di una classe è indicativo di responsabilità separate. E lo sto gestendo correttamente. È forse solo una coincidenza pura?

Grazie per aver dedicato del tempo a leggere tutto questo.

Sidenote: senza refactoring non avrei mai notato la responsabilità separata.

    
posta Chris Wohlert 31.03.2017 - 10:39
fonte

2 risposte

4

Il tuo secondo refactoring è decisamente sbagliato. L'aggiunta di variabili di carte, pin e quantità alla classe ATM è concettualmente sbagliata. Questi parametri non appartengono all'ATM, appartengono a una transazione specifica.

Il design dovrebbe modellare il problema del mondo reale. Mantieni CashBalance nella classe ATM e prova a mettere le variabili di carte, pin e quantità in una classe ATMTransaction. Ti ritroverai con qualcosa che è un mix tra il tuo primo e il terzo refactoring e, si spera, avrà più senso.

Nota che devi avere due funzioni di convalida - una che convalida la transazione stessa (per controllare il PIN della carta e che abbia abbastanza soldi) e un'altra per convalidare la transazione contro l'ATM (per verificare che l'ATM abbia abbastanza soldi per coprire l'importo della transazione).

In risposta alla tua domanda iniziale, la risposta è "dipende". A volte devi passare dei parametri, a volte è indicativo di un problema con il design. Non esiste una regola semplice che puoi applicare e che sarà sempre corretta, e se cerchi di percorrere questa strada, creerai più problemi di quanti ne risolvi. Tuttavia, se trovi che hai un piccolo gruppo di parametri che stai ripetutamente passando insieme alle funzioni (es. Carta, pin e importo nell'esempio sopra) allora questo indica che deve esserci un nuovo livello di astrazione che incapsula loro (cioè la classe ATMTransaction che propongo).

    
risposta data 31.03.2017 - 10:53
fonte
3

Direi che nessuno dei tuoi esempi separa realmente le responsabilità relative al completamento di un prelievo. In tutti i tuoi refactoring stai mescolando il metodo Validate in giro, che prende tutte le variabili come argomenti. Inoltre, nel tuo ultimo esempio, entrambe le classi "conoscono" tutte le variabili, che possono essere un'indicazione (almeno in questo esempio) che questa è una falsa separazione.

Per separare le responsabilità, considera la convalida del PIN la responsabilità della Carta (come entità che ha solo bisogno di conoscere il PIN inserito e il numero della carta), controlla che il cliente abbia i fondi per fare del prelievo la responsabilità di un Conto (che ha solo bisogno di conoscere l'importo e che detiene il saldo), e controllare che ci sia abbastanza denaro per dispensare la responsabilità del bancomat (che ha solo bisogno di conoscere l'importo e che detiene la liquidità).

Quindi, per rispondere alla tua domanda: sì, il passaggio di (gli stessi) parametri all'interno di una classe potrebbe suggerire che la classe potrebbe essere divisa per separare le responsabilità.

    
risposta data 31.03.2017 - 23:20
fonte