Простой рефакторинг, если в if(...) сложное условие вынесите его в метод и назовите так, чтобы сразу можно было понять, что это за условие
public enum PeopleType
{
Child,
Teenager,
YoungMan,
Man,
OldMan,
DeadMan
}
...
public decimal Deposit;
public PeopleType PeopleType { get; }
public void Pay(int value)
{
if (PeopleType == PeopleType.YoungMan || PeopleType == PeopleType.Man || PeopleType == PeopleType.OldMan)
Deposit -= value;
else throw new NotSupportedException();
}
Применим Decomposite Conditional
public void Pay(int value)
{
if (IsSolventPerson())
Deposit -= value;
else throw new NotSupportedException();
}
private bool IsSolventPerson()
{
return PeopleType == PeopleType.YoungMan || PeopleType == PeopleType.Man || PeopleType == PeopleType.OldMan;
}
Если идут несколько условий подряд:
public void Pay(int value)
{
if(PeopleType == PeopleType.YoungMan)
Deposit -= value;
if(PeopleType == PeopleType.Man)
Deposit -= value;
if(PeopleType == PeopleType.OldMan)
Deposit -= value;
else throw new NotSupportedException();
}
Заменим на одно условие:
public void Pay(int value)
{
if (PeopleType == PeopleType.YoungMan || PeopleType == PeopleType.Man || PeopleType == PeopleType.OldMan)
Deposit -= value;
else throw new NotSupportedException();
}
А потом применим Decompose Conditional
public void Pay(int value)
{
if (IsSolventPerson())
Deposit -= value;
else throw new NotSupportedException();
}
private bool IsSolventPerson()
{
return PeopleType == PeopleType.YoungMan || PeopleType == PeopleType.Man || PeopleType == PeopleType.OldMan;
}
Простой метод, если в теле нескольких операторов if дублируются код - вынесите дублирующийся код из них Бывает сложно это сделать, когда этот код что-то меняет и должен быть вызван после определенного кода и перед определенным кодом, тогда этот рефакторинг стоит прервать и попытаться распутать эти вызовы, если это сложно, то стоит остановить рефакторинг
...
if (PeopleType == PeopleType.YoungMan)
{
Deposit -= value;
Print();
}
if (PeopleType == PeopleType.Man)
{
Deposit -= value;
Print();
}
if (PeopleType == PeopleType.OldMan)
{
Deposit -= value;
Print();
}
...
Применяем рефакторинг:
if (PeopleType == PeopleType.YoungMan)
Deposit -= value;
if (PeopleType == PeopleType.Man)
Deposit -= value;
if (PeopleType == PeopleType.OldMan)
Deposit -= value;
Print();
Разобран простой случай, обычно бывает сложнее и приходится разбираться что именно делает дублирующийся метод, это происходит, когда в теле if слишком много строк
Не бойтесь прервать поток выполнения метода и выйти из него при помощи break(цикл) или return (В данном примере нужно воспринимать == - как очень сложную и долгую операцию, а сравнение flag - как очень простую, поэтому я не хочу выполнять только сравнениея флага, если метод уже зашел в какой-то if)
public void Pay(int value)
{
var flag = true;
if (flag && PeopleType == PeopleType.YoungMan)
{
Deposit -= value;
flag = false;
}
if (flag && PeopleType == PeopleType.Man)
{
Deposit -= value;
flag = false;
}
if (flag && PeopleType == PeopleType.OldMan)
{
Deposit -= value;
flag = false;
}
}
Заменяем
public void Pay(int value)
{
if (PeopleType == PeopleType.YoungMan)
{
Deposit -= value;
return;
}
if (PeopleType == PeopleType.Man)
{
Deposit -= value;
return;
}
if (PeopleType == PeopleType.OldMan)
{
Deposit -= value;
return;
}
}
На самом деле все if в системе деляется на два типа - первый когда выполения этого условия это что-то необычное, тогда мы кидаем Exception
if(...)
thro ... tion();
и когда все if'ы равновероятны, и могут быть вызваны - это пример выше, замечу что во втором случае поток выполнения вполне может пойти дальше, и если это уже не имеет смысла (предыдущий пример, если зашли в один if - то уже не зайдем в другой) нужно также явно указать это используя return, также бывает полезно все маловероятные условия помещать в начало метода
Избегайте вложенных условий if, если такие встречаются попробуйте их распрямить, все маловероятные условия поставьте первыми и выполните return Было:
public void Pay(int value)
{
if (PeopleType == PeopleType.DeadMan)
return;
else if (PeopleType == PeopleType.Child)
return;
else if (PeopleType == PeopleType.Teenager)
return;
Deposit -= value;
}
Стало:
public void Pay(int value)
{
if (PeopleType == PeopleType.DeadMan)
return;
if (PeopleType == PeopleType.Child)
return;
if (PeopleType == PeopleType.Teenager)
return;
Deposit -= value;
}
Либо вы можете сгенерировать исключение, и заменить пример из Decompose Conditional на обратный
public void Pay(int value)
{
if (IsInsolventPerson())
throw new NotSupportedException();
Deposit -= value;
}
public bool IsInsolventPerson() => PeopleType == PeopleType.DeadMan || PeopleType == PeopleType.Child ||
PeopleType == PeopleType.Teenager;
Что будет правильнее
Если в методе встречается switch или много if'ов, и в дальнейшем эта часть системы будет изменяться, то нужно выделить несколько наследников исходного класса, в которых определить поведение, которое раньше определялось и swith'e или if'ax
Можно применить Replace Class with Subclasess, если иерархия объектов уместна, либо Replace Class with State or Strategy если объект может переходить из одного состояние в другое во время жизни
Replace Class with Subclasess
Replace Class with State or Strategy