Skip to content

Instantly share code, notes, and snippets.

  • Save eterekhin/6e1d19419aa8b68706af1a0b40d87867 to your computer and use it in GitHub Desktop.
Save eterekhin/6e1d19419aa8b68706af1a0b40d87867 to your computer and use it in GitHub Desktop.
Fowler.Decompose Conditional.Consolite Conditional Expression.Consolidate Duplicate Conditional Fragment.Remove Control Flag.Replace Nested Conditional with Guard Clauses. Replace Conditional with Polimorphism

Decomposite Conditional

Простой рефакторинг, если в 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;
        }

Consolite Conditional Expression

Если идут несколько условий подряд:

      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;
        }

Consolidate Duplicate Conditional Fragment

Простой метод, если в теле нескольких операторов 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 слишком много строк

Remove Control Flag

Не бойтесь прервать поток выполнения метода и выйти из него при помощи 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, также бывает полезно все маловероятные условия помещать в начало метода

Replace Nested Conditional with Guard Clauses

Избегайте вложенных условий 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;

Что будет правильнее

Replace Conditinal with Polimorphism

Если в методе встречается 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment