Как нельзя и как можно!

0. Нельзя черпать вдохновение на сайте http://govnokod.ru/pascal, но можно и нужно задуматься о своем месте в мироздании. Возможно в качестве дворника, или кондуктора Вы принесете больше пользы человечеству.

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

Не нужно чистить только что созданный объект, а вот разрушать созданный локальный объект следует, при чем обязательно с использованием try finally. Помните сон разума рождает чудовищ.

2. Не забывайте, что парные процедуры BeginUpdate, EndUpdate, DisableControls, EnableControls и т. п. требуют try finally точно так же как и Create, Free

Если не использовать EnableControls, в блоке finally, то любое исключение приведет к потере работоспособности формы и вероятно потребует перезагрузить приложение. Для простого пользователя это бóльшая проблема чем утечка памяти из-за того, что некоторый объект не был бы разрушен.

3. Не следует использовать оператор GOTO, откройте для себя такое понятие как структурное программирование . Требование может показаться очевидным, но тем не менее кто-то продолжает использовать этот оператор.

4.Не добавляйте и не оставляйте информацию, которая затрудняет анализ кода поиск нужной информации и массовое внесение изменений.
Не оставляйте закомментированные участки кода, лишние пустые строки, неиспользуемые переменные, самоочевидные комментарии и пр. Если в рамках текущей задачи нет времени на более тщательную проверку, то хотя бы добавляйте понятный комментарий в формате DoTo.


Еще пример:


Спасибо капитан Очевидность, мы знали, что если файл пустой, то в нем ни чего нет, хотелось бы знать какой файл, почему пустой, что в строке ниже удаляется...

5. Не смешивайте визуальное и не визуальное программирование. Например: если форма в файле dfm называется "Список клиентов", а где-то в коде оно меняется на "Список абонентов", то это несет дополнительные сложности, если нужно массово изменить все формы связанные с клиентами. Т.е. свойства которые необходимо изменять в коде должны меняться только в коде, а в dfm-файле должно быть установлено умолчательное значение. Тоже самое относится к тексту запросов и текстовым сообщениям.

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

7. Недопустимо чтобы после ваших изменений появлялись новые Hint`ы и Warning`и. Старые Hint`ы и Warning`и следует устранять, если они расположены около ваших изменений.

8. Нельзя бездумно обращаться к объектам внутри обработчика события. Что произойдет если это же событие будет присвоено другому объекту? Что будет если исходный объект кто-то скопипастит? Вот характерный пример:

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

Этот обработчик можно безопасно использовать в различных ситуациях. Можно было бы еще и переименовать его, чтобы было видно, что он не привязан к конкретному гриду.
Такой подход не может рассматриваться как общее требование, потому, что в некоторых случаях логика работы требует обращения к какому-то конкретному объекту, таким образом требуется обдумывать решение в каждом конкретном случае.

9. Нельзя обращаться к объектам, которые могут быть пустыми. Нельзя выполнять приведение типов, если нет уверенности в совместимости типов. Проверяйте значения на допустимость в обработчиках событий! Например:


Очевидно, что DataSet может быть nil, DataSource может быть nil, Sender может быть не TBbsDBGridEh, т.е. в одной строке три потенциальных места для ошибки. Проверять можно как показано в предыдущем примере. В случае недопустимых, или пустых значений, в зависимости от логики работы можно либо ни чего не делать, либо поднимать исключение с человеческим текстом.

10. Не следует освобождать не инициализированные объекты.

Что будет, если ошибка возникнет в конструкторе? Clipboard будет содержать мусор, мы попытаемся обратится к этому объекту в разделе finaly, и получим другую ошибку, а информацию о первой ошибке потеряем. Что будет, если ошибка произойдет в ClipBoard.Close? Мы не уничтожим объект и получим утечку памяти. Вариант исправления:

Если в конструкторе произойдет ошибка, то в раздел finally мы не попадем. Общий шаблон использования объектов такой:

или



11. Не следует вызывать один пользовательский обработчик события из другого события. Например:

Здесь по нажатию Ctrl+C происходит вызов обработчика двойного клика, который копирует данные в буфер обмена. Из названия gSearchResultDblClick ни как не следует, что обработчик этого события должен что-то скопировать в буфер обмена. Вполне возможно, в будущем к двойному клику будет привязана другая более стандартная функциональность, при этом и обработчик Ctrl+C перестанет работать. Лучше вынести код gSearchResultDblClick в отдельный метод и во всех обработчиках событий вызывать этот метод. А еще лучше создать действие (Action), и обрабатывать нажатия клавиш используя возможности Delphi, вместо того, чтобы изобретать свой собственный велосипед.



12. Ни когда не освобождайте объект внутри метода этого объекта в Delphi это недопустимо. Также недопустимо уничтожение объекта внутри обработчика события, объекта, т.к. обработчики событий вызываются из методов объекта. Например нельзя уничтожать форму в обработчике события OnClose этой формы:

Здесь можно присвоить параметру Action значение caFree, но лучше всегда использовать стандартный шаблон работы с модальной формой Create-ShowModal-Free.

13. Не используйте вложенные with, не используйте большие блоки кода внутри with. Например:


Надо сказать, что большие методы и блоки кода сами по себе затрудняют анализ кода и обычно являются дурным тоном а также признаком низкого профессионализма. Использование же with особенно сильно запутывает логику работы программы, при этом не позволяют использовать переход по ctrl+click и не позволяют видеть значения переменных в процессе отладки. Поэтому надо всегда избегать использования оператора with за исключением очень коротких и очевидных методов. Лучше вообще ни когда его не использовать.

14. Не используйте Free для глобальных переменных и полей

Здесь при первом вызове, объект frmEditPayDoc равен nil, затем он создается и разрушается. После Free переменная frmEditPayDoc содержит ссылку на разрушенный объект но не nil. Таким образом при повторном вызове будет обращение к разрушенному объекту и ошибка доступа к памяти. В исходном коде ошибка маскировалась большим количеством дублированного кода, который в примере не показан. Чтобы избежать таких ошибок используйте глобальный стандартный метод FreeAndNil, который разрушает объект и обнуляет значение переменной.

15. Избегайте дублирования кода, ибо "копипаст" — зло. См. пример из п. 14. Единственная разница в блоках if/else это создание объекта, если он был пустым. Таким образом очень легко переписать условие следующим образом:


Но гораздо быстрее сразу писать без дублирования кода, чем потом пытаться найти отличия в двух ветках условия.
Вот еще характерный пример:

За длиннющим текстом "qSomeFriendlyQueryName.FieldByName('Some_Friendly_Field_Name').Value" теряется ясность понимания кода и что вообще предполагается сделать. Хотя если была цель сломать мозг своему коллеге, то стиль выбран правильно.

16. Не оставляйте Hint и Warning. Warning рекомендуется интерпретировать как ошибки компиляции.

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

Ошибка довольно серьезная, перед выполнением цикла значение суммы может содержать любое значение, но с большой вероятностью оно будет содержать 0, таким образом на этапе тестирования ошибка не будет замечена, однако при длительной работе там может содержаться любое значение и кто-то попадет на "лавэ" без каких-либо предупреждений (в 90-х за это отправляли раков кормить).

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

Здесь видно, что обработчик события aAnnulPayUpdate использует то значение N9.Enabled, которое сам же меняет. Это происходит не явно, и ошибку трудно найти. Можно просто добавить свойство с удобочитаемым названием и использовать его.

Одна строчка написанного кода исключила бы ошибку и сделала бы код более ясным.

18. Не сравнивайте результат окна диалога с неумолчательным значением (обычно mrNo).

Что будет, если кто-то добавит третью кнопку mbCancel? Что будет если пользователь нажмет крестик в верхнем правом углу? Действие будет выполнено, что неожиданно и не правильно. Можно конечно сделать крестик недоступным и надеяться, что ни кто не добавить кнопку Cancel, но лучше переписать условия так:


Общий шаблон для создания условий выглядит так:



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

Тип TDateTime это обычное число с плавающей точкой (Double), в целой части хранятся сутки (количество дней прошедших с 30.12.1899), в дробной части хранится время. Для получения целой и дробной части числа можно использовать например функции Trunc и Frac.


Еще характерный пример.

Здесь трудно с уверенностью сказать, какие мысли были в голове у автора, скорее всего см. п. 1, однако допустим, что автор хотел округлить дату и время до целых долей секунды. В реальности происходит преобразование даты и времени в строку в строго фиксированном формате, а обратно происходит преобразование из строки формат который указан в региональных настройках компьютера, что приводит к ошибке. Есть много вариантов округления, например такой:

Полезными могут оказаться также функция Math.RoundTo и константы HoursPerDay, MinsPerDay, SecsPerDay, MSecsPerDay.

20. Ни когда не устанавливайте свойство формы Position в значение poDesktopCenter. Это означает, что форма будет расположена посередине рабочего стола, который представляет из себя прямоугольник охватывающий координаты всех имеющихся мониторов. При наличие двух мониторов форма будет расположена между ними, это неожиданно и не правильно, используйте значение Position = poScreenCenter.

21. Не следует забывать, что в выпадающем списке может быть выбрано недопустимое значение с ItemIndex = -1.

Здесь в некоторых случаях будет ошибка. Которую трудно обнаружить на этапе тестирования.

22. Если изменяемое свойство некоторого объекта это другой объект, то нельзя просто ограничиться присваиванием ссылки.

Представим себе, что сначала присвоили некоторое значение, а потом удалили экземпляр объекта TSession. Получается, что поле FSession содержит не пустую ссылку на несуществующий объект. Обращение по такой ссылке приведет к ошибке при этом ни как не возможно проконтролировать допустимость обращения. Вот работающий шаблон:

Теперь если экземпляр объекта будет удален, то поле FSession будет содержать значение nil.