Техника: Составление методов (рефакторинг М. Фаулера)

Начало Код с душком (рефакторинг М. Фаулера) ^[1].
В продолжении, техника рефакторинга по книге Рефакторинг. Улучшение существующего кода Мартин Фаулер ^[2].

Синтаксис будет на C#, но главное понимать идею, а её можно использовать в любом другом языке программирования.

Составление методов.

1. Выделение метода (преобразуйте фрагмент кода в метод, название которого объясняет его значение).
   Длинный метод разбиваем на логические под-методы.
   From

   void PrintUserInfo(decimal amount)
   {
       PrintParentInfo();
   
       Console.WriteLine(string.Format("имя: {0}", name);
       Console.WriteLine(string.Format("возраст: {0}", age);
       Console.WriteLine(string.Format("кол-во: {0}", amount);
   }
   

   To

   void PrintUserInfo(decimal amount)
   {
       PrintParentInfo();
       PrintUserDetails();
   }
   void PrintUserDetails()
   {
       Console.WriteLine(string.Format("имя: {0}", name);
       Console.WriteLine(string.Format("возраст: {0}", age);
       Console.WriteLine(string.Format("кол-во: {0}", amount);
   }
   

2. Встраивание метода (поместите тело метода в код, который его вызывает и удалите метод).
   Излишняя косвенность (разбивка на методы) может усложнить класс.
   From

   int GetPoints()
   {
       return HasMaxSum() ? decimal.One : decimal.Zero;
   }
   bool HasMaxSum()
   {
       return summ >= maxSumm;
   }
   

   To

   int GetPoints()
   {
       return summ >= maxSumm ? decimal.One : decimal.Zero;
   }
   

3. Встраивание временной переменной (замените этим выражением, все ссылки на данную переменную).
   Лишний промежуточный код.
   From

   decimal cost = order.GetCost();
   return cost > 1000;
   

   To

   return order.GetCost() > 1000;
   

4. Замена временной переменной вызовом метода (преобразуйте выражение в метод).
   Метод может вызываться в любом месте класса, если таких мест много.
   From

   decimal MethodA()
   {
       decimal summ = amount * cost;
       if(summ > 1000)
       {
           //do something
           return summ * 10;
       }
       return 0;
   }
   decimal MethodB()
   {
       //do something
   
       decimal summ = amount * cost;
   
       return summ != 0 ? summ * 100  : 1;
   }
   

   To

   decimal MethodA()
   {
       decimal summ = GetSumm();
       if(summ  > 1000)
       {
           //do something
           return summ * 10;
       }
       return 0;
   }
   decimal MethodB()
   {
       //do something
   
       return GetSumm() != 0 ? GetSumm() * 100  : 1;
   }
   
   decimal GetSumm()
   {
       return amount * cost;
   }
   

5. Введение поясняющей переменной (поместите результат выражения или его части во временную переменную).
   Упрощается чтение и понимание кода.
   From

   if(VisualItems.ColumnCount == FocusedCell.X && (key == Keys.Alt | Keys.Shift) && WasInitialized() && (resize > 0))
   {
       // do something
   }
   

   To

   bool isLastFocusedColumn = VisualItems.ColumnCount == FocusedCell.X;
   bool altShiftPressed = (key == Keys.Alt | Keys.Shift);
   bool wasResized = resize > 0;
   
   if(isLastFocusedColumn && altShiftPressed && WasInitialized() && wasResized)
   {
       // do something
   }
   

6. Расщепление поясняющей переменной (создайте для каждого присвоения отдельную временную переменную).
   Упрощается чтение и понимание кода.
   From

   decimal temp = 2 * (height + width);
   Console.WriteLine(temp);
   temp = height * width;
   Console.WriteLine(temp);
   

   To

   decimal perimetr = 2 * (height + width);
   Console.WriteLine(perimetr);
   decimal area = height * width;
   Console.WriteLine(area);
   

7. Удаление присвоений параметрам (лучше воспользоваться временной переменной).
   Методы не должны менять значения входящих параметров, если это явно не указано (например out, ref в C#).
   From

   int Discount(int amount, bool useDefaultDiscount, DateTime date)
   {
       if(value == 0 && useDefaultDiscount)
       {
           value = 10;
       }
   }
   

   To

   int Discount(int amount, bool useDefaultDiscount, DateTime date)
   {
       int result = amount;
       if(value == 0 && useDefaultDiscount)
       {
           result = 10;
       }
   }
   

8. Замена метода объектом методов (преобразуйте метод в отдельный объект).
   Декомпозиция класса для гибкости кода.
   From

   class MessageSender
   {
       public Send(string title, string body)
       {
           // do something
           // long calculation of some condition
   
           if(condition)
           {
               // sending message            
           }
       }
   }
   

   To

   class MessageSender
   {
       public Send(string title, string body, decimal percent,  int quantity)
       {
           // do something
           Condition condition = new Condition (percent, quantity, dateTime);
   
           if(condition.Calculate())
           {
               // sending message            
           }
       }
   }
   
   class Condition
   {
       public Condition(decimal percent, int quantity, DateTime dt)
       {
       }
   
       public bool Calculate()
       {
           // long calculation of some condition
       }
   }
   

9. Замещение алгоритма (замените алгоритм на более понятный).
   Оптимизация кода для лучшей читаемости и/или производительности.
   From

   string FoundPerson(List<string> people)
   {
       foreach(var person in people)
       {
           if(person == "Max")
           {
                return "Max";
           } 
           else if(person == "Bill")
           {
                return "Bill";
           }
           else if(person == "John")
           {
                return "John";
           }
           return "";
       }
   }
   

   To

   string FoundPerson(List<string> people)
   {
       var candidates = new List<string>{ "Max",  "Bill", "John"};
       foreach(var person in people)
       {
           if(candidates.Contains(person))
           {
               return person;
           }
       }
       return "";
   }
   

Немного реальности и процедуре code review.

Перед рефакторингом или для проверки внесенных изменения применяется процедура code review.
В данном случае, речь пойдёт о «замечаниях» (критериях) review-ера.

• Необходимо обязательно поправить.
  Как правило, это очевидные вещи, которые могут приводить: к исключениям (например отсутствие проверок на null), к просадке производительности, к изменению логики работы программы.
  Сюда же можно отнести и архитектурные штрихи. Например если у вас весь проект написан на MVC/MVP, а новый коллега добавил форму, заколбасив всё в один файл/класс. Чтобы не создавать прецедент и следовать правилам, конечно лучше этот спагетти-код переделать под общую архитектуру.
• Замечания-вопросы.
  Если вам, что-то не понятно, не стесняйтесь задавать вопросы: почему именно такое решение, почему выбран такой алгоритм, зачем такие навороты. Вопросы полезны как для общего развития, можно освоить полезную фичу или лучше узнать текущий код. Так и для понимания того, что коллега выбрал решение осознанно.
• Замечания-предложения.
  Тут высказываются предложения по улучшению кода, но если code commiter проигнорирует их, то я не буду настаивать.
  Если же мне как code commiter-у что-то предлагают, я чаще стараюсь сделать это. Тут я исхожу из того соображения, что review-ер это преподаватель, который проверяет код и он прав, конечно если предложения не доходит до абсурда.
  Что-то предлагая, можно это аргументировать кодом для наглядности.

Реальность такова, что грань между этими критериями может быть разная для каждого программиста, но это терпимо — нормально. Хуже когда у review-ера отсутствует 3-й критерий, т.е. он всегда прав, а если не прав, то докажите ему это (но не всё доказательно, всё-таки не математика). Становясь уже code commiter-ом он наоборот, только и применяет 3-ий критерий.
А когда сталкиваются два таких человека, это флейм, крики (видел и с матерком :)).
Чтобы разрешить конфликт, нужна 3 сила, как правило другой программист (желательно с большим опытом) должен высказать своё мнение.

Про конфликты интересов и их решения расскажу в следующей статье.

Автор: den_labs

Источник ^[3]