Бибичев Андрей2008 год, декабрь

www.custis.ru

Безудержный REFACTORINGили как не убиться об стену

НА ПРАВАХ ВВЕДЕНИЯ

Определение

Поводы

Каталог

Самые популярные

Автоматический рефакторинг

«Безудержный Refactoring», (с) 2008 2 из 77

Классический труд

«Безудержный Refactoring», (с) 2008 3 из 77

XP

Цитируем wikipedia…

Рефакторинг или Реорганизация — процесс полного или частичного преобразования внутренней структуры программы при сохранении еѐ внешнего поведения.

В его основе лежит последовательность небольших эквивалентных(т.е., сохраняющих поведение) преобразований

Источник: http://ru.wikipedia.org/wiki/Рефакторинг

«Безудержный Refactoring», (с) 2008 4 из 77

Продолжаем цитировать …

Рефакторинг изначально не предназначен для исправления ошибок и добавления новой функциональности, но помогает избежать ошибок и облегчить добавление функциональности. Он выполняется для улучшения понятности кода или изменения его структуры, для удаления «мѐртвого кода» — всѐ это для того, чтобы в будущем код было легче поддерживать и развивать

Источник: http://ru.wikipedia.org/wiki/Рефакторинг

«Безудержный Refactoring», (с) 2008 5 из 77

Откуда может возникнуть необходимость в рефакторинге?

«Безудержный Refactoring», (с) 2008 6 из 77

1. Унаследованный (legacy) код

«Безудержный Refactoring», (с) 2008 7 из 77

«Код надо писать так, как будто его будет поддерживать и сопровождать законченный маньяк, который знает ваш домашний адрес…»

2. «Второй проход» по свеженаписанной функциональности

«Безудержный Refactoring», (с) 2008 8 из 77

«Первое лезвие бреет чисто,второе – еще чище»

3. Инкрементальный дизайн

«Безудержный Refactoring», (с) 2008 9 из 77

Так как это делать?- Есть целый каталог рефакторингов!!!

«Безудержный Refactoring», (с) 2008 10 из 77

http://www.refactoring.com/catalog/index.html

• Add Parameter

• Change Bidirectional Association to Unidirectional

• Change Reference to Value

• Change Unidirectional Association to Bidirectional

• Change Value to Reference

• Collapse Hierarchy

• Consolidate Conditional Expression

• Consolidate Duplicate Conditional Fragments

• Convert Dynamic to Static Construction by Gerard M. Davison

• Convert Static to Dynamic Construction by Gerard M. Davison• Decompose Conditional

• Duplicate Observed Data

• Eliminate Inter-Entity Bean Communication (Link Only)• Encapsulate Collection

• Encapsulate Downcast

• Encapsulate Field

• Extract Class

• Extract Interface

• Extract Method

• Extract Package by Gerard M. Davison• Extract Subclass

• Extract Superclass

• Form Template Method

• Hide Delegate

• Hide Method

• Hide presentation tier-specific details from the business tier (Link Only)

• Inline Class

• Inline Method

• Inline Temp

• Introduce A Controller (Link Only)• Introduce Assertion

• Introduce Business Delegate (Link Only)• Introduce Explaining Variable

• Introduce Foreign Method

• Introduce Local Extension

• Introduce Null Object

• Introduce Parameter Object

• Introduce Synchronizer Token (Link Only)

• Localize Disparate Logic (Link Only)• Merge Session Beans (Link Only)

• Move Business Logic to Session (Link Only)• Move Class by Gerard M. Davison• Move Field

• Move Method

• Parameterize Method

• Preserve Whole Object

• Pull Up Constructor Body

• Pull Up Field

• Pull Up Method

• Push Down Field

• Push Down Method

• Reduce Scope of Variable by Mats Henricson• Refactor Architecture by Tiers (Link Only)

• Remove Assignments to Parameters

• Remove Control Flag

• Remove Double Negative by Ashley Frieze and Martin Fowler• Remove Middle Man

• Remove Parameter

• Remove Setting Method

• Rename Method

• Replace Array with Object

• Replace Assignment with Initialization by Mats Henricson

• Replace Conditional with Polymorphism

