Пост «Удаление кода» Неда Бэтчелдера (Ned Batchelder) недавно появился на HN, хотя изначально он был написан в 2002 году. Здесь я хочу повторить несколько мыслей Неда, и занять более решительную, чем он, позицию: удаляйте код, как только вы замечаете, что он больше не нужен, без лишних вопросов. Я также предложу некоторые советы из окопов, как определять кандидатов в мертвый код.
То что мертво умереть не может!
Это не просто «очень умная» и своевременная отсылка к поп-культуре. Мертвый код, то есть код, который никогда не выполняется в вашей программе — это реальная помеха для поддержки вашей кодовой базы. Сколько раз вы не могли добавить что-то, что казалось простой функцией или улучшением, только потому что были поставлены в тупик сложностью кода, который должен работать рядом с этой функцией? Насколько приятнее была бы ваша жизнь, если бы добавить новую функцию или исправить ошибку было бы так же просто, как вы предполагали, когда планировали свою работу?
Каждый раз, когда вы хотите внести изменения, необходимо учитывать, как они взаимодействуют со всеми существующими функциями, «костылями», известными ошибками и ограничениями окружающего кода. Это легче сделать, имея меньше кода вокруг объекта, который вы хотите добавить, т.к. при этом нужно будет учитывать меньше вещей и будет меньше вариантов, что что-то пойдет не так. Мертвый код особенно вреден, потому что кажется, что нужно учитывать взаимодействие с ним, но, поскольку он мертв, это всего лишь отвлекающий маневр. Он не может принести пользу вам, так как никогда не выполняется.
Тот факт, что мертвый код может никогда не умереть — это реальная угроза для вашей способности работать с данной кодовой базой. В худшем случае, если код, который никогда не вызывается не будет удаляться, размер вашего приложения будет расти вечно. Прежде чем вы заметите, что, возможно, только несколько тысяч действительно функционирующих строк окружены на несколько порядков большим количеством кода, который не делает ничего полезного.
Он должен уйти
Нед (Бэтчелдер, не Старк) был немного более деликатным и тактичным, чем я в этой статье:
Скажем, у вас есть превосходный класс, который имеет кучу методов. В один прекрасный день вы обнаруживаете, что некий отдельный метод больше не вызывается. Вы оставите его или удалите?
В данном случае нет простого ответа на этот вопрос, потому что это зависит от класса и от метода. Ответ зависит от того, предполагаете ли вы, что этот метод может понадобиться снова в будущем.
Источник
Я скажу так: выжигайте землю и не оставляйте код в живых. Лучший код, это код которого у вас нет.
Если вы не такие смелые как я, помните, что системы контроля версий прикроют вас в случае, если вам когда-либо снова понадобится этот код.
Тем не менее, я никогда не испытывал необходимости возвращать что-то, что я уже удалил, по крайней мере, не в буквальном смысле возвращения строка за строкой, дословно, участка кода, который я ранее удалил.
Конечно, я не говорю о таких вещах, как возврат ошибочных коммитов — мы все люди, и я делаю так же много ошибок, как и другие. Я имею в виду, что я никогда не удалял объекты, отправленные в продакшен, а затем возвращал спустя несколько недель или месяцев, думая: «Ну и дела, похоже код, который я писал год или более назад был очень хорош, так что верну-ка я его обратно». Кодовая база живет и развивается с течением времени, поэтому старый код, вероятно, не вяжется с новыми идеями, техниками, фреймворками и стилем используемым сегодня. Я мог бы вернуться к старой версии для обновления, особенно, если это какие-то тонкие моменты, но я никогда не возвращал код обратно массово.
Так что сделайте одолжение себе и своей команде и удалите мертвый код как только заметите его.
Как мы сюда попали?
Пост Неда в подробностях объясняет, как и почему возникает «мертвый» код — возможно, тот, кто вносит изменения не считает, что код должен исчезнуть навсегда и комментирует его или добавляет условную компиляцию. Возможно, человек, делающий изменения не знает достаточно, чтобы понять, что код на самом деле мертв (об этом расскажу позже).
Добавлю еще одну гипотезу в список: мы все можем просто лениться. Безусловно, легче чего-то не делать (т. е. оставить как есть), чем что-то сделать (удалить).
Лень, в конце концов, одна из трех великих добродетелей программиста. Но лень, про которую говорит Ларри Уолл другого рода: «Качество, которое заставляет вас прилагать больше усилий, чтобы снизить общие затраты энергии». С этой точки зрения, удаление мертвого кода это Лень с большой буквы Л — делать что-то, что легко сделать сейчас, чтобы уберечь себя от необходимости делать что-то сложное в будущем. Мы все должны стараться развивать такую Лень. Мне нравится думать о ней, как о «дисциплинированной лени», нашей ежедневной привычке.
Как нам выбраться отсюда?
Я провожу большую часть своего времени программируя на Python, для которого, к сожалению, IDE обычно не могут правильно анализировать полную кодовую базу и автоматически находить никогда не вызываемый код. Но, сочетая дисциплину и некоторые инструменты для анализа программы во время выполнения (run-time tooling), мы можем подойти к этой проблеме с двух сторон.
В простых случаях, хорошее чувство кода может помочь выявить и удалить мертвый код в то время как вы вносите изменения. Представьте, что вы работаете над определенной функцией и замечаете, что одна из веток if/else никогда не может быть выполнена. Я называю это «dead code in the small»* и его довольно легко заметить и удалить, но это требует немного больше усилий, чем можно было бы потратить.
До тех пор пока вы не выработаете привычку замечать это во время всей вашей обычной рутинной работы вы можете добавить ещё один шаг к действиям, которые вы выполняете перед коммитом: проверьте нет ли рядом с вашими изменениями мертвого кода. Это может произойти как раз перед отправкой кода вашим коллегам (вы ведь проводите code review, правда?) так что им не придётся повторять этот процесс, во время просмотра ваших изменений.
Другой вид мертвого кода появляется при удалении последнего, использующего его класса или функции, когда ты вносишь изменения, не понимая, что это последнее место, которое его использует. Это «dead code in the large»*, и его труднее обнаружить в ходе обычного программирования, если только вам не повезло обладать эйдетической памятью, или знать кодовую базу как свои пять пальцев.
Вот когда нам могут помочь инструменты для анализа программы во время выполнения. В Magnetic мы используем пакет Неда (да, того же Неда) coverage.py, чтобы помочь нам принимать решения о мертвом коде. Обычно coverage используется во время тестирования, чтобы гарантировать, что ваши тест-кейсы правильно выполняют тестируемый код, но мы также используем его в нашем коде «работающем как обычно», чтобы понять, что используется, а что нет:
import coverage
cov = coverage.Coverage(
data_file="/path/to/my_program.coverage",
auto_data=True,
cover_pylib=False,
branch=True,
source=["/path/to/my/program"],
)
cov.start()
# ... делаем что-нибудь ...
cov.stop()
cov.save()
Здесь создается объект Coverage с несколькими опциями, чтобы сделать отчет более удобным. Сначала мы говорим ему, где сохранять свои данные (мы будем использовать их позже, для создания удобного HTML-отчета, о том что используется, а что — нет), и просим его автоматически открывать и добавлять их в этот файл с помощью auto_data=True. Далее мы просим не беспокоится об обработке стандартной библиотеки и установленных пакетов — это не наш код, так что мы можем предположить, что многое из того что он содержит, может нами не использоваться. Это не мёртвый код, который мы должны поддерживать, так что можно смело его игнорировать. Мы просим его вычислить покрытие ветвлений (выполняются ли оба состояния true и false для каждого оператора if). И, наконец, мы указываем где находятся наши исходники, так что он может соединить свои знания о том, что используется и не используется в исходном коде, для составления отчета.
После запуска нашей программы, мы можем создать HTML-отчет:
$ COVERAGE_FILE=/path/to/my_program.coverage coverage html -d /path/to/output
Который выглядит примерно так:
(полный пример HTML-отчета можно найти в документации coverage.py)
Строки выделенные красным цветом не вызывались во время выполнения программы. Эти строки (и, возможно, методы) кандидаты для удаления в качестве мертвого кода.
Я оставлю вам три предупреждения об использовании этого подхода для нахождения и удаления мертвого кода:
- Будьте осторожны при рассмотрении итогов работы coverage — тот факт, что строка или функция не были выполнены в течение одного запуска программы не означает, что они обязательно мертвы или недоступны вообще. Вы всё равно должны проверять код, чтобы определить, действительно ли они совершенно мертвы в вашем приложении.
- Вычисление покрытия означает, что ваша программа должна выполнить больше работы, так она будет медленнее, при запуске в этом режиме. Я бы не рекомендовал запускать это в продакшене, но в промежуточной среде (staging environment) или в целевых сценариях все должно быть в порядке. Как всегда, если производительность является важной задачей, вы должны точно измерить, какое влияние оказывает вычисление покрытия, прежде чем запускать его.
- Наконец, не доверяйте отчетам о покрытии кода во время тестовых запусков. Какой-то код может быть мертв, но тесты все равно будут запускать его; и какой-то код может быть жив, но не запускаться тестами!
Слова на прощание
Перед тобой, дорогой читатель, я должен извиниться. Я опустил важную часть поста Неда, когда цитировал его ранее.
Он говорит:
В данном случае нет простого ответа на этот вопрос, потому что это зависит от класса и от метода. [...] Грубый ответ может быть таков: если этот класс является частью фреймворка, то оставьте его, если же он часть приложения, то удалите его.
Если вы пишете библиотеку или фреймворк, а не приложение, то вопрос мертвого кода становится с одной стороны сложнее, а с другой проще. В сущности, вы никогда не сможете удалить часть публичного API (за исключением несовместимости в мажорных версиях). Фактически весь ваш публичный API это живой код, даже если вы сами не используете его. Но за внешним интерфейсом, все ещё может возникать мертвый код, и он должен быть удален.
Удаляйте свой мертвый код!
* — отсылки к парадигмам программирования «programming in the small» и «programming in the large» (прим. перев.)
Комментарии (61)
aquamakc
20.05.2016 10:11+1В компилируемых языках вполне можно #define`ами отрезать ненужные для конкретного случая куски кода.
Выжигать надо только то, что уже ТОЧНО не нужно.
DuDDiTs
20.05.2016 10:11+1Про #define в статье упоминалось
возможно, тот, кто вносит изменения не считает, что код должен исчезнуть навсегда и комментирует его или добавляет условную компиляцию.
Только если какой то код вообще не нужен, то нет смысла его оставлять. Вся проблема в том чтобы найти такой код.aquamakc
20.05.2016 10:17Код бывает разный. Например, код программируемого микроконтроллера. На базе одного микроконтроллера строится несколько устройств отличающихся рядом функций. Большая часть кода универсальна. В таком случае для компиляции прошивки под конкретный девайс define`ами отрезается лишний функционал.
DuDDiTs
20.05.2016 10:33+4Если вы знаете, что этот код точно для чего то нужен и этот #define может когда-то выполняться, то выходит, что код не мертвый.
Ну и эти рассуждения о мертвом коде наверное в меньшей степени полезны для компилируемых языков. Да и автор оригинала рассуждает в контексте python'a
wsf
20.05.2016 13:29+2Это абсолютно нормальная практика для языка типа С, это не мертвый код, он ведь работает у вас, просто в специфичных случаях.
M-A-XG
20.05.2016 10:11>Мертвый код, то есть код, который никогда не выполняется в вашей программе — это реальная помеха для поддержки вашей кодовой базы.
Утверждение неверно.
Некоторый код может включатся / выключатся конфигами, когда нужно.
И это должно быть явно видно в самом коде.DuDDiTs
20.05.2016 10:14+6Если код может включаться, то он уже и не мертвый. Тут главное не «выжечь» его по ошибке
nikitasius
20.05.2016 10:51Страшнее это мертвый код в проекте, который в одном единственном файле на 300кб. Вот это реальный экстрим.
aquamakc
20.05.2016 11:20В таком случае должны возникнуть претензии к разработчику(архитектору). Я, кстати, видел такой файл. Причём это был один класс. Было мучительно больно его приводить к виду «более-менее можно понять что оно делает».
gearbox
20.05.2016 13:27+1да там уже не мертвого кода бояться надо.
burjui
20.05.2016 14:07Вот уж точно. На одной прошлой работе я помогал портировать на iOS игру на C++, в ней вся логика и отрисовка были заключены в одном методе на ~3500 строк. Самой большой проблемой было понять, что, чёрт возьми, в нём вообще происходит. О рефакторинге и удалении мёртвого кода я даже не заикался, т.к. там всё было построено на анти-функциональной парадигме: туча глобальных переменных, всё усыпано состояниями, как булка — маком, классы как имена для пачек процедур. В таком проекте нет понятия "мётрвый код", т.к. всё сыпется от малейшего чиха.
aquamakc
20.05.2016 14:13+2Как показывает практика в таких случаях лучшее, что можно сделать — написать всё с нуля.
MooNDeaR
21.05.2016 16:00Я вот сейчас на новой работе столкнулся с функцией на 4300+ строк. В ней прописана какая-то аццки непонятная вещь по инициализации системы. Интересно, что в том же файле определяются еще 20+ вспомогательных функций. И самое крутое — ни одного комментария. Вообще. Оно просто работает и я понятия не имею как :)
ivanbolhovitinov
20.05.2016 11:25+2Явно мертвый код надо хоронить, иногда с почестями, иногда не прощаясь.
А сомнительный мертвый код я стараюсь убирать в два прохода.
Сначала код комментится, потом удаляется. Причем между первой и второй итерациями код должен пройти хотя бы тестовую среду.
Поэтому если я встречаю код, закомментированный по причине, которую я сходу не помню — в топку без разговоров.
samizdam
20.05.2016 23:13Используем похожий подход для сомнительного кода, который вроде и мёртвый, но просто грохнуть страшно.
В первом проходе прячем в коммент с меткой TODO и планируемой датой для удаления — например через неделю после следующего мажорного апдейта.
После дня икс, увидев такую тудушку можно удалять с чистой совестью: программа живёт и чувствует себя хорошо без него на всех окружениях, спустя целую итерацию.
Явно неиспользуемый и откровенно старый выжигаем без сожаления.
kamilgarey
20.05.2016 11:33Боле точно это рассмотрено у Фаулера http://martinfowler.com/bliki/SacrificialArchitecture.html
Не рассмотрен случай когда, «мёртвый код» написан тобой. (Здесь я имею ввиду код зависящий от настроек которые никогда не будут выставлены) Сам наступал на эти грабли (тащил старый код).
Shamov
20.05.2016 12:36А я сознательно не удаляю мёртвый код, если он реализует какую-нибудь необычную вещь, которую нельзя будет в будущем воспроизвести прямо из головы с первой попытки. Вот, например, я сначала сделал что-то одним способом. Затем придумал способ получше. Старую реализацию я оставляю прямо там, где она была, если она представляет какой-то интерес. Может в будущем пригодится в другом проекте. Вместо того, чтобы поддерживать библиотеку сниппетов с детальными описаниями, я храню интересный код прямо там, где он изначально появился. Так мне гораздо проще его находить. Когда нужно повторно реализовать что-то такое, что ты уже раньше делал, намного проще вспомнить то, в каком проекте и с какой целью ты это раньше делал, чем искать какой-то абстрактный сниппет среди сотен других. Память устроена ассоциативно.
DuDDiTs
20.05.2016 12:37Мне кажется так можно быстро захламить проект, лучше уж использовать систему контроля версий для альтернативных реализаций каких-то фич
Shamov
20.05.2016 12:58Пока мне не удавалось ничего захламить. А система контроля версий для этого подходит ещё хуже, чем специализированная база сниппетов с описаниями.
aquamakc
20.05.2016 12:45Как правильно говорят — система контроля версий помогает. В крайнем случае можно создать отдельный проект или просто текстовый файл для хранения подобных конструкций, само-собой с подробным описанием, чтоб даже через год открыть и вспомнить что этот код делает.
Shamov
20.05.2016 13:00-1Мне не помогает. Проблема отдельного проекта или текстового файла в том, что там отсутствует контекст, из которого понятно, как это лучше использовать.
DuDDiTs
20.05.2016 13:10Ну так в контроле версий и контекст сохраняется и описание к коммитам можно писать
Но дело хозяйское, главное чтобы удобно былоShamov
20.05.2016 13:33-1Зато в контроле версий нет удобного поиска. Нужно более-менее точно помнить время, когда ещё была старая реализация. В принципе, мой способ сочетает в себе всё необходимое и лишён недостатков. Во-первых, старая реализация сохраняется вместе с контекстом её использования. Во-вторых, её всегда можно быстро найти, поскольку она есть в самой последней версии кода проекта. В любых других способах нет либо того, либо другого. Хотя, я признаю, что другие способы более правильные с методологической точки зрения. И если отсутствие методологической чистоты считать недостатком, то недостатки у моего способа всё-таки есть. Просто я быдло… и методологическая чистота не находится в числе моих приоритетов :)
aquamakc
20.05.2016 13:37проблема в таком случае возникает, если этот код открывает другой разработчик, да даже сам через год работы в других проектах забываешь где рабочий код, а где «на память»
Shamov
20.05.2016 13:44-1Нет, другие разработчики этого мёртвого кода, естественно, не видят. В основную ветку я сливаю только то, что реально используется. Такой мёртвый код живёт только в моих собственных репозиториях. (Похоже я забыл сказать самое главное :)
0xd34df00d
21.05.2016 01:44git log <filename>
git grep
git blameShamov
21.05.2016 08:12-1Вижу здесь необходимость указывать конкретный файл и регулярные выражения. Каждая из этих вещей по отдельности мешает счесть такой поиск удобным.
0xd34df00d
21.05.2016 18:16А как вы тогда в вашей системе ищете конкретный код? Ну, конкретный файл и конкретные строчки в нём?
Shamov
21.05.2016 18:36-1Ассоциативно. По памяти. Я всегда помню, в каком проекте я уже что-то подобное делал. И приблизительно знаю, в каком файле это может быть.
0xd34df00d
22.05.2016 21:36+1Так если вы знаете, в каком проекте/файле вы это уже делали, то вы точно так же можете натравить эти замечательные команды на эти файлы.
Shamov
22.05.2016 22:25Часто я не знаю, что конкретно нужно искать. Нужно открыть какие-то файлы проекта, посмотреть на них глазами, и тогда становится понятно, куда ещё можно заглянуть или что конкретно следует искать.
Давайте предположим… просто ради смеха, что grep — не самый лучший инструмент для поиска кода. Вам когда-нибудь удавалось найти с его помощью хоть что-то в коде, если это был не print() с сообщением об ошибке? Не предложить кому-нибудь сделать это, а реально найти что-нибудь самому…0xd34df00d
23.05.2016 05:48Да, удавалось. Как правило, я помнил какое-то ключевое слово в окрестности того, что мне нужно было найти. Имя класса, имя метода, имя параметра, что-то такое.
Shamov
23.05.2016 06:55Вам везёт. А я уже стар и не могу рассчитывать на то, что память надёжно сохранит мелкие подробности.
gearbox
23.05.2016 14:50Вам когда-нибудь удавалось найти с его помощью хоть что-то в коде, если это был не print() с сообщением об ошибке?
Пользуюсь grep, знакомство с чужим проектом практически всегда начинается с него. Да, сижу в vim, а не в IDE. Нет, потребности не ощущаю. Да, работал в IDE, вкус устриц знаю.
Shamov
23.05.2016 14:55Отлаживаетесь, наверное, в чистом gdb…
0xd34df00d
23.05.2016 15:23Более того, gdb использую лишь для получения трейсов.
Shamov
23.05.2016 15:37-2Я разочарован. Думал, вы вообще не отлаживаетесь, поскольку пишете код сразу без ошибок.
wsf
20.05.2016 13:28Все логично, код должен работать, периодически устраиваю проходы по коду вычищая всякую дрянь, сразу становится опрятнее, чище и понятнее. В случае если что-то может пригодится (то есть почти никогда) всегда можно вытащить историю из системы контроля версий.
BalinTomsk
20.05.2016 17:05-1Чем хороши Delphi и C# тем что в режиме отладчика там видны методы, которые никем не вызываются и их можно удалить
grossws
20.05.2016 17:30-1И что, методы вызываемые через reflection по имени на базе пользовательского ввода тоже подсветит как используемые?
burjui
20.05.2016 17:49Не в курсе, как в Delphi и C#, а в Java такие методы разработчики обычно аннотируют:
@SuppressWarnings("unused") public void doSomething() ...
Это решение не идеально, т.к. такие методы никогда не будут "мертвы". Впрочем, при очередной чистке кода можно поискать в проекте методы с этой аннотацией и проверить, используются ли они на самом деле. Но лучше всего, конечно, избегать использования reflection, по возможности, дабы использовать преимущества статической типизации на полную катушку.
grossws
20.05.2016 18:15-2Вы, наверное, также аннотирует весь public API, если пишите библиотеку, да?
samizdam
20.05.2016 23:19+1За последний код, осознал, что мне больше нравиться коммитить удаления строк и файлов, чем добавлять их.
Просто физическое удовольствие, когда git status выводит жирный список deleted, от предвкушения предстоящего коммита =)
netpilgrim
28.05.2016 16:20+1Идея. В continuous integration прикрутить систему ачивок для стимуляции хороших практик:
«уборщик кода» — больше всех удалил кода
«нинзя» — не сломал ни одного теста
и т. д )DuDDiTs
28.05.2016 16:23+1Главное заранее хорошо все продумать.
«уборщик кода» может до добра не довести :)
Vehfl
Хуже «мертвого» кода может быть только простыня из закомментированного кода
DuDDiTs
Ну она хотя бы сразу в глаза бросается и не прикидывается чем-то реально работающим
burjui
Закоментированный код хуже тем, что компилятор не проверяет ни его синтаксическую, ни, тем более, семантическую корректность: даже если он и понадобится в будущем, никто не гарантирует, что он хотя бы скомпилируется, не говоря уж о том, чтобы работать. По возможности, любой мёртвый код, если он может когда-нибудь пригодиться, должен быть покрыт тестами, иначе это бомба замедленного действия.
igorch96
А кто сказал, что к моменту использования комментированного кода его тесты будут актуальны?
burjui
Разумеется, речь шла о мёртвом, но незакоментированном коде, иначе тесты не имеют смысла. Моё мнение насчёт закоментированного кода: его быть не должно. Системы контроля версий всегда помогут восстановить удалённый код, а закомментированный код хуже, чем удалённый, по указанным выше причинам — к тому же, он занимает драгоценное место в исходниках, мозоля глаз.
igorch96
Согласен.
Wilk
Здравствуйте.
Целиком и полностью поддерживаю озвученную Вами позицию относительно удаления закоментированного кода. Однако, я сталкивался с тем, что некоторые разработчики считают, что это код там очень нужен, и если он мне мешает, я могу удалить его в своей локальной копии, а в репозитории пусть лежит всё целиком. Я вижу оправдание этому только в случае, если такой закоментированный код, во-первых, не содержит сложной логики, например, представляет собой какой-то отладочный вывод, который лениво набивать вновь, либо содержит наброски решения с соответствующим комментарием, т.е., фактически, является псевдокодом, и, во-вторых, будет в результате удалён сразу после того, как какая-либо необходимость в нём отпадёт.
Crandel
Не согласен, всегда можно настроить фолдинг закомиченного кода, но ты уверен, что оно не влияет на работу твоего приложения
M-A-XG
О да.
У меня на работе хоть и есть git, но старый код почему-то не удаляется.
Типа, а вдруг потом потребуется. :)
Mixim333
Полностью согласен.
>тот, кто вносит изменения не считает, что код должен исчезнуть навсегда и комментирует его или добавляет условную компиляцию
— вот это самое поганое — добавление условной компиляции, никогда ее не понимал и не использовал, т.к. это верный путь к граблям на пути к релизу.
>помните, что системы контроля версий прикроют вас в случае, если вам когда-либо снова понадобится этот код
— с этим полностью согласен, в совокупности с предыдущей цитатой можно сделать вывод: если Git\SVN хранит всю историю Вашей кодовой базы, то зачем что-то комментировать — удаляйте мертвый код под чистую, чтобы он не «вонял», даже есть термин: «smell code».
grossws
Есть термин "code smell", а smell code выглядит как указание к действию