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.