• Replace Conditional with Visitor by Ivan Mitrovic• Replace Constructor with Factory Method

• Replace Data Value with Object

• Replace Delegation with Inheritance

• Replace Error Code with Exception

• Replace Exception with Test

• Replace Inheritance with Delegation

• Replace Iteration with Recursion by Dave Whipp• Replace Magic Number with Symbolic Constant

• Replace Method with Method Object

• Replace Nested Conditional with Guard Clauses

• Replace Parameter with Explicit Methods

• Replace Parameter with Method

• Replace Record with Data Class

• Replace Recursion with Iteration by Ivan Mitrovic• Replace Static Variable with Parameter by Marian Vittek

• Replace Subclass with Fields

• Replace Temp with Query

• Replace Type Code with Class

• Replace Type Code with State/Strategy

• Replace Type Code with Subclasses

• Reverse Conditional by Bill Murphy and Martin Fowler• Self Encapsulate Field

• Separate Data Access Code (Link Only)• Separate Query from Modifier

• Split Loop by Martin Fowler

• Split Temporary Variable

• Substitute Algorithm

• Use a Connection Pool (Link Only)• Wrap entities with session (Link Only)

«Безудержный Refactoring», (с) 2008 11 из 77

Наиболее популярные

• Rename• Extract method• Change signature• Encapsulate field• Introduce variable

• Extract superclass• Replace constructor with factory

method

«Безудержный Refactoring», (с) 2008 12 из 77

Современные среды автоматизируют выполнение многих рефакторингов,

избавляя от рутины

«Безудержный Refactoring», (с) 2008 13 из 77

Даже автоматические рефакторингине гарантируют полную корректность

«Безудержный Refactoring», (с) 2008 14 из 77

• Чисто синтаксического анализа кода не хватает – нужен семантический (и зачастую весьма продвинутый)

• Далее пример весьма распространенной ситуации с переименованием члена класса, имя которого используется как литерал

• Современные инструменты решают эту проблему, но в полуавтоматическом режиме (т.е. остается место для ошибки!)

«Безудержный Refactoring», (с) 2008 15 из 77

КОД С ДУШКОМ

Примеры «запахов»

«Броузинг» при чтении кода

Дублирование кода:пример выполнения рефакторинга

Всегда ли плохо дублирование?

«Безудержный Refactoring», (с) 2008 16 из 77

«Код с душком» (запахи, smells)

• Имя не отражает функциональность• Тыква => Карета

• Длинные методы• Что такое «длинные»? Несколько экранов?

• Есть и обратная ситуация (много мелких методов)

• Дублирование кода

«Безудержный Refactoring», (с) 2008 17 из 77

Размер метода и концепция «броузинга» при чтении кода

«Безудержный Refactoring», (с) 2008 18 из 77

public void PrintSpecialText()

{

PrintAlfa();

PrintBeta();

PrintGamma();

}

private void PrintAlfa()

{

Console.WriteLine("alfa");

}

private void PrintBeta()

{

Console.WriteLine("beta");

}

private void PrintGamma()

{

Console.WriteLine("gamma");

}

• Много мелких приватных методов, используемых в одном месте – тоже плохо(на ровном месте увеличивается «косвенность» при чтении кода)

• Обратите внимание,что для многих рефакторингов есть обратные:

• Extract Method• Inline Method

Самый популярный «запах»- дублирование кода

«Безудержный Refactoring», (с) 2008 19 из 77

Пример из реального кода

• C# 1

• WinForms-приложение• Метод вычисления стиля

элемента грида (по XML-описанию)

«Безудержный Refactoring», (с) 2008 20 из 80

«Безудержный Refactoring», (с) 2008 21 из 77

«Безудержный Refactoring», (с) 2008 22 из 80

«Безудержный Refactoring», (с) 2008 23 из 77

Итак, как было

«Безудержный Refactoring», (с) 2008 24 из 77

...

NewStyleFromXML(RowCol, …)

NewStyleFromXML(inout CellRange, …)

...

…

XMLTableHelper...

get_Style() : CellStyle

set_Style(CellStyle)

...

…

RowCol

…

...

Style : CellStyle

...

<<value>>

