Начало Код с душком (рефакторинг М. Фаулера) .
В продолжении, техника рефакторинга по книге Рефакторинг. Улучшение существующего кода Мартин Фаулер.
Синтаксис будет на C#, но главное понимать идею, а её можно использовать в любом другом языке программирования.
Составление методов.
- Выделение метода (преобразуйте фрагмент кода в метод, название которого объясняет его значение).
Длинный метод разбиваем на логические под-методы.
Fromvoid 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); }
- Встраивание метода (поместите тело метода в код, который его вызывает и удалите метод).
Излишняя косвенность (разбивка на методы) может усложнить класс.
Fromint GetPoints() { return HasMaxSum() ? decimal.One : decimal.Zero; } bool HasMaxSum() { return summ >= maxSumm; }
To
int GetPoints() { return summ >= maxSumm ? decimal.One : decimal.Zero; }
- Встраивание временной переменной (замените этим выражением, все ссылки на данную переменную).
Лишний промежуточный код.
Fromdecimal cost = order.GetCost(); return cost > 1000;
To
return order.GetCost() > 1000;
- Замена временной переменной вызовом метода (преобразуйте выражение в метод).
Метод может вызываться в любом месте класса, если таких мест много.
Fromdecimal 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; }
- Введение поясняющей переменной (поместите результат выражения или его части во временную переменную).
Упрощается чтение и понимание кода.
Fromif(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 }
- Расщепление поясняющей переменной (создайте для каждого присвоения отдельную временную переменную).
Упрощается чтение и понимание кода.
Fromdecimal 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#).
Fromint 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; } }
- Замена метода объектом методов (преобразуйте метод в отдельный объект).
Декомпозиция класса для гибкости кода.
Fromclass 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 } }
- Замещение алгоритма (замените алгоритм на более понятный).
Оптимизация кода для лучшей читаемости и/или производительности.
Fromstring 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