Недавно я смотрел «Все мелочи» из RailsConf 2014. Во время этого выступления Сэнди Метц реорганизует функцию, которая включает в себя большой вложенный оператор if:
def tick
if @name != 'Aged Brie' && @name != 'Backstage passes to a TAFKAL80ETC concert'
if @quality > 0
if @name != 'Sulfuras, Hand of Ragnaros'
@quality -= 1
end
end
else
...
end
...
end
Первый шаг - разбить функцию на несколько более мелких:
def tick
case name
when 'Aged Brie'
return brie_tick
...
end
end
def brie_tick
@days_remaining -= 1
return if quality >= 50
@quality += 1
@quality += 1 if @days_remaining <= 0
end
То, что я нашел интересным, было способом, которым были написаны эти меньшие функции. brie_tick
Например, был написан не путем извлечения соответствующих частей исходной tick
функции, а с нуля, ссылаясь на test_brie_*
модульные тесты. Как только все эти юнит-тесты пройдены, brie_tick
считается выполненным. Как только все маленькие функции были выполнены, исходная монолитная tick
функция была удалена.
К сожалению, докладчик, казалось, не знал, что этот подход привел к тому, что три из четырех *_tick
функций были неправильными (а другая была пустой!). Существуют крайние случаи, в которых поведение *_tick
функций отличается от поведения исходной tick
функции. Например, @days_remaining <= 0
в brie_tick
должно быть < 0
- так brie_tick
не работает правильно при вызове с days_remaining == 1
и quality < 50
.
Что здесь пошло не так? Это провал тестирования - потому что не было тестов для этих конкретных крайних случаев? Или провал рефакторинга - потому что код должен был быть преобразован шаг за шагом, а не переписан с нуля?
источник
Ответы:
Обе. Рефакторинг с использованием только стандартных шагов из оригинальной книги Фаулерса определенно менее подвержен ошибкам, чем переписывание, поэтому часто предпочтительнее использовать только эти виды детских шагов. Даже если для каждого граничного случая нет модульных тестов, и даже если среда не обеспечивает автоматическое рефакторинг, одно изменение кода, такое как «вводная переменная объяснения» или «функция извлечения», имеет гораздо меньший шанс изменить подробности поведения существующий код, чем полная перезапись функции.
Однако иногда переписывание фрагмента кода - это то, что вам нужно или нужно делать. И если это так, вам нужны лучшие тесты.
Обратите внимание, что даже при использовании инструмента рефакторинга всегда существует определенный риск появления ошибок при изменении кода, независимо от применения меньших или больших шагов. Вот почему рефакторинг всегда нуждается в тестах. Также обратите внимание, что тесты могут только уменьшить вероятность ошибок, но никогда не доказать их отсутствие - тем не менее, использование методов, таких как просмотр кода и покрытие ветвлений, может дать вам высокий уровень достоверности, а в случае перезаписи раздела кода это Часто стоит применять такие приемы.
источник
brie_tick
этом никогда не проверять проблемный@days_remaining == 1
случай, например, с помощью@days_remaining
установки на10
и-10
.Одна из вещей, которая действительно трудна в работе с унаследованным кодом: получение полного понимания текущего поведения.
Устаревший код без тестов, которые ограничивают все варианты поведения, является распространенным явлением в дикой природе. Что оставляет вас в догадке: означает ли это, что неограниченное поведение - это свободные переменные? или требования, которые не указаны?
Из разговора :
Это более консервативный подход; если требования могут быть недооценены, если тесты не отражают всю существующую логику, то вы должны быть очень и очень осторожны в том, как будете действовать.
Наверняка вы можете утверждать, что если тесты неадекватно описывают поведение системы, то у вас «провал тестирования». И я думаю, что это справедливо - но на самом деле не полезно; это обычная проблема в дикой природе.
Проблема не совсем в том, что преобразования должны были быть пошаговыми; скорее, выбор инструмента рефакторинга (оператор клавиатуры? а не управляемая автоматизация) не соответствовал охвату тестами из-за более высокого уровня ошибок.
Эту проблему можно было бы решить либо с помощью инструментов рефакторинга с более высокой надежностью, либо путем введения более широкого набора тестов для улучшения ограничений в системе.
Поэтому я думаю, что ваше соединение плохо выбрано;
AND
неOR
.источник
Рефакторинг не должен изменять внешне видимое поведение вашего кода. Это цель.
Если ваши юнит-тесты не пройдены, это означает, что вы изменили поведение. Но прохождение юнит-тестов никогда не является целью. Это помогает более или менее достичь вашей цели. Если рефакторинг изменяет внешне видимое поведение и все модульные тесты проходят успешно, то ваш рефакторинг не удался.
Рабочие юнит-тесты в этом случае только дают неверное ощущение успеха. Но что пошло не так? Две вещи: рефакторинг был небрежным, а юнит-тесты были не очень хорошими.
источник
Если вы определили «правильный» как «тест пройден», то по определению нет ничего плохого в том, чтобы изменить непроверенное поведение.
Если определенное поведение ребра должно быть определено, добавьте тест для него, если нет, тогда все в порядке, чтобы не заботиться о том, что происходит. Если вы действительно педантичны, вы можете написать тест, который проверяет,
true
когда в этом крайнем случае документирует, что вас не волнует, каково поведение.источник