CellRange

К чему преобразуем

«Безудержный Refactoring», (с) 2008 25 из 77

XMLTableHelper

get_Style() : CellStyle {abstract}

get_NewClearStyle() : CellStyle {abstract}

NewStyleFromXML(XMLNode, bool)

StyledElement

.ctor(RowCol, …)

get_Style() : CellStyle

get_NewClearStyle() : CellStyle

StyledElementForRowCol

.ctor(CellRange, …)

get_Style() : CellStyle

get_NewClearStyle() : CellStyle

StyledElementForCell

...

get_Style() : CellStyle

set_Style(CellStyle)

...

…

RowCol

…

...

Style : CellStyle

...

<<value>>

CellRange

«Безудержный Refactoring», (с) 2008 26 из 77

«Безудержный Refactoring», (с) 2008 27 из 80

«Безудержный Refactoring», (с) 2008 28 из 80

«Безудержный Refactoring», (с) 2008 29 из 77

«Безудержный Refactoring», (с) 2008 30 из 77

А если бы был C# 2.0 или выше– то всё было бы проще!

• За нас всѐ сделал бы компилятор:– Анонимные делегаты

– Автоматические замыкания

– И не надо городить иерархии классов…

«Безудержный Refactoring», (с) 2008 31 из 77

«Безудержный Refactoring», (с) 2008 32 из 77

Дублирование кода– это всегда плохо?

Моѐ авторское мнение:

НЕ ВСЕГДА!

«Безудержный Refactoring», (с) 2008 33 из 77

CustIS::DataAccess

Cursor

CustIS::DataAccess::MySql

MySqlCursor

CustIS::DataAccess::OdpNet

OdpNetCursor

Пример

«Безудержный Refactoring», (с) 2008 34 из 77

Реализации похожи,

но должны быть независимы

Реализации похожи,

но должны быть независимы

Ингода борьба с дублированием кода начинает мне напоминать

«охоту на ведьм»

• Я видел много кода с большим дублированием, который было не сложно сопровождать и развивать

• И видел код вообще без дублирования, который очень сложно развивать и сопровождать

«Безудержный Refactoring», (с) 2008 35 из 77

АНТИПАТТЕРНЫ ПРОВЕДЕНИЯ РЕФАКТОРИНГА

Две крайности

Traceability исправлений

Чем плох«безудержный» рефакторинг

Пример

Темная сторона инструментов автоматического анализа кода

«Безудержный Refactoring», (с) 2008 36 из 77

Две крайности

«Безудержный Refactoring», (с) 2008 37 из 77

«Везде оставь свой след»убивает трассируемость изменений

«Безудержный Refactoring», (с) 2008 38 из 77

Кто изменил

Что изменил

Зачем изменил

• Имея номер бага (дела, issue) нужно уметь отвечать

• какие изменения в коде это повлекло?

• кто их сделал?

• А для фрагмента кода:

• в связи с чем он написан/исправлен?

• кто это сделал?

• По номеру ревизии:

• какие баги исправлены, какие фичи вошли?

«Безудержный Refactoring», (с) 2008 39 из 77

Источник: «Интеграция систем управления разработкой», Фомин С.

«Безудержный Refactoring», (с) 2008 40 из 80

Номер бага

К этой картинке хочется добавить: историю изменений тоже читают!

«Безудержный Refactoring», (с) 2008 41 из 77

Итак, избыточный рефакторинг может«зашумить» историю изменений

Далее следует пример

«Безудержный Refactoring», (с) 2008 42 из 77

«Безудержный Refactoring», (с) 2008 43 из 80

«Безудержный Refactoring», (с) 2008 44 из 80

«Безудержный Refactoring», (с) 2008 45 из 80

Что мы видели?

• Под видом рефакторинга проведено банальное «причесывание» кода, причем вкусовое (ни качества, ни понятности оно не добавило)

• То, что действительно следовало бы поменять (заменить сложные условия на методы, избавиться от литеральных строк в пользу строковых констант и т.д.) – не сделано

«Безудержный Refactoring», (с) 2008 46 из 77

Откуда это берется1. Смена «окружения»

– Новая версия языка/библиотек– Новые модные паттерны и/или изменение в

