Pull to refresh

Comments 30

А можно и без всяких автоматов обойтись. Можно помечать старые Функции и методы тегом @deprecated, плюс в устаревший метод вкладывать вызов логера, который логирует вызов старых методов и где-нибудь начинают всплывать в логах записи о выполении старых методов. А так же помечать целые классы @deprecated и проверять в autoload'e наличие этого тега.

После этого в правильных IDE начнут всплывать подсказки у разрабочиков, что эти методы или классы уже устарели и их надо бы заменить, но и в логи активно сыпаться начнёт всё это дело.
Да, такой подход тоже можно использовать, но тогда это будет означать ручную работу, а это обычно занимает на порядок больше времени и дает на порядок больше ошибок.
Зато так можно помечать конструкции любой сложности, а не только простые для авто-замены вещи
+ надежнее.

Для меня автоматизированный рефакторинг — зло.
Вы говорите о том, что ручной рефакторинг лучше, чем автоматизированный. Да, во многих случаях это так, потому что выполнение автоматических замен требует аккуратности. Тем не менее, если заменять только простые вещи (и поддающиеся замене), то она наоборот, будет более надежной, потому что человек может ошибиться при выполнении той же самой (рутинной) операции.
Все хорошо до тех пор, пока идентификаторы уникальны для всего объема текста проекта. Например, имена классов. А вот давайте поговорим про нетривиальный случай, когда надо переименовать поле в одних классах, а в других то же поле не трогать :)
Приведите пожалуйста пример, в котором требуется только переименовать поле класса. Допротобуфные сериализации не в счёт.
Я не знаю, что такое «допротобуфные сериализации», смотрите не вызовите такими словами кого-нибудь…

У нас тут на проекте внезапно решили привести все к новой naming convention. Код на C++, с теймплейтами и #define-ами. Классы переименовать труда не составило — почти все уникальны для проекта. Делаем немножко магии с awk, sed, mv и bash, профит ^_^

Проблемы настали для полей некоторых классов, потому как без мета-информации отличить одни от других нельзя. Общий объем рефакторинга исключает ручную правку. Только классов переименовано порядка 200, порядка 4000 файлов исходных кодов «задето» переименованием.

Как переименовывать поля — ума не приложу :)
Подобные проблемы обычно решаются интеграционным тестированием. Добавил фичу со старыми методами — тесты развалились — надо идти править.
Автоматическая замена в моем понимании скорее зло — вместе с переименованием методов изменяется и логика, и код, расчитанный на старую логику, разломается, даже если сделает новые вызовы.
В данном конкретном случае даже тесты не нужны — компиляция должна упасть.
в данном конкретном случае компиляция пхп не упадет. пхп и не такое скомпилирует
Это да. К сожалению.
Полностью согласен. Перед релизами сливать все в тестовую ветку. Что можно покрыть юнит тестами, что нельзя тестить вручную.
Ну практически все что вы описали может делать хорошая IDE'шка, причем намного удобнее «разруливать» конфликты, наблюдая непосредественный контекст вызова.
Вообще, таким образом можно применить лишь очень ограниченный набор простых рефакторингов (Rename Class / Method и т.п.). Что-то более сложное, например «выделение класса», «замена кода типа» все равно придется вручную разруливать ибо страшно это доверять скрипту, даже с тестами…
Даже сама хорошая IDE'шка очень часто бывает бессильна
Да, чудес не бывает(
Тут важна совокупность инструментов и процессов. Рефакторинг должен быть постоянным процессом и его нельзя откладывать «на потом».
А чтобы его не откладывать на потом, нужно его не бояться. А не бояться возможно только при максимальном покрытии кода тестами.
А это чаще всего чудеса, особенно на проектах, доставшихся в наследство.
Мне кажется, что список из 9 не автоматических действий при рефакторинге сложно назвать автоматическим
Большая часть этих действий должны выполняться с помощью тех самых скриптов, о которых шла речь выше. То есть, выполнить вручную нужно совсем не 9 шагов. Помимо этого, рефакторинг «автоматизированный», а не автоматический :). Предлагаемый мной метод не является заменой ручному рефакторингу, а является примером того, как скрипты могут помочь выполнить этот рефакторинг существенно быстрее и надежнее.
Можно старый метод не удалять, а проксировать его на новый, до разлива на продакшн. А после разлива (например на сл. день), делать второй коммит с удалением уже старого проксирующего метода. Это обычно спасает в большинстве ситуаций.
Старые ветки всё ещё могут долго жить (и в больших проектах живут :)), поэтому удалять функцию можно только через большой промежуток времени, к сожалению.
Согласен, поэтому и написал «например». Думаю в каждом конкретном случае разработчик сам определяет этот промежуток.
У нас, например, недавно я вводил новый функционал Class::Method, но на следующий день решил исправить на вызов Class::anotherMethod — сделал именно так, как описал выше. Хотя здесь функционал был новый и понятное дело, что никто еще не должен был успеть с ним поработать :)

PS: привет коллегам из Баду)
Я так понимаю, раздел «Рефакторинг использования SQL в phpBB» был специально добавлен, чтобы водянистая заметка выглядела большой и более… кхм, ПРОГРАММИСТИЧЕСКОЙ!
Эта часть изначально присутствовала в статье, просто была скрыта за спойлером. Эта часть важна для иллюстрации подхода и чтобы показать на живом примере, как это делается.
Вспомнились те времена (2003-2006, 2013?), когда phpBB был дрявый, эх…
Риторический вопрос. Что будет быстрее, твой собственный вариант sql_query_escaped с кучей preg_match_all и implode, или вызов сишной функции вида $sql=sprintf(«SELECT forum_id FROM TOPICS_TABLE WHERE topic_id = %s»,"'"+mysql_real_escape($topic_id)+"'"), где вот это вот последнее экранирование можно вывести в функцию с коротким и удобным именем?
>>если вставляемые значения — это только числа, то можно использовать «?d»;
Перефразирую — можно использовать %d.
Это к тому, что иногда можно использовать уже существующие функции, неоправданно забытые, вместо изобретения своего медленного велосипеда.
Мой preg_match вряд ли будет узким местом в SQL-запросах :), можете сами проверить.
Речь в первую очередь о велосипедах.
Sign up to leave a comment.