Комментарии 24
Обратитесь с этим вопрос в Q&A
У ТРУшного (целевого) программиста при взгляде на такой код должны встать дыбом волосы и по телу мурашки пройтись. Так их и вычисляют. Всем, кто пытается код рефакторить — отказ. :)
Отличное тестовое задание. Нормальный спец просто переписал бы всё нафиг с нуля, а недокодер попытался бы исправить. В любом случае по результату было бы хорошо видно как мыслит человек и сколько у него опыта.
Кстати, как минимум, человек должен будет сказать, что под 5.3 код не запустится, и поэтому пусть дают ему нормальное тестовое задание, которое можно запустить под всеми версяими PHP
Это если известно, что код должен делать. В противном случае нормальный спец отрефакторил бы код в процессе его понимания. После чего переписывание может и не понадобится (не знаком с php, потому не знаю какова ситуация в этом случае). Поэтому я бы советовал автору создать svn-репозиторий и коммитить туда после каждого рефакторинга. В случае переписывания функции с нуля, следует в одном коммите добавить комментарий с описанием того, что делает функция, а в следующем уже полностью заменить ее тело. Результат послать нанимателям. Это даст им на много больше информации, чем несколько лаконичных строчек кода, которые окажутся в итоговой программе.
Ну конкретно этот код так просто не отрефакторишь, так как он вместо/помимо параметров использует переменные из глобальной области видимости, причем три на вход, и одну на выход, соответственно если это переписывать через руки — придется изменить не только код функции, но и те места где она вызывается.
Я бы написал об этом в комментариях к коммиту. По моему опыту, изменение способа вызова функции в сотне мест вполне решаемая за час-два задача.
Ну если изменилось только количество параметров — это одно, с глобальными переменными ситуация ведь в корне иная, глобальные переменные могут быть инициализированы не только в вызывающей функции, а в любом другом месте до вызова choose_period(), и выходное значение может быть использовано где угодно.
Потому чтобы получить доступ к этим значениям — придется объявление глобальных переменных вынести на уровень выше, а-ля:
Получается шило на мыло, от глобальных переменных в итоге не избавились (для этого придется перековырять весь код где они используются), да и к тому-же есть шанс столкнуться с тем что названные таким образом переменные уже используются в somefunction(), например $TEXT — вполне себе безымянная переменная.
$PERIOD_FROM = '01.01.1970';
$PERIOD_TO = '19.01.2038';
// где-нибудь там
function somefunction() {
// эта функция и понятия не имеет о том что существуют глобальные переменные $PERIOD_FROM, $PERIOD_TO и $TEXT
choose_period();
}
// десять тысяч строк спустя
echo $TEXT;
Потому чтобы получить доступ к этим значениям — придется объявление глобальных переменных вынести на уровень выше, а-ля:
$PERIOD_FROM = '01.01.1970';
$PERIOD_TO = '19.01.2038';
function somefunction() {
global $PERIOD_FROM, $PERIOD_TO, $TEXT;
$TEXT = choose_period($PERIOD_FROM, $PERIOD_TO);
}
echo $TEXT;
Получается шило на мыло, от глобальных переменных в итоге не избавились (для этого придется перековырять весь код где они используются), да и к тому-же есть шанс столкнуться с тем что названные таким образом переменные уже используются в somefunction(), например $TEXT — вполне себе безымянная переменная.
В данном кокретном случае надо r/choose_period/choose_period_new/ а потом r/somefunction/choose_period/
И отдельно рефакторить функции и глобальные переменные.
А вообще зачастую проще и правильней вытащить из говнокода логику, ее пересмотреть и заново спроектировать и написать. Часто говнокодеры делают ужасные по последствиям логические и архитектурные ошибки. Обычный рефакторинг, оставляющий логику неизменной, тут не поможет.
И отдельно рефакторить функции и глобальные переменные.
А вообще зачастую проще и правильней вытащить из говнокода логику, ее пересмотреть и заново спроектировать и написать. Часто говнокодеры делают ужасные по последствиям логические и архитектурные ошибки. Обычный рефакторинг, оставляющий логику неизменной, тут не поможет.
Ну так а я не про это разве писал? :)
Проблема в том, что часто такая логика пронизывает весь проект. Может, пересмотреть и переписать её возможность есть, но переписать значительный кусок проекта, где встречается такая логика — нет ни времени, ни возможности — бизнес не даст.
Вот и тянется сей «балласт» многие годы. Иногда, правда, можно заменять старый код на новый по частям, но далеко не всегда можно обеспечить такую «обратную совместимость».
Вот и тянется сей «балласт» многие годы. Иногда, правда, можно заменять старый код на новый по частям, но далеко не всегда можно обеспечить такую «обратную совместимость».
В таком случае не надо делать рефакторинг just for fun. Для него нужно иметь четкую цель — упростить развитие или расширение, решить глобальную проблему типа производительности. И эта цель, обоснованная требованиями бизнеса и найдет и время и возможности. А нет — значить и не надо. Красота кода, сама по себе денег не приносит. Работает, кушать не просит — и нехай пусть живет. Переписывать можно и кусочками в качестве решения проблем, но обойдется это дороже, хоть и размазано по времени.
Это все конечно не про открытые проекты для себя :)
Это все конечно не про открытые проекты для себя :)
Ну вот каждому соискателю раздашь по одной функции — глядишь рефакторинг и готов :) Хуже-то точно не напишут :)
Бывает пишут и хуже (если не брать в расчёт вышеприведённый код).
Пишут как просто плохо, так и ООП ради ООП.
Идея раздать по одной функции, конечно, хороша, но т.к. у каждого соискателя свой стиль, свои правила оформления, да если ещё и кода плохого мегабайт 5-10… — потом будет ад в поддержке.
Пишут как просто плохо, так и ООП ради ООП.
Идея раздать по одной функции, конечно, хороша, но т.к. у каждого соискателя свой стиль, свои правила оформления, да если ещё и кода плохого мегабайт 5-10… — потом будет ад в поддержке.
С такими мыслями, уже ничем не поможешь. А рефакторить разными людьми без общей идеи и кооперации — лучше сразу застрелиться.
Это еще что… мне недавно вот такое прислали :-))
Я долго думал: они шутят или тут правда надо исправить ВСЕ ошибки: pastebin.ru/314666
Я долго думал: они шутят или тут правда надо исправить ВСЕ ошибки: pastebin.ru/314666
В каком бреду (и году) это задание писалось?..
Это не необычное, а самое обычное скучное тестовое задание.
я бы отказался такое рефакторить
«Хороший» фаршик. К сожалению, часто приходится видеть такое в старых и серьезных проектах, например, мегафон.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Тестовое задание