осознании «мировых истин»– Другой стандарт кодирования

2. Темная сторона инструментов автоматической проверки кода– Анекдот про то, кто такой зануда

3. «Я преобразую код, чтобы разобраться в нем»– Опасно, когда руки работают вперед головы!

«Безудержный Refactoring», (с) 2008 47 из 77

Даже 100% покрытие кодаUNIT-тестами не дает гарантий

• В императивных языках поведение одного и того же фрагмента кода может сильно отличаться в зависимости от «окружения» (side-эффекты)

• Особенно остро это чувствуется при:– многопоточном программировании– работе с СУБД– взаимодействии с внешними системами и

устройствами– GUI

«Безудержный Refactoring», (с) 2008 48 из 77

ТЕМНАЯ СТОРОНА АВТОМАТИЧЕСКОЙ

ПРОВЕРКИ КОДА

Пример советов R#

catch {}

Опасности

Опять семантика! Пример

«Безудержный Refactoring», (с) 2008 49 из 77

ReSharper (R#) – отличный инструментНо даже на солнце бывают пятна…

Далее идет пример его советов на фрагментах кода

«Безудержный Refactoring», (с) 2008 50 из 77

«Безудержный Refactoring», (с) 2008 51 из 80

To «var» or not to «var»

«Безудержный Refactoring», (с) 2008 52 из 77

Илья Рыженков

из JetBrains

перечисляет

ряд выгод от

использования

VAR

Dare Obasanjo

из RSS Bandit

не соглашается

А более «тяжеловесные» инструменты типа FxCop?

• Здесь любимый пример, когда они эффективны:

– Контроль соглашений по именованию• при этом семантику названия они проверить

не могут!

– Контроль типичных «ляпов» a la

catch (Exception) { /*пусто*/ }

«Безудержный Refactoring», (с) 2008 53 из 77

Пытался найти по кодам наших проектов, но обнаружил только вMS Visual Studio 2005 SDK ver. 4.0

Скачать можно отсюда:• http://www.microsoft.com/downloads/details.aspx?familyid=51A5C65B-C020-4E08-8AC0-

3EB9C06996F4&displaylang=en

Далее идут выдержки из файлов:

• .\VSSDK40\MPF\Shell\TaskProvider.cs

• .\VSSDK40\MPF\Shell\Package.cs

«Безудержный Refactoring», (с) 2008 54 из 77

«Безудержный Refactoring», (с) 2008 55 из 80

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

«Безудержный Refactoring», (с) 2008 56 из 77

А вот вообще «волшебный» фрагмент кода с такими же «волшебными» комментариями:- что-то пытаемся сделать- а если не смогли, то это оказывается не проблема! (и можно ничего не делать…)

- зачем тогда вообще пытались что-то сделать?!

«Безудержный Refactoring», (с) 2008 57 из 77

А вот всё же вставлена хоть какая-то диагностика.НО! Она будет только в Debug-варианте…

А как разбираться и диагностировать проблемы,имея на руках только Release?

Не лучше бы было писать файловый логили использовать Windows Event Log,который как раз и задуман на случаи, когда программе надо на что-топожаловаться, а куда жаловаться – не понятно ?!

Итак, опасности:

• Слепое следование советам этих инструментов

• Формальное исправление замечаний (без понимания, лишь бы отвязалась)

• Ложное чувство уверенности в качестве кода

• При этом, на настройку этих инструментов может уходить много сил

«Безудержный Refactoring», (с) 2008 58 из 77

«Безудержный Refactoring», (с) 2008 59 из 77

Опять же, не хватает семантики!

«Безудержный Refactoring», (с) 2008 60 из 77

• Пример из жизни достаточно распространенного и опасного ляпа, который не ловится инструментами автоматической проверки кода:

• Immutable (неизменный) класс для хранения многополевогопервичного ключа

«Безудержный Refactoring», (с) 2008 61 из 80

Этот класс весьма уязвим,и то, что _values объявлен как readonly –

не спасает:

«Безудержный Refactoring», (с) 2008 62 из 77

Исправление ситуации

«Безудержный Refactoring», (с) 2008 63 из 80

Книга насчитывает около сотни весьма ценных практических статей-советов, только процентов 5

из которых можно проверить автоматически…

«Безудержный Refactoring», (с) 2008 64 из 77

ТАК КАК ЖЕ БЫТЬ

Преемственность

Дополнения к стандарту кодирования

Общие аккуратные настройки инструментов

Code review, как эффективная профилактика

«Безудержный Refactoring», (с) 2008 65 из 77

1. Культивируйте преемственность

• Умение понимать и следоватьчужим идеям:

– прежде чем изменять код,его следует понять

– прежде чем реализовывать новую функциональность, поищите – не реализовано ли уже где-то что-то аналогичное

– если что-то не нравится в том, как аналогичную задачу решили до вас, то не надо молча делать новое по-своему, а старое оставлять по-прежнему (либо менять и там, и там, либо наступить себе на горло и делать аналогично старому)

«Безудержный Refactoring», (с) 2008 66 из 77

Про преемственность

Если до вас рыли норы, то не нужно начинать рыть траншеи только на том основании, что лично вам оно так удобнее

«Безудержный Refactoring», (с) 2008 67 из 77

2. Дополнения кстандарту кодирования

• Стандарт кодирования должен быть!

• Дополните его разделом «Внесение изменений в существующий код» примерно со следующим содержанием

«Безудержный Refactoring», (с) 2008 68 из 77

• следует уважать и по возможности сохранять авторский стиль форматирования:

– если объем исправлений мал и код не удовлетворяет текущим правилам форматирования, то исправления должны быть внесены в стиле форматирования существующего кода

– если объем исправлений достаточно велик, то новые куски могут быть отформатированы в соответствии с текущими представлениями о правильном стиле, а форматирование остальных частей должно быть оставлено как есть

– весь файл может быть переформатирован тольков случае почти полного его переписывания (т.е. почти полной замены старой реализации на новую)

«Безудержный Refactoring», (с) 2008 69 из 77

• формальное следование советам инструментов автоматического анализа кода не может считаться "выполнением рефакторинга"

– это допустимо выполнять (например, устранять warning-и ReSharper-а) только при условии необходимости внесения в данный файл существенных исправлений логики

(мелкий патчинг, который выражается в правке одной-пяти строк кода не может считаться существенным исправлением)

«Безудержный Refactoring», (с) 2008 70 из 77

• Объемные рефакторинги следует фиксировать отдельными (от исправлений функциональности) commit-ами с соответствующим комментарием

«Безудержный Refactoring», (с) 2008 71 из 80

3. Настройка инструментов

«Безудержный Refactoring», (с) 2008 72 из 77

• Обеспечьте, чтобы у всех разработчиков были одинаковые настройки используемых инструментов (форматирования и проверки кода)

• Причем эти настройки должны соответствовать принятым у вас соглашениям

«Безудержный Refactoring», (с) 2008 73 из 77

4. Code review:лучшее лечение – это профилактика

«Безудержный Refactoring», (с) 2008 74 из 77

сборочныйсервер

коллега аналитикили PO

демо

(1) автоматическиесборка + тесты

(2) Code Review(3) Сделано то, что нужно?Оно работает? Это удобно?

Feedback

Feedback

Советы по проведению Code Review:1. Code review выполнять обязательно и сразу

после реализации бага (как часть Defenition-of-Done)

2. Пары автор-проверяющий должны образовываться «самопроизвольно» и меняться

3. Найденные недочеты лучше изложить в системе ведения дел, чтобы сам автор их устранил. Если их много и они сложные –проверяющий и автор садятся за клавиатуру вместе– это важно, чтобы был «обучающий» эффект

(ошибки/проблемы больше не повторялись)– разногласия обсуждаются устно (можно с

привлечением других членов команды)

«Безудержный Refactoring», (с) 2008 75 из 77

Сильно повышает качество!Проверено практикой

«Безудержный Refactoring», (с) 2008 76 из 77

СПАСИБО ЗА ВНИМАНИЕ!ВОПРОСЫ?

Контакты: andrew@custis.ru

Материал опубликован: www.custis.ru

«Безудержный Refactoring», (с) 2008 77 из 77