Комментарии 53
Знаете определение унаследованного кода? Это такой код, который не покрыт тестами. Любой код, который покрыт тестами, можно спокойно выкинуть и переписать заново.
Поэтому в вашем опросе нет самого важного пункта: «покрываем тестами, потом переписываем как хочется».
Поэтому в вашем опросе нет самого важного пункта: «покрываем тестами, потом переписываем как хочется».
Если код можно протестировать, это уже хорошо. Быть может, он не так уж и плох?
Серебряная пуля — это всегда хорошо! Бодрит и вселяет.
Покрыт тестами знаменитый
=================================================
float Q_rsqrt( float number )
{
int i;
float x2, y;
const float threehalfs = 1.5F;
x2 = number * 0.5F;
y = number;
i = * ( int * ) &y;
i = 0x5f3759df — ( i >> 1 );
y = * ( float * ) &i;
y = y * ( threehalfs — ( x2 * y * y ) );
return y;
}
=================================================
Дальше чего, как переписывать будете?
Покрыт тестами знаменитый
=================================================
float Q_rsqrt( float number )
{
int i;
float x2, y;
const float threehalfs = 1.5F;
x2 = number * 0.5F;
y = number;
i = * ( int * ) &y;
i = 0x5f3759df — ( i >> 1 );
y = * ( float * ) &i;
y = y * ( threehalfs — ( x2 * y * y ) );
return y;
}
=================================================
Дальше чего, как переписывать будете?
При чем тут серебряная пуля? Если это покрыто тестами, то переписывать будем так, как хочется.
Тесты — это всего лишь повышение уверенности, что мы ничего не сломаем.
Тесты — это всего лишь повышение уверенности, что мы ничего не сломаем.
Насколько я понимаю, тесты, в основном, проверяют пограничные случаи, где могут быть проблемы, исключения и тп, а так же несколько штатных срабатываний (подали 10.23, получили 13.45, к примеру)… В приведенном примере приводится сложная функция, которая выдает различные значения в зависимости от входного параметра, а тест, скорей всего, проверяет такие основные моменты, как выход за границы диапазона, соответствие такого-то тестового входного значения такому-то заранее известному выходному значению и тп, вполне перечислимый на пальцах набор проверок… Но, если вы не понимаете, что это за функция и по какому закону она работает, то вы не сможете ее переписать даже имея на руках эти тесты, вам придется полностью вникать в суть функции или переписывать тесты, причем так, чтобы они покрывали все возможные варианты поведения функции (как черный ящик) для всех возможных вариантов входных данных и только после этого методом тыка пытаться повторить это поведение так, чтобы этот бесконечный тест не упал…
Но, если вы не понимаете, что это за функция и по какому закону она работает, то вы не сможете ее переписать даже имея на руках эти тесты
Дело не в том, по какому закону функция работает, дело в том, что она должна делать.
В данном примере все сравнительно банально — берем граничные значения, равномерно распределенные внутри диапазона значения и несколько случайных (в смысле — случайных на момент теста) значений, а потом для них считаем отклонение результатов этой функции от эталона (в смысле, стандартной реализации sqrt). Если где-то отклонение выше допустимого — начинаем разбираться.
Но вообще, конечно, такие вещи покрываются еще и снаружи — т.е., дело в реальности не в том, что именно считает эта функция, а в том, как это используется и для чего. С хорошей вероятностью там есть тесты на реальных вариантах употребления, и до тех пор, пока они живы, мы можем быть сравнительно уверены, что система работает.
Это два отдельных момента. Первый — unit-тесты для нетривиальных математических функций. Когда-то этому была посвящена отличная статья тут, на Хабре.
Ну и второй — пытаться что-то рефакторить, не понимая до конца (не пожелав разобраться, на самом деле), это один из самых больших грехов разработчика (на мой вкус, конечно).
Ну и второй — пытаться что-то рефакторить, не понимая до конца (не пожелав разобраться, на самом деле), это один из самых больших грехов разработчика (на мой вкус, конечно).
пытаться что-то рефакторить, не понимая до конца (не пожелав разобраться, на самом деле), это один из самых больших грехов разработчика (на мой вкус, конечно).
Тут ситуация троякая.
С одной стороны, есть код, который, не отрефакторив, не поймешь. Тут мы под рефакторингом понимаем переименование и вынесение методов.
С другой стороны, есть код, который бесполезно пытаться понять, и надо переписать заново. Это, конечно, формально «рефакторинг», потому что внешнее поведение не меняется, но по факту — просто новый код по заданным требованиям.
И вот только с третьей стороны есть «я это отрефакторю, потому что мне хочется, пофиг, почему тут так было».
кстати, а это не та функция-хак быстрого извлечения квадратного корня, используемая в квейке? а то магическое число больно что-то напоминает…
Таких мест все равно не много. Их можно оставить как есть.
Самый сложный этап добиться того, чтоб главной проблемой стали вот такие вот маленькие функции.
Самый сложный этап добиться того, чтоб главной проблемой стали вот такие вот маленькие функции.
Бешено плюсую. Жаль, что многие этого не понимают.
НЛО прилетело и опубликовало эту надпись здесь
Цепная реакция — очень болезненная штука зачастую.
Ну и с дебрей комментариев на Хабре: i.imgur.com/wGUTG.gif
Ну и с дебрей комментариев на Хабре: i.imgur.com/wGUTG.gif
Чтобы что-то менять нужна причина. Причина «код написан плохо» — очень слабая, т.к. она не имеет явной связи с успешностью проекта (увеличение выручки, уменьшение затрат на поддержку).
Однако, если нужно расширять функциональность, тогда нужно смотреть какие участки кода могут вызвать проблемы, и если есть подозрение, и ресурсы позволяют, то переписывать плохой код.
Однако, если нужно расширять функциональность, тогда нужно смотреть какие участки кода могут вызвать проблемы, и если есть подозрение, и ресурсы позволяют, то переписывать плохой код.
Вы очень точно сформулировали мысль, спасибо.
Работал с людьми, для которых желание постоянно поддерживать код видеальном красивом состоянии, непрерывно рефакторить, переписывать с использованием все более новых плюшек и в итоге срывать сроки было почему-то важнее успешного завершения проекта. Наверное такое отношение к коду основано на недоумии.
Работал с людьми, для которых желание постоянно поддерживать код в
Полностью согласен. Менять надо по какой-то причине. Например, этот код трудно поддерживать, надо часто вносить изменения, он мало кому понятен, он работает неправильно. В этих случаях код надо исправлять. О полном переписывании в большинстве случаев речи не идет.
Причина «код написан плохо» — серьезная проблема, есть проект должен развиваться. Почитайте статьи о техническом долге, там хорошо объясняется, почему плохой код — это экономически не выгодно. С другой стороны, в случае если система должна «просто работать» и она выполняет свою функцию, трогать ее конечно же не нужно.
Ох, всего навсего «некий printf» делает непонятно что и непонятно как. Как-то я обнаружил код в неком самописном заголовочном файле, которым все пользуются (такие обычно живут в каталогах
Пальму первенства держит, я даже не знаю, с чем его сравнить. Народ (как потом выяснилось) что-то такое знал, типа «иногда Qt глючить начинает», но предпочитал не разбираться.
Вот такой бывает «старый код».
shared/, или common/, или MyCompanyLib/):#define size_t INT16;Пальму первенства держит, я даже не знаю, с чем его сравнить. Народ (как потом выяснилось) что-то такое знал, типа «иногда Qt глючить начинает», но предпочитал не разбираться.
Вот такой бывает «старый код».
Недавно услышал:
Если есть возможность покрыть тестами (или покрытие уже есть) и переписать код в рамках текущей задачи — я стараюсь это сделать.
Даже при небольших сроках существования проекта (год-полтора) появляются черные дыры, которые кому-то нужно латать. Меня очень вдохновил подход с заботой о «Будущих Разработчиках».
The Boy Scouts have a rule: «Always leave thecampgroundcodebase cleaner than you found it.»
Если есть возможность покрыть тестами (или покрытие уже есть) и переписать код в рамках текущей задачи — я стараюсь это сделать.
Даже при небольших сроках существования проекта (год-полтора) появляются черные дыры, которые кому-то нужно латать. Меня очень вдохновил подход с заботой о «Будущих Разработчиках».
В целом позиция «работает — не трогай» очень мудра. Если можно реюзить то это надо делать и направлять свои силы на что-то новое или эксперименты. Просто так править код по идеологическим причинам глупо.
Проблемы надо исправлять, но на рабочем используемом коде это надо делать аккуратно — смотреть use cases для него и составлять тесты по ним и только после этого делать рефакторинг.
В целом фанатизм — это плохо. Как только есть подозрения о каких-то идеях, не поддерживаемые рациональными причинами, то сразу надо искать фанатиков и тщательно от них избавляться — привычка думать шаблонами приводить к шаблонному результату. По русски это «хотели как лучше, получилось как всегда».
Проблемы надо исправлять, но на рабочем используемом коде это надо делать аккуратно — смотреть use cases для него и составлять тесты по ним и только после этого делать рефакторинг.
В целом фанатизм — это плохо. Как только есть подозрения о каких-то идеях, не поддерживаемые рациональными причинами, то сразу надо искать фанатиков и тщательно от них избавляться — привычка думать шаблонами приводить к шаблонному результату. По русски это «хотели как лучше, получилось как всегда».
Переписывать код большой системы сложно и дорого — это и объективные причины, и нежелание руководства платить за неактуальные в данный момент работы, и опасения что переписанный код перестанет работать, да бог знает что ещё. Можно попытаться разобраться откуда вообще берётся желание переписывать код и стоит ли это делать. Есть же и другие проблемы, не имеет ли смысл их тоже решить а попутно и с кодом разобраться.
При сроках работы проекта не год-два, а десять лет и больше всплывают такие факторы, как:
— невозможность достать дистрибутив системы, в которой выполняется программа — либо невозможно установить этот дистрибутив на современное оборудование, либо отсутствие патчей безопастности;
— язык программирования безнадёжно устарел, и на нём очень сложно вносить изменения;
— используемые шаблонизаторы/системы/языки программирования внезапно отказываются работать с новыми технологиями (например utf8);
— отсутствие людей, знакомых с устаревшими технологиями.
Иллюстрации по этим пункам:
— Одна из частей системы работает на BlackHat linux, работает долго и надёжно, но этого линукса уже нет. Хорошо что это только почтовый сервер, и его легко перенести на другую систему.
— Perl 5.6 был хорошим языком в 2000-м году, сейчас на CPAN почти нет сложных модулей, которые на нём заработают, и взять их неоткуда, так как правила ротации модулей на CPAN прямо требует удаления старых версий.
— Раньше HTML::Template был очень даже прогрессивным шаблонизатором, потом им стал Template-toolkit, теперь это Mojo::Template. Заставить себя править старые шаблоны, написанные на HTML::Template? очень трудно — он кажется кучей туупого дерьма.
— Perl 5.6 имеет проблемы в DBI и utf-8, и исправить их невозможно. Соответственно переход с cp1251 на utf в рамках этой версии требует кучи подпорок.
— я знал одного старого сотрудника вычислительного центра, который единственный помнил как запускать одну систему, установленную в единственном экземпляре, которая перезапускалась крайне редко. Он умрёт — и систему придётся как-то переписывать…
Всё это я к тому, что надо не код править бесконечно, надо организовывать программные системы в виде слабо связанных блоков с простыми интерфейсами, с тем чтобы при изменении условий оставлять интерфейсы неизменными. Перенос почтового сервера на новую операционную систему как раз хорошо это иллюстрирует.
Наличие тестов программной системы скорее всего сильно не поможет, если это не тесты крупных кусков со стабильными интерфейсами.
При сроках работы проекта не год-два, а десять лет и больше всплывают такие факторы, как:
— невозможность достать дистрибутив системы, в которой выполняется программа — либо невозможно установить этот дистрибутив на современное оборудование, либо отсутствие патчей безопастности;
— язык программирования безнадёжно устарел, и на нём очень сложно вносить изменения;
— используемые шаблонизаторы/системы/языки программирования внезапно отказываются работать с новыми технологиями (например utf8);
— отсутствие людей, знакомых с устаревшими технологиями.
Иллюстрации по этим пункам:
— Одна из частей системы работает на BlackHat linux, работает долго и надёжно, но этого линукса уже нет. Хорошо что это только почтовый сервер, и его легко перенести на другую систему.
— Perl 5.6 был хорошим языком в 2000-м году, сейчас на CPAN почти нет сложных модулей, которые на нём заработают, и взять их неоткуда, так как правила ротации модулей на CPAN прямо требует удаления старых версий.
— Раньше HTML::Template был очень даже прогрессивным шаблонизатором, потом им стал Template-toolkit, теперь это Mojo::Template. Заставить себя править старые шаблоны, написанные на HTML::Template? очень трудно — он кажется кучей туупого дерьма.
— Perl 5.6 имеет проблемы в DBI и utf-8, и исправить их невозможно. Соответственно переход с cp1251 на utf в рамках этой версии требует кучи подпорок.
— я знал одного старого сотрудника вычислительного центра, который единственный помнил как запускать одну систему, установленную в единственном экземпляре, которая перезапускалась крайне редко. Он умрёт — и систему придётся как-то переписывать…
Всё это я к тому, что надо не код править бесконечно, надо организовывать программные системы в виде слабо связанных блоков с простыми интерфейсами, с тем чтобы при изменении условий оставлять интерфейсы неизменными. Перенос почтового сервера на новую операционную систему как раз хорошо это иллюстрирует.
Наличие тестов программной системы скорее всего сильно не поможет, если это не тесты крупных кусков со стабильными интерфейсами.
Да, как раз о похожей ситуации и идет речь. Только не ставится вопрос о переписывании всего проекта, это недостижимая цель, еще Брукс об этом писал.
Оффтоп: а почему не попросить единственного сотрудника задокументировать процедуру перезапуска системы? :)
там чуйка трэба
Типа того, но ближе описанное в habrahabr.ru/post/164207/#comment_5649439
Мой случай — в середине перестройки все разбежались, денег ни на что нет, а система должна работать… старый дедушка последний кто её может перегрузить. Ему это уже лет 20 как непрофильно, но других вариантов нет. Кстати этот дедушка на восьмом десятке освоил линукс на хорошем уровне…
Мой случай — в середине перестройки все разбежались, денег ни на что нет, а система должна работать… старый дедушка последний кто её может перегрузить. Ему это уже лет 20 как непрофильно, но других вариантов нет. Кстати этот дедушка на восьмом десятке освоил линукс на хорошем уровне…
Если код работает корректно — не трогаем.
Если код выдаёт, как Вы сказали, «не совсем корректный HTML» — вносим правки (без переписывания всего кода).
Если код стал доставлять много хлопот (частые правки, регрессии) — кроем тестами и переписываем.
Если код выдаёт, как Вы сказали, «не совсем корректный HTML» — вносим правки (без переписывания всего кода).
Если код стал доставлять много хлопот (частые правки, регрессии) — кроем тестами и переписываем.
Один из ключевых моментов — наличие времени на улучшение текущего кода. Если время на разработку критично, что бывает лучше оставить работающий, но устаревший код.
Оказывается он формирует не совсем корректный HTML
По-моему это означает «не работает». Цель — вывод корректного HTML, значит скрипт свою цель не выполняет.
Вы живете в идеальном мире :) ошибка в 1 из 100 случаев — вполне может быть пропущена и код будет предполагаться правильным
А вот исправлять это или нет — это не вопрос выбора (разумеется исправлять), это вопрос приоритетов задач. Может быть кроме этого все остальное вообще не работает, или программа падает каждые пять минут?
А вот исправлять это или нет — это не вопрос выбора (разумеется исправлять), это вопрос приоритетов задач. Может быть кроме этого все остальное вообще не работает, или программа падает каждые пять минут?
почитайте «Refactoring» – M. Fowler и «Working Effectively with Legacy Code» – M. Feathers.
90% комментариев выше лучше не читайте.
90% комментариев выше лучше не читайте.
— Папа, а почему солнце утром всходит, а вечером заходит?
— Ты уверен?
— Да
— Точно? Каждый день? Ты проверял?
— Да
— Я тебя умоляю, ничего не трогай!
— Ты уверен?
— Да
— Точно? Каждый день? Ты проверял?
— Да
— Я тебя умоляю, ничего не трогай!
Ваши варианты ответов немного из разных плоскостей, как и почему.
Сначала надо решить переписывать ли код?
Не все решения должны быть идеальны, разработчик может негодовать от ужасов кода и его неэффективности, однако пользователь при этом доволен, как слон и лучше ему не надо.
Успешное ПО решает проблемы пользователей, пытаться угадать, а что бы еще пригодилось и решить проблему о которой пользователь и не заявляет — бесполезная трата времени, надо спрашивать, наблюдать, давать то, что нужно массовому пользователю.
Есть отличная книга Эффективная работа с унаследованным кодом — это работает в случае, когда вы уже решили точно переписать код.
Сначала надо решить переписывать ли код?
Не все решения должны быть идеальны, разработчик может негодовать от ужасов кода и его неэффективности, однако пользователь при этом доволен, как слон и лучше ему не надо.
Успешное ПО решает проблемы пользователей, пытаться угадать, а что бы еще пригодилось и решить проблему о которой пользователь и не заявляет — бесполезная трата времени, надо спрашивать, наблюдать, давать то, что нужно массовому пользователю.
Есть отличная книга Эффективная работа с унаследованным кодом — это работает в случае, когда вы уже решили точно переписать код.
Проголосовал за вариант «Вносим изменения при появлении необходимости». Сейчас поясню.
Если код работает, причем работает так как надо, в нем нет ошибок, или они есть, но, как вы пишете, годами не трогаются (а ошибки ли это тогда, если они никого не ипут?), то такой код не надо трогать. Работает — не трогай.
Но вот если в коде появляется ошибка, и в этот код страшно смотреть, то тут путь один: обложить его тестами (если еще не), и переписать.
Если код работает, причем работает так как надо, в нем нет ошибок, или они есть, но, как вы пишете, годами не трогаются (а ошибки ли это тогда, если они никого не ипут?), то такой код не надо трогать. Работает — не трогай.
Но вот если в коде появляется ошибка, и в этот код страшно смотреть, то тут путь один: обложить его тестами (если еще не), и переписать.
У нас есть один очень старый продукт, которым ежедневно пользуемся как мы, так и множество наших заказчиков.
Большинство разработчиков этой системы уже не работают с нами. Код написан очень запутанно и совершенно не очевидно. Кроме того начальные идеи были обезображены требованиями различных заказчиков по допиливанию. Никаких тестов, никакой документации. В итоге мы имеем систему, которая работает, но КАК она работает почти никто точно не знает. Система и язык, на котором она писалась устарели.
Дважды предпринимальсь попытки полностью переписать систему с нуля с преферансом и дамами. На это было потрачено больше года. Новая система получилась медленной и корявой. Было принято мужественное решение её полностью выбросить и отказаться от дальнейшей поддержки.
Мы пошли другим путем, периодически мы берем небольшие куски и полностью переписываем их. При этом неизбежно сталкиваясь с кучами граблей как в процессе переписывания, так и после запуска переписанной части в релиз. Несколько раз приходилось откатывать полностью готовый функционал из-за того, что кто-то вспоминал о совершенно незаметной но категорически нужной фиче.
Для нас не сработал революционный путь и мы пошли по пути эволюции. Сейчас все счастливы, система становится гибче, быстрее и красивее, как в плане кода, так и в плане интерфейса.
Большинство разработчиков этой системы уже не работают с нами. Код написан очень запутанно и совершенно не очевидно. Кроме того начальные идеи были обезображены требованиями различных заказчиков по допиливанию. Никаких тестов, никакой документации. В итоге мы имеем систему, которая работает, но КАК она работает почти никто точно не знает. Система и язык, на котором она писалась устарели.
Дважды предпринимальсь попытки полностью переписать систему с нуля с преферансом и дамами. На это было потрачено больше года. Новая система получилась медленной и корявой. Было принято мужественное решение её полностью выбросить и отказаться от дальнейшей поддержки.
Мы пошли другим путем, периодически мы берем небольшие куски и полностью переписываем их. При этом неизбежно сталкиваясь с кучами граблей как в процессе переписывания, так и после запуска переписанной части в релиз. Несколько раз приходилось откатывать полностью готовый функционал из-за того, что кто-то вспоминал о совершенно незаметной но категорически нужной фиче.
Для нас не сработал революционный путь и мы пошли по пути эволюции. Сейчас все счастливы, система становится гибче, быстрее и красивее, как в плане кода, так и в плане интерфейса.
… кто-то вспоминал о совершенно незаметной но категорически нужной фиче.
нередко сталкивался с такой ситуацией: код написан как попало, но что-то делает. Проблема, что никто точно не знает, что точно он делает, по какому алгоритму и на основании каких требований. Если иметь в комментариях или ТЗ это описание — можно просто выкинуть его и переписать заново, сохранив интерфейс и ключевое поведение. Но как правило комментариев нет, ТЗ устарело, а распутывать эту магию нет ни времени ни особого желания, все равно что-то да упустишь.
То есть весьма желательно иметь комментарий, что же именно хотел добиться разработчик.
Во-во, незаметные но категорически нужные фичи — основное препятствие благим целям покрыть всё тестами…
У нас в компании, на проект время от времени выделяется отдельное время на рефакторинг. Кроме того, постоянно пишутся и обновляются тесты, покрытие которыми, на вскидку, 60-70%. + местный рефакторинг по мере выполнения связанных задач. В итоге код поддерживается в нормальном, актуальном состоянии.
Тут все зависит от того что мы хотим добиться. Нужно понимать, что любые изменения это инвестиции, и стоят ли они того или нет, зависит от их доходности.
Если система работает, и все хорошо (наприме нам когда-то давно написали и настроили интернет магазин) но иногда надо делать мелкие исправления и дополнения, то лучше всего не трогать говнокод. Зач
Если систему нужно развивать, и весь говнокод находится в кусках, которые стабильны, работают и не меняются, то вносим изменения по мере необходимости.
Если система активно развивается, и говнокод находится в этих самых активно изменяемых частях, то беспощадно его преписываем.
Если система работает, и все хорошо (наприме нам когда-то давно написали и настроили интернет магазин) но иногда надо делать мелкие исправления и дополнения, то лучше всего не трогать говнокод. Зач
Если систему нужно развивать, и весь говнокод находится в кусках, которые стабильны, работают и не меняются, то вносим изменения по мере необходимости.
Если система активно развивается, и говнокод находится в этих самых активно изменяемых частях, то беспощадно его преписываем.
Сразу вспоминается известная поговорка: «Не трогай то что работает, того кто работает, и то чем работает, тот кто работает.»
НЛО прилетело и опубликовало эту надпись здесь
Переписываем код (предварительно покрывая тестами), когда предстоит его изменять — это основная причина для рефакторинга. Какие-то куски, которые непонятно что или как делают, просто изолируются (современные IDE позволяют это делать с минимальной вероятностью что-то испортить) до лучших времён.
Но бывают ситуации, когда менять нужно (серьёзный баг, например, или «оченьнужнаяфича»), но разбираться толком некогда, не говоря о том, чтобы покрывать проблемный участок тестами, фиксирующими текущее поведение. Тогда для нужного набора параметров формируем ещё одну ветвь исполнения, где чуть ли не захардкоженной таблицей возвращаем нужные данные, а общие случаи старый код обрабатывает по старому.
С html-шаблонами отдельная песня. Стоит глобальная задача заменить шаблонизатор и перейти на современную вёрстку. Делать это в два этапа нерационально — большой кусок работы (перевод старой вёрстки на новый шаблонизатор или перевод новой вёрстки на старый шаблонизатор) будет гарантированно выкинут. А в один этап невозможно зафиксировать существующее поведение. Приходится постепенно вытеснять старую верстку и шаблонизатор, надеясь лишь на багрепорты.
Но бывают ситуации, когда менять нужно (серьёзный баг, например, или «оченьнужнаяфича»), но разбираться толком некогда, не говоря о том, чтобы покрывать проблемный участок тестами, фиксирующими текущее поведение. Тогда для нужного набора параметров формируем ещё одну ветвь исполнения, где чуть ли не захардкоженной таблицей возвращаем нужные данные, а общие случаи старый код обрабатывает по старому.
С html-шаблонами отдельная песня. Стоит глобальная задача заменить шаблонизатор и перейти на современную вёрстку. Делать это в два этапа нерационально — большой кусок работы (перевод старой вёрстки на новый шаблонизатор или перевод новой вёрстки на старый шаблонизатор) будет гарантированно выкинут. А в один этап невозможно зафиксировать существующее поведение. Приходится постепенно вытеснять старую верстку и шаблонизатор, надеясь лишь на багрепорты.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
«Работает — не трогай». Стоит ли нарушать правила?