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

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

Синтаксис будет на 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(decimal amount);
    }
    void PrintUserDetails(decimal amount)
    {
        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(amount == 0 && useDefaultDiscount)
        {
            amount = 10;
        }
        return amount;
    }
    

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

  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 сила, как правило другой программист (желательно с большим опытом) должен высказать своё мнение.

Про конфликты интересов и их решения расскажу в следующей статье.
Share post
AdBlock has stolen the banner, but banners are not teeth — they will be back

More
Ads

Comments 28

    +7
    Удивительно, насколько творчески переработаны примеры из книги Фаулера. Впервые вижу такое глубинное переосмысление классики.
      +1
      Спасибо, хотя внимательный глаз заметит, что тут ессно больше от Фаулера, чем моего, от классики никуда не денешься.
      Ну а в общем, конечно пытался вложить своё видение данных критериев
      +1
      У вас в 7 примере «Удаление присвоений параметрам» перепутаны имена amount и value.
        0
        Автору:
        Как понял, вы хотите цикл статей создать. Попросил бы также затронуть тему рефакторинга 'Duplicate Observed Data'. Дело в том что нужно отделять код интерфейсной части от кода бизнес-логики. В этом рефакторинге предлагается создать новый класс предметной области и сделать интерфейсный класс его наблюдателем. НО. Мы же можем сделать и наоборот, а иногда это дает выигрыш. Наоборот когда класс предметной области становится наблюдателем интерфейсного класса. Возникает вопрос: на основании каких критериев следует принимать решение о том кто должен стать наблюдателем, интерфейсный или предметный?
          0
          Да, есть и такая техника, поэтому обязательно расскажу и про неё.
          0
          Мне кажется что в п.7 пропущены 'return', ну или надо было просто снабдить комментарием '// много кода'
            0
            Спасибо, поправил.
            0
            Хоть убей, но мне не нравится создавать новый метод на каждый чих. Да это улучшает читабельность, но ведь метод это не только исходный код — это вызов подпрограммы со своим контекстом и мало того, что приходится переносить контекст, так еще и просто так увеличиваем время выполнения. По моему мнению IDE должны давать возможность создавать именнованные блоки ради таких целей, которые можно сворачивать.
              0
              >>это вызов подпрограммы со своим контекстом и мало того, что приходится переносить контекст, так еще и просто так увеличиваем время выполнения
              Скорость работы конечно падает, вопрос только на сколько сильно? Можете привести реальную статистику с «лишними» функциями и без них? Верить надо только профайлеру.
                0
                К примеру, если функция вызывается в цикле, то достаточно сильно, а для интерпретируемых языков вообще беда (да я знаю про байт-коды).
                  0
                  Да, вы безусловно правы! Но преждевременная оптимизация что? ;) Правильно ЗЛО! Вы сдаете фичу как правило тогда, когда Вы завершили по ней хоть какую-либо работу. Далее ваш пропускается через руки тестировщика. Уверяю Вас он найдет повод побеспокоиться, если у Вас где-то что-то медленно! Вот когда Вам реально скажут, что именно тормозит. Вот тогда и следует вернуть код из ф-ции и проинлайнить его обратно на место!
                  Задача рефакторинга дать Вам вменяемого качества код, в котором можно будет достаточно быстро разобраться. Чтобы потом, можно было быстро воткнуть туда оптимизацию где она реально нужна!

                  Попробую привести аналогию, мы программисты иногда благодаря аналогиям быстрее понимаем о чем идет речь.
                  Рефакторинг, на мой взгляд, очень похож на ситуацию когда Вам нужны все книги и справочники по программированию, Вы их с жесткого диска не удаляете и в текущий момент времени вы их даже не открываете. Держите открытыми в данный момент только один два справочника. Потому что именно они Вам сейчас нужны! А представляете если бы было открыто 200 книг всей Вашей коллекции, еще столько же вкладок в вашем браузере и еще на рабочем столе В вашей берлоге где лежит ноутбук за которым работаете все книги с книжных полок. Что это будет? Это будет Хаос! В этом всем Вы будете быстрее раздражаться, т.к. найти что-то нужное будет сложно. Из-за раздражения будет теряться Ваше желание работать и вы быстрее будете уставать. Может все-таки уборка не такое уж и плохое занятие? Ведь когда Вам понадобиться мануал по алгоритмам загуглить не долго!

                  Последней аналогией, хотел сказать, что Вам не сложно будет «нагуглить» то место которое нужно будет оптимизировать, но благодаря порядку в исходном коде Вы быстрее это сделаете! Да и профайлер прямо носом ткнет, если конечно пользоваться умеете им
                    0
                    Т.е. Я должен написать красивый код, а потом узнать, что он выполняется медленно и снова сделать его некрасивым? Ужс)
                      0
                      Именно так. Причем оптимизацию можно делать только 1) проверяя под все целевые архитектуры 2) проверяя под всеми используемыми компиляторами с нужными параметрами сборки.
                        0
                        Да ну вы шутите :-) Мне не нравится такой подход. Лучше я буду писать сразу красивый код ориентируясь на возможности и потребности платформ.
                          0
                          Вы нас тролите! Не стоит )
                            0
                            Эмм, нет не троллю, я и правда считаю, что сначала писать код, а потом начинать думать — это очень странный подход к разработке.
                              0
                              Думать надо сразу.
                              Но жертвовать понятностью кода для повышения производительности нужно только тогда, когда это реально неоходимо. В остальных случаях достаточно заложить возможность дальнейшей оптимизации. В этом очень помогает инкапсуляция алгоритмов.
                                0
                                Скажите мне на чем вы пишите и я скажу в чем у вас проблема :-) Подозреваю, что вы просто не компилируете исходный код и получается, что рабочий код у вас не отличается от кода разработки?
                                  0
                                  Java.
                                  Если не компилировать, не запускать и не тестировать — тогда зачем вообще писать?
                                  И вообще, не вижу связи между моим комментарием и вашим. Может, поясните?
                                    0
                                    Вы пишите, что у вас бывают ситуации, когда вы жертвуете понятностью кода для производительности.

                                    Я утверждаю, что этого можно избежать практически в любом языке программирования.

                                    Т.е. вы пишите красивый код и не трогаете его, а все проблемы с производительностью решаете за счет грамотной компиляции, препроцессоров, макросов и т.д.

                                    А красивый код автоматически подразумевает нормальные уровни абстракции и создание библиотек на каждом из них.

                                    И, конечно, есть много языков которые с трудом тянут нормальную кодогенерацию и оптимизацию кода (называть не буду чтобы не будить спящего зверя :-)
                                0
                                Могу поделиться своим исходным кодом, который я писал думая о производительности кода в тот момент, когда я его писал. Хотите его почитать? ;)
                  0
                  Конечно на каждый чих не стоит, для Ассемблера это вообще критично.
                  Тут самое главное, уметь определить момент когда нужно это сделать (и это касается любого рефакторинга). А такой момент может вообще не наступить.
                  0
                  >>Синтаксис будет на C#, но главное понимать идею, а её можно использовать в любом другом языке программирования.
                  ну и зачем? Т.е. тоже самое можно написать про оригинал книги, там код на Smaltalk, С++, Java но также все применимо к другим ЯП, нужно лишь понимать методику. В чем смысл ваших постов? Пересказать классику на новый лад? Мда…
                    0
                    Во-первых не все читали/дочитали классику. Это tutorial для них.
                    Во-вторых, нового лада тут нет, это старый добрый Фаулер. Напоминание для тех кто в теме.
                    И поскольку постов по данной книге/тематике не обнаружил, захотелось написать.
                    Не исключаю конечно, что для кого-то, это является неинтересным и тривиальным.
                      0
                      Я уже намекал на практику, намекну еще. Взять к примеру непонятный код из какого-нить проекта на гитхабе и зарефакторить его. Вот это думаю будет достаточно понятный «на пальцах» мануал )
                        0
                        Если не читали классику — пусть идут и читают.
                        Иначе будет нахватывание рецептов по верхушкам без понимания сути.
                      +1
                      return GetSumm() != 0 ? GetSumm() * 100 : 1; А не лучше ли сохраниться в промежуточную переменную. В данном примере конечно вычисления простые, но GetSumm с тем же успехом может происходить достаточно сложные расчеты…
                        0
                        Да тут вы правы. И в примере как раз и показано 2 варианта, можно сохраниться во временную переменную и можно обойтись без этого.
                        Тут всё индивидуально (факторов много, наберётся на отдельную статью), хотя в 9 из 10 случаев мы пользуемся временной переменной.

                      Only users with full accounts can post comments. Log in, please.