...
int x = (i << 8) | (i >> 2) | ((i & 0x06) ^ 0xAA);
// if (x >= 0x12345)
// x = x >> 3;
calc_something_complicated(x);
...
мне хочется рвать и метать. Расчехлить blame, найти автора и заглянуть ему в глаза.Иначе, что мне остается: только думать, что автор этих строк, видимо, бился с формулой, пытался подогнать результат под что-то (возможно, какой-то тест). Достиг ли он результата? Или может оно так и продолжает глючить... Кто знает. Единственное, о чем этот код однозначно говорит, что автор не был уверен в том, что пишет. Потому, что если он был уверен, то удалил бы этот фрагмент или раскомментировал бы его навсегда.
На втором месте у меня стоит отладочная печать, навеки оставленная в коде:
...
int x = (i << 8) | (i >> 2) | ((i & 0x06) ^ 0xAA);
// std::cerr << "На этот раз x = " << x << std::endl;
calc_something_complicated(x);
...
Снова получается, что автор сомневался и так и не отладил все до конца.Конечно, подобные куски могут появляться просто по невнимательности и забывчивости, но как может помочь это оправдание? Никак.
Ну на третьем месте нашего хит-парада - блоки TODO.
...
int x = (i << 8) | (i >> 2) | ((i & 0x06) ^ 0xAA);
// TODO: Заменить тип int на int64, так как на носу эра 64-разрядных машин.
// Программист Вася, 06.10.2009. Если что, вы знаете где меня искать.
calc_something_complicated(x);
...
Тут еще куда ни шло. Куски TODO можно найти автоматическим поиском при подготовке релиза и перед ответственным слиянием. Но каждый TODO должен быть подписан и датирован, а лучше еще и детально объяснен. Ни что так не помогает оценить "нужность" куска кода, как дата его последней модификации.Итак, вывод тут только один: в production коде никогда не должно быть закомментированных кусков. А если они есть, то они должны сопровождаться четкими комментариями, поясняющими их суть.
браво. хоть и не оригинально, но все равно.
ОтветитьУдалитьсам не люблю оставленные TODO. Почему их не сделать сразу, нафига оставлять на потом?
За совет ставить дату и автора в TODO -- big thanks. Оценил.
Если же этот TODO - плановая работа, баг и т.д., то этой информации место не в исходнике, а в плане/спринте/итерации и т.д. Как мне кажется - исходник должен отражать текущее состояние проекта. Его прошлое отражается в системе контроля версий и баг трекере, а будущее - в плане разработки. А TODO - это какое-то упрощенное все вместе. ;-)
ОтветитьУдалитьПо поводу TODO не соглашусь. Как пример, не всегда в коде должен быть реализован весь функционал. Но оставить заглушки с комментарием и пометкой TODO для будущего релиза бывает удобно.
ОтветитьУдалитьОставленная "на потом" отладочная печать тоже бывает полезна. Но лучше, когда она оформлена единообразно, например,
ОтветитьУдалитьmodule_debug("x=%d", x);
и включается каким-нибудь ключиком - глобально или помодульно.
>автор сомневался и так и не отладил все до конца
После написания код обычно живёт своей жизнью, и бывает нужно включить отладку в конкретном месте уже после того, как автор всё отладил. Неважно, насколько он при этом "сомневался" :)
И потом, если подходить с этой логикой, то и ассерты не нужны... Зачем они, если автор всё отладил и ни в чём уже не сомневается?