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

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

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

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

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

void PrintUserInfo(decimal amount)
{
    PrintParentInfo();

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

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

Встраивание метода (поместите тело метода в код, который его вызывает и удалите метод).
Излишняя косвенность (разбивка на методы) может усложнить класс.
From
```
int GetPoints()
{
    return HasMaxSum() ? decimal.One : decimal.Zero;
}
bool HasMaxSum()
{
    return summ >= maxSumm;
}
```
To
```
int GetPoints()
{
    return summ >= maxSumm ? decimal.One : decimal.Zero;
}
```
Встраивание временной переменной (замените этим выражением, все ссылки на данную переменную).
Лишний промежуточный код.
From
```
decimal cost = order.GetCost();
return cost > 1000;
```
To
```
return order.GetCost() > 1000;
```

Замена временной переменной вызовом метода (преобразуйте выражение в метод).
Метод может вызываться в любом месте класса, если таких мест много.
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;
}

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

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

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

bool isLastFocusedColumn = VisualItems.ColumnCount == FocusedCell.X;
bool altShiftPressed = (key == Keys.Alt | Keys.Shift);
bool wasResized = resize > 0;

if(isLastFocusedColumn && altShiftPressed && WasInitialized() && wasResized)
{
    // do something
}

Расщепление поясняющей переменной (создайте для каждого присвоения отдельную временную переменную).
Упрощается чтение и понимание кода.
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);
```

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

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

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

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

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

        if(condition)
        {
            // sending message            
        }
    }
}

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

Замещение алгоритма (замените алгоритм на более понятный).
Оптимизация кода для лучшей читаемости и/или производительности.
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 "";
    }
}

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

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