494 читателя, 26 постов
Администрация
Модераторы
Блог для публикования отрывков программ, которые представляют из себя результат бессознательного потока мыслей программиста. Ну, или отсутствие мыслей такового при написании.
Причины и следствия
Здесь я хочу поделиться с вами тремя примерами неадекватного кода. И в то же время постараюсь разобрать и классифицировать каждый случай. Тем самым расскажу не только «что такое плохо?», но и «почему?»
Не так давно пришлось воскрешать некогда очень популярный в своих кругах проект, занимавший первые строки в поисковиках и оставивший ощутимый след в архиве. Сейчас он тихонечко катится «на нейтралке». После глобальной переработки и запуска проекта в полную силу, я обязательно напишу «как это было». А сейчас о том коде, который не прошел цензуру.
Похожий код уже публиковался на хабрахабре. Коротко. По ссылке: выборка идентификатора для вставки нового элемента в таблицу базы данных MySQL (выбирается наибольший идентификатор, прибавляется единица, вставляется новая запись). Автор шедевра, который я вам хочу представить, пошел еще дальше. ID выбирается случайно, попытки вставить запись не прекращаются до тех пор, пока генератор псевдослучайных чисел не угадает свободный ID.
Код выглядел примерно так:
Причина. Незнание автором особенностей SQL (auto_increment в частности).
Совет. Учиться, учиться и еще раз учиться. Читать умные книжки, смотреть чужие коды, критиковать собственный код, просить совета у более опытных программистов.
В десятом классе на уроке информатики я получил задание написать программу, определяющую является ли введенное число полным квадратом. Программа была написана и зачтена. Через два года, пробежав глазами по коду, я не смог понять почему он работает. Алгоритм заключался в сравнении корня введенного числа и частного введенного числа и его корня (см. код чуть ниже).
Выглядело это так:
Причина. Я не проанализировал свой код. С математической точки зрения, условие «sqrt(x) = x/sqrt(x)» выполняется всегда (при x > 0), и лишь благодаря ограничению разрядной сетки операндов программа выполняла свою функцию (в конце дробной части возникала погрешность).
Совет. Понять свой код. Убедиться в том, что в программе протекают задуманные процессы.
И напоследок страшная сказка на ночь. Следующий код, в отличие от приведенных выше, никогда и никем не был написан (я очень на это надеюсь). Хотя его аналоги на других языках программирования, похоже, встречаются на самом деле. Например, #define TRUE FALSE (С, я полагаю?), упомянутый ранее. Или пресловутая «собака» (@) в PHP.
Ассемблер:
Я не стану уверять вас, что этот код работоспособен (с ассемблером имел дело давно… и неправда). Общий смысл, который я хотел передать: в стек помещается адрес сегмента стека, в регистр адреса сегмента стека пишется случайное значение, потом некоторые действия и «востановление» регистра адреса сегмента стека из «стека». Что будет дальше никому неизвестно. Подобные случаи грозят очень долгой отладкой.
Причина. Абсолютно не продумана последовательность операций. Не учтены последствия к которым может привести безобидный набор команд.
Совет. Контролировать косвенное влияние написанного кода на то, что было написано ранее или может быть написано в будущем. Недопускать неявных ограничений, накладываемых кодом, на программу в целом и отдельные ее участки.
Все написанное выше можно найти в любой, мало-мальски грамотной, книжке по программированию. И, возможно, теперь, Вам захочется прочитать одну из них. Также, надеюсь, теперь, Вы станете внимательнее относиться к Вашему коду.
upd: Ввиду появления большого количества защитников «случайных» идентификаторов в таблицах БД, отвечу всем сразу: если необходимо скрыть реальные ID, нужно использовать mod_rewrite, а не коверкать БД.
Недостаток знаний/опыта.
Не так давно пришлось воскрешать некогда очень популярный в своих кругах проект, занимавший первые строки в поисковиках и оставивший ощутимый след в архиве. Сейчас он тихонечко катится «на нейтралке». После глобальной переработки и запуска проекта в полную силу, я обязательно напишу «как это было». А сейчас о том коде, который не прошел цензуру.
Похожий код уже публиковался на хабрахабре. Коротко. По ссылке: выборка идентификатора для вставки нового элемента в таблицу базы данных MySQL (выбирается наибольший идентификатор, прибавляется единица, вставляется новая запись). Автор шедевра, который я вам хочу представить, пошел еще дальше. ID выбирается случайно, попытки вставить запись не прекращаются до тех пор, пока генератор псевдослучайных чисел не угадает свободный ID.
Код выглядел примерно так:
$id = 0;
while (!$id || mysql_error()) {
$id = rand(1, 10000000);
mysql_query("INSERT INTO `table` (id) VALUES ('".$id."'");
}Причина. Незнание автором особенностей SQL (auto_increment в частности).
Совет. Учиться, учиться и еще раз учиться. Читать умные книжки, смотреть чужие коды, критиковать собственный код, просить совета у более опытных программистов.
Непонимание.
В десятом классе на уроке информатики я получил задание написать программу, определяющую является ли введенное число полным квадратом. Программа была написана и зачтена. Через два года, пробежав глазами по коду, я не смог понять почему он работает. Алгоритм заключался в сравнении корня введенного числа и частного введенного числа и его корня (см. код чуть ниже).
Выглядело это так:
readln(x);
if sqrt(x) = x/sqrt(x) then
writeln(x, ' - полный квадрат')
else
writeln(x, ' - не является полным квадратом');Причина. Я не проанализировал свой код. С математической точки зрения, условие «sqrt(x) = x/sqrt(x)» выполняется всегда (при x > 0), и лишь благодаря ограничению разрядной сетки операндов программа выполняла свою функцию (в конце дробной части возникала погрешность).
Совет. Понять свой код. Убедиться в том, что в программе протекают задуманные процессы.
Невнимательность.
И напоследок страшная сказка на ночь. Следующий код, в отличие от приведенных выше, никогда и никем не был написан (я очень на это надеюсь). Хотя его аналоги на других языках программирования, похоже, встречаются на самом деле. Например, #define TRUE FALSE (С, я полагаю?), упомянутый ранее. Или пресловутая «собака» (@) в PHP.
Ассемблер:
push ss
mov ss, 01f7
; ...
pop ssЯ не стану уверять вас, что этот код работоспособен (с ассемблером имел дело давно… и неправда). Общий смысл, который я хотел передать: в стек помещается адрес сегмента стека, в регистр адреса сегмента стека пишется случайное значение, потом некоторые действия и «востановление» регистра адреса сегмента стека из «стека». Что будет дальше никому неизвестно. Подобные случаи грозят очень долгой отладкой.
Причина. Абсолютно не продумана последовательность операций. Не учтены последствия к которым может привести безобидный набор команд.
Совет. Контролировать косвенное влияние написанного кода на то, что было написано ранее или может быть написано в будущем. Недопускать неявных ограничений, накладываемых кодом, на программу в целом и отдельные ее участки.
Все написанное выше можно найти в любой, мало-мальски грамотной, книжке по программированию. И, возможно, теперь, Вам захочется прочитать одну из них. Также, надеюсь, теперь, Вы станете внимательнее относиться к Вашему коду.
upd: Ввиду появления большого количества защитников «случайных» идентификаторов в таблицах БД, отвечу всем сразу: если необходимо скрыть реальные ID, нужно использовать mod_rewrite, а не коверкать БД.
комментарии (95)
Однако неплохая база, в которой 10 000 000 записей, значит и проект не просто хомяк :)
Уверен, тяжело вам пришлось!
(кстати, на вышеупомянутом сайте было написано сколько элементов в этой таблице).
Если кто-то может что-то нехорошее сделать зная id, то нужно удалить весь код и писать заново.
mt_rand в состоянии давать равномерно распределенные псевдо-случайные числа от 1 до 231 — 1, период у нее вообще огромен, короче, для целей генерации уникального INT id более чем подходит.
Иногда такие размашистые формулы получались :)
>С математической точки зрения, условие «sqrt(x) = x/sqrt(x)» выполняется всегда.
Я зануда — при x=0 не выполняется.
i= -1/i
Это если с математической точки зрения.
На компе такое, если это не специальный пакет, скорее всего не выполнится.
if 1 == 2: # блин, этот участок кода никогда не выполняется :(
Причина: 1 не равен 2-м
Совет: используйте «if 1 == 1: ...»
Могу предположить, что там изначально стояло другое, осмысленное условие, но в процессе отладки оно многократно менялось, и/или автор не захотел вымарывать большой кусок кода полностью, а закомментировать было лень, и/или в этом блоке используется альтернативный вариант алгоритма или неиспользуемая на данный момент недописанная функциональность и автор хотел сохранить его на будущее (а если закомментируешь, то другой программист или даже сам можешь случайно стереть закомментированный код, как ненужный), вот и оставил такое.
После рефакторинга, скорее всего, этот кусок был бы удалён, но когда проект уже работает, может быть не до рефакторинга — работает, значнит ничего не трогай :)
Сам я часто использовал такой кусок:
a = 1
if a == 2 call somefunction
и т.д.
чтобы иметь возможность в режиме отладки вызвать какую-нибудь функцию, которая в нормальном режиме работы программы не нужна.например, просмотреть статус какого-нибудь объекта, записать что-нибудь в лог и т.п.
потому что изменить значение переменной «а» в отладчике легко, а вот вызвать какую-нибудь произвольную функцию скорей всего невозможно (в firebug возможно всё, потому я больше не пользуюсь таким приёмом :)
#ifdef SOME_FLAG
…
#else
…
#endif
#ifdef SOME_FLAG
…
#else
…
#endif
Некоторые редакторы, кстати, помечают блок между #if 0 и #endif как комментарий.
Код же как он написан, является бомбой замедленного действия, как потому, что будет прогрессирующий апокалипсис при количестве записей, приближающемся к 10 млн, так и потому, что забивается на то, почему собственно был mysql_error() — может база отключилась.
У Дурова id в контакте 1, сразу интересно, у кого 2, 3 и т.д., т.е. можно делать какие-то выводы о том, как стартовал проект, о личных связях г-на Дурова, например.
В конце концов, это доп. звено в цепи security through obscurity: если злоумышленник не может предсказать идентификаторы внутри вашей системы, ломать ему будет сложней.
Использование же праймари ключ таким образом…
Я наверное сегодня ночью не усну, после увиденого кода…
По поводу разов — небо упадет на землю, если будет у вас вероятность коллизии 10-5, а не 10-20?
Что же делать — давайте двигаться к этой культуре.
Кстати, по первой незадаче — можно же max(id)+1. Если в транзакции.
Но я очень сомневаюсь, что человек не знающий про auto_increment, знает о существовании InnoDB и уж тем более транзакций.
→
выстрелить себе в ногуперенести сегмент стека :)Хочется верить, что на ассемблере пишут только профессионалы. Страшно представить, что бы творилось, будь он проще для освоения/популярнее.
Идею мне подсказал коллега (сами потом долго смеялись), а «код» написал я, специально для статьи.
вопервых потому что слишком маленький или по другой причине.
Нельзя брать кусок кода и сразу делать выводы.
К моменту разработки такое решение могло оказаться максимально приемлемым!
А тут пытаются…
Корме того есть понятие application lifetime по истечении которого надо делать
refactoring.
Что Вы судя по всему успешно и сделали.
Проверка на true:
bool value;
…
if (value.ToString().Length == 4)
{
…
}
Совет. Чтобы понять свой код нужно написать тест к этому коду.
так вот алгоритм открытия файла у него выглядел следующим образом:
// Псевдокод:
while(!file.Open())
{
// Подождем еще немножко
thread.Sleep(1000);
}
P.S. Комментарий в коде выглядел именно так, как привел его я :)
А тормоза учитывали? — тут вам не простой регэксп написать. А если не апач и вообще не веб?
Я и сам могу выдумать много способов, как скрыть реальные ID, и мне даже больше о душе «не коверкать БД», но дело-то не в этом. Дело в том, что «защитники», в частности, я, утверждают, что такой подход имеет смысл и право на существование, вы же упорно списываете его в кодобред.
karbas@arc|~$ python --version
Python 2.5.1
karbas@arc|~$ python -c 'print 1==1
True = False
print True'
True
False
undefined = true;.Нет гарантий, что некий не слишком эрудированный кодер не выберет для своей переменной такое говорящее имя, а последовать могут всякие странности типа обращений к несуществующим полям и объектам там, где всё, казалось бы, проверяется, в чужих, сторонних, «взрослых» библиотеках. Кстати, и защита от этой пакости тоже есть: если ваша библиотека изолирована в функцию (так обычно делают всякие «приватные» поля и методы, см. классиков), можно объявить в ней
var undefined;и жить спокойно.