Если что-то работает - это ещё не значит, что это работает правильно

a.k.a. "Если что-то работает, это ещё не значит, что это вообще должно работать".

Примеры из жизни.

Пример 0.

Как вам такой код?

procedure TForm1.Button6Click(Sender: TObject);
var
 s:string;
begin 
 s:=DateToStr(Now); \\ чтоб к имени прибавлялась дата
 EF.ActiveWorkbook.SaveAs(Form1.Edit.Text + s[1] + s[2] + '_' + s[4] + s[5] + '_' + s[7] + s[8] + s[9] + s[10]); 
end;

Рабочий код? Рабочий. Ведь тут "код подошёл, всё работает, спасибо!".

Хороший ли это код? Да ни в коем разе!

Тут сразу несколько проблем. Первое что бросается в глаза - посимвольная склейка. Почему не склеивать сразу строки? Но это так, придирка. Настоящая проблема - неявное предположение о фиксированном формате даты. DateToStr переводит дату в строку, основываясь на текущих региональных установках. Которые могут меняться.

Что означает, что год у меня на машине может быть представлен двумя цифрами, а не четырьмя - и тогда этот код красиво вылетит с Access Violation или выведет мусор. А ещё год может идти не последним числом, а первым.

Как это исправить? Ну, вы можете зафиксировать формат даты-времени, вызвав перегруженный вариант DateToStr, принимающий дополнительный параметр - настройки форматирования. Но гораздо лучшим решением будет использование функции вроде FormatDateTime:

procedure TForm1.Button6Click(Sender: TObject);
var
  s: string;
begin 
  s := FormatDateTime('dd_mm_yyyy', Now); // чтоб к имени прибавлялась дата
  EF.ActiveWorkbook.SaveAs(Form1.Edit.Text + s); 
end;

Очень просто. Код ещё не идеален, но хотя бы он будет работать корректно в любых условиях (и, кроме того, теперь он выглядит красивее и понятнее!).

Пример 1.

Начнём с этого - всем известная реализация Base64 на http://www.delphi3000.com/ от Daniel Wischnewski из Delphi-PRAXiS - далеко не "noname" товарищ. Исходник имеет рейтинг 9/10. Разошёлся по многим FAQ и используется в куче программ (в том числе, он использовался в EurekaLog).

Казалось бы, что может пойти не так?

Оказывается, что эта реализация вообще не работает (под этим подразумевается, что она не работает корректно, иными словами, не должна работать вообще). Конкретно: этот код содержит memory corruption bug. Ещё конкретнее: в Base64Encode, третья строка "mov EAX, EBX" - какой-такой ещё EBX? Он неопределён. Параметры в register-процедуру передаются в регистрах EAX, ECX и EDX. EBX же вообще не связывается с параметрами, при входе в функцию значение регистра EBX - произвольно. Правильный вариант кода выглядит, например, так: "mov EAX, InSize" (как это и сделано в Base64Decode).

Столкнулся я с этим в поддержке EurekaLog. Мы получили баг-отчёт с memory corruption на ровном месте. К счастью была предоставлена воспроизводимая демка, и я по ней вышел на этот код, который когда-то был взят с Delphi3000 и встроен в EurekaLog. Вместе с багом. Удивительно, что это работало несколько лет. Ещё удивительнее, что никто, использующий этот код или его копии из FAQ, за 7 лет не нашёл этой ошибки. Окей, наверняка кто-то сталкивался с загадочными багами, но причину найти не могли. Вот она, сила Copy&Paste.

Конкретно в варианте с EurekaLog EBX наследовался из вызывающего, где он оказывался равен OutSize из Base64EncodeStr (вместо InSize; InSize <= OutSize). Соответственно, Base64Encode кодировала больше данных, чем было нужно (заодно портя память, т.к. выходной буфер был предназначен только для содержания кодированных InSize байт, а не OutSize байт). Например, вместо взятия, скажем, 11-ти байт с входного буфера и записи их в 16-ти байтный выходной буфер, эта функция брала 16 байт из исходного буфера (ага, шанс на AV не сработал) и записывала 24 байта в выходной буфер (который имел размер 16 байт). Да, алгоритм работал, да он выполнял кодирование/декодирование. Но работал неверно - он портил память. Но это работало. В 99.9% случаев всё "просто работало" и не было никаких ошибок или вылетов. Но иногда ваше приложение вылетало. Причём в месте, совершенно никак не связанным с исходной проблемой. Это был какой-то совершенно другой код.

Ситуацию в этом случае усложняет то, что пример написан на ассемблере, что затрудняет его чтение и анализ.

Вот корректный вариант реализации Base64, который использует только Паскаль и, более того, работает быстрее вышеуказанной ассемблерной реализации. Не забывайте только, что этот код, попадая под определение "опубликовано в интернете" - говно.

Примечание: кажется, не все понимают, зачем я выложил этот код. Код выложен только как пример исправления. Это не означает, что его нужно брать и использовать. Он не предназначен для этого. Вместо него нужно использовать стандартный модуль EncdDecd (в новых версиях Delphi - Soap.EncdDecd).

Пример 2 - посыл сообщений фиксированному недокументированному окну будет работать... до следующего обновления системы! Ну, конечно же вы будете винить "кривую Windows", а не свою программу.

Пример 3 - вы ошибочно передаёте GetDesktopWindow куда-то вместо 0. Это будет работать... пока вызывающий не захочет показать окно. Это может не происходить у вас, но происходить на машине вашего друга. Или не происходить в XP, но происходить в Windows 2000. И снова: будете ли вы винить свою программу или "кривую Windows, написанную глупым Биллом Гейтсом на коленке"?

Пример 4 (в четырёх частях). В DllMain чрезвычайно сложно сделать всё правильно. Если вы что-то писали в initialization или begin/end - почти наверняка вы уже облажались. Если ваши DLL при этом работают - то только благодаря случайности, как и в предыдущих примерах. Но иногда они будут вылетать. А вот и пример. Довольно искусственный, зато хорошо воспроизводимый.

Пример 5 и его очень похожий вариант - вы можете ошибиться в сигнатуре функции и это будет работать, пока... не изменится какая-то маленькая деталь в вызывающем. Иногда достаточно просто пересобрать проект с другими настройками или другой версии компилятора, чтобы работающая программа сломалась бы. Посмотрите по ссылке пример с RunDLL32 - как часто вы сами видели такое во всяких "полезных советах" и "скрытых возможностях Windows"?

Пример 6 - если вы не позаботитесь о выравнивании ваших данных, то ваш код будет работать. Но очень редко выдавать неверные результаты. Удачи вам поймать эту ситуацию под отладчиком!

Это не говоря уже про такие тривиальные вещи. Вот ещё пример. И ещё. Если вы использовали в своём коде недокументированную возможность, то как бы удобна и нужна она ни была - откажитесь от неё. Потому что иначе ваша программа перестанет работать с выходом новой версии ОС или даже просто сервис пака.

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

Мораль истории: когда кто-то говорит "у меня это работает, спасибо", опасайтесь: тут что-то не так.

Выводы о качестве кода или хотя бы о его минимальной корректности и пригодности не должны основываться на тестовом прогоне. Вы должны вдумчиво анализировать код. Используйте теоретическую базу: обоснуйте, почему этот код должен работать, не полагайтесь на множественные прогоны (хотя отбрасывать их, конечно же, не стоит).

Читать далее: 90% кода в интернете - говно.