Я присутствовал на мероприятии, посвященном мастерству программного обеспечения, пару недель назад, и один из комментариев был «Я уверен, что мы все распознаем плохой код, когда мы его видим», и все мудро кивнули без дальнейшего обсуждения.
Подобные вещи всегда волнуют меня, потому что есть тот трюизм, что все думают, что они водитель выше среднего. Хотя я думаю, что могу распознать плохой код, я хотел бы узнать больше о том, что другие люди считают запахами кода, так как это редко обсуждается подробно в блогах людей и только в нескольких книгах. В частности, я думаю, что было бы интересно услышать обо всем, что является запахом кода на одном языке, но не на другом.
Я начну с простого:
Код в контроле исходного кода, который имеет высокую долю закомментированного кода - почему он там? это должно было быть удалено? это половина готовой работы? может быть, это не следовало закомментировать и делалось только тогда, когда кто-то что-то тестировал? Лично я нахожу такие вещи действительно раздражающими, даже если это просто странная строка, но когда вы видите большие блоки, перемежающиеся с остальным кодом, это совершенно неприемлемо. Это также обычно указывает на то, что остальная часть кода, вероятно, также имеет сомнительное качество.
источник
printf("%c", 7)
как правило, звонит мне в тревогу. ;)Ответы:
Обычно находящийся внутри бессмысленного
try..catch
блока, он привлекает мое внимание. Примерно так же, как и/* Not sure what this does, but removing it breaks the build */
.Еще пара вещей:
if
операторовprocess
,data
,change
,rework
,modify
Тот, который я только что нашел:
Правильно, потому что необходимость грубой силы ваших соединений с MySQL - это правильный путь. Оказалось, что в базе данных были проблемы с количеством соединений, поэтому время ожидания истекло. Вместо того, чтобы отлаживать это, они просто пытались снова и снова, пока это не сработало.
источник
Главный красный флаг для меня - дублированные блоки кода, потому что он показывает, что человек либо не понимает основ программирования, либо слишком напуган, чтобы внести надлежащие изменения в существующую базу кода.
Раньше я также считал отсутствие комментариев красным флажком, но недавно поработав над множеством очень хороших кодов без комментариев, я отошел от этого.
источник
Код, который пытается показать, насколько умен программист, несмотря на то, что он не добавляет реальной ценности:
источник
swap(x, y);
^_^
20 000 (преувеличение) линейных функций. Любая функция, которая занимает более пары экранов, нуждается в перефакторинге.
В том же духе, файлы классов, которые, кажется, существуют вечно. Вероятно, есть несколько концепций, которые можно абстрагировать в классы, чтобы прояснить назначение и функцию исходного класса и, возможно, место его использования, если только они не являются внутренними методами.
неописательные, нетривиальные переменные или слишком много тривиальных неописательных переменных. Это делает вывод о том, что на самом деле происходит загадка.
источник
Что еще хуже, это из коммерческой библиотеки!
источник
Комментарии настолько многословны, что если бы существовал английский компилятор, он скомпилировался бы и работал бы отлично, но не описывал ничего, чего не делает код.
Кроме того, комментарии к коду, с которыми можно было покончить, должны были придерживаться некоторых основных правил:
источник
/
from*/
, поэтому весь код до конца следующего*/
игнорируется. К счастью, подсветка синтаксиса делает такие вещи редкими в наши дни.a
для user_age? В самом деле?i = i + 1; //increment i
Код, который выдает предупреждения при компиляции.
источник
(unsigned int)
чем загромождать мои списки предупреждений / ошибок мягкими предупреждениями. Я бы не хотел, чтобы список предупреждений стал слепым пятном. Это также намного больше ЛАВАШ объяснить другим людям , почему вы игнорируете предупреждение , чем это объяснить , почему вы делаете бросок естественноints
дляunsigned ints
.Функции с числами в имени вместо описательных имен , например:
Пожалуйста, сделайте так, чтобы имена функций что-то значили! Если doSomething и doSomething2 делают схожие вещи, используйте имена функций, которые различают различия. Если doSomething2 является прорывом функциональности от doSomething, назовите его для его функциональности.
источник
mshtml
- это разбивает мне глаза :(Волшебные числа или Волшебные струны.
источник
200
другой стороны ...Возможно не худший, но наглядно показывает уровень реализации:
Если язык имеет цикл for или конструкцию итератора, то использование цикла while также демонстрирует уровень понимания языка разработчиками:
Плохое написание / грамматика в документации / комментариях кушает у меня почти столько же, сколько сам код. Причина этого в том, что код предназначен для чтения людьми и запуска машин. Вот почему мы используем языки высокого уровня, если ваша документация трудна для прохождения, это заставляет меня упреждающе формировать негативное мнение о базе кода, не глядя на нее.
источник
Тот, который я сразу заметил, - это частота глубоко вложенных блоков кода (если, то, и т. Д.). Если код часто имеет глубину более двух или трех уровней, это признак проблемы дизайна / логики. И если он идет как 8 гнезд глубиной, то должна быть чертовски веская причина, чтобы он не разбился.
источник
return
утверждение, но когда он вызывает более 6 уровней вложенности, то я думаю, что он приносит гораздо больше вреда, чем пользы.Оценивая студенческую программу, я иногда могу сказать, что это мгновение. Это мгновенные подсказки:
Мои первые впечатления неверны редко, и эти предупреждающие сигналы звучат примерно в 95% случаев . За одним исключением, студент, плохо знакомый с языком, использовал стиль из другого языка программирования. Раскопки и перечитывание их стиля в идиоме другого языка убрали тревожные колокола для меня, и тогда студент получил полную оценку. Но такие исключения редки.
При рассмотрении более сложного кода, это мои другие предупреждения:
С точки зрения стиля я вообще не люблю видеть:
Это всего лишь ключи к плохому коду. Иногда то, что может показаться плохим кодом, на самом деле не так, потому что вы не знаете намерений программиста. Например, может быть веская причина, по которой что-то кажется слишком сложным - возможно, в игре было другое соображение.
источник
Персональный фаворит / любимая мозоль: IDE генерирует имена, которые вводятся. Если TextBox1 является основной и важной переменной в вашей системе, у вас есть еще одна вещь, которая произойдет, после проверки кода.
источник
Полностью неиспользуемые переменные , особенно когда переменная имеет имя, подобное именам переменных, которые используются.
источник
Многие люди упомянули:
Хотелось бы, чтобы это было реализовано, по крайней мере, они сделали заметку. Что я думаю хуже:
Todo бесполезны и сбивают с толку, если вы никогда не удосужились удалить их!
источник
//TODO
комментарий? Потрясающие!// TODO
, используйте свой трекер ошибок, вот для чего он!Метод, который требует от меня прокрутки вниз, чтобы прочитать все это.
источник
Соединения в именах методов:
Пояснение: причина, по которой этот сигнал звучит тревожно, состоит в том, что он указывает на то, что метод, вероятно, нарушает принцип единственной ответственности .
источник
addEmployee(); updatePayrate();
), то я не думаю, что это проблема.Связывание явно исходного кода GPL в коммерческую программу с закрытым исходным кодом.
Это не только создает непосредственную юридическую проблему, но, по моему опыту, оно обычно указывает либо на небрежность, либо на безразличие, что отражается и в других частях кода.
источник
Языковая независимость:
TODO: not implemented
int function(...) { return -1; }
(так же, как "не реализовано")0
,-1
илиnull
как исключительные возвращаемые значения.Определенный язык (C ++):
array new
который, очевидно, не является безопасным для RAII.printf
.Microsoft C ++ специфично:
C ++ / OOP специфично:
источник
Странный стиль отступа.
Есть пара очень популярных стилей, и люди перенесут эти дебаты в могилу. Но иногда я вижу кого-то, кто использует действительно редкий или даже доморощенный стиль отступов. Это признак того, что они, вероятно, не программировали ни с кем, кроме себя.
источник
Использование множества текстовых блоков вместо перечислений или глобально определенных переменных.
Фигово:
Лучше:
Лучший:
источник
Слабо типизированные параметры или возвращаемые значения в методах.
источник
if...else
блок, он становитсяif...else if...[...]...else
блоком$lesseeloginaccountservice
if
заявления. Пример из кода: кif (!($lessee_obj instanceof Lessee && $lessee_obj != NULL))
которому я стучалif ($lessee_obj == null)
источник
Запах кода: не следование передовым методам
Вот новость для вас: у 50% населения мира уровень интеллекта ниже среднего. Хорошо, так что некоторые люди будут обладать средним интеллектом, но давайте не будем придирчивыми. Кроме того, одним из побочных эффектов глупости является то, что вы не можете распознать свою собственную глупость! Вещи выглядят не так хорошо, если объединить эти утверждения.
Было упомянуто много хороших вещей, и в целом кажется, что не следование передовым методам - это запах кода.
Лучшие практики, как правило, не придуманы случайно, и часто существуют по какой-то причине. Много раз это может быть субъективно, но по моему опыту они в основном оправданы. Следование лучшим практикам не должно быть проблемой, и если вам интересно, почему они такие, как есть, изучите это, а не игнорируйте и / или жалуйтесь на это - возможно, это оправдано, а может, и нет.
Одним из примеров лучшей практики может быть использование curlies с каждым блоком if, даже если он содержит только одну строку:
Вы можете не думать, что это необходимо, но я недавно прочитал, что это основной источник ошибок. Об использовании скобок также всегда говорилось о переполнении стека , и проверка того, что операторы if имеют скобки, также является правилом в PMD , статическом анализаторе кода для Java.
Помните: «Потому что это лучшая практика» никогда не является приемлемым ответом на вопрос «почему вы это делаете?» Если вы не можете сформулировать, почему что-то является лучшей практикой, тогда это не лучшая практика, это суеверие.
источник
Комментарии, в которых говорится: «Это потому, что дизайн подсистемы Froz полностью загружен».
Это продолжается весь абзац.
Они объясняют, что должен произойти следующий рефакторинг.
Но не сделал этого.
Теперь им, возможно, сказали, что они не могут изменить его своим начальником в то время из-за проблем со временем или компетенцией, но, возможно, это было из-за мелочности людей.
Если руководитель считает, что j.random. программист не может сделать рефакторинг, тогда супервайзер должен это сделать.
В любом случае, это происходит, я знаю, что код был написан разделенной командой с возможной политикой власти, и они не реорганизовали скомпрометированные конструкции подсистем.
Правдивая история. Это может произойти с тобой.
источник
Может кто-нибудь придумать пример, когда код должен законно ссылаться на файл по абсолютному пути?
источник
/dev/null
и друзья в порядке. Но даже такие вещи/bin/bash
подозрительны - что, если у вас есть какая-то странная система, которая имеет/usr/bin/bash
?/home/tom/dev/randomhacking/thing.wsdl
. Это преступно безумно, что это поведение по умолчанию./dev/null
: У меня есть привычка при разработке под Windows держать приложения и библиотеки подc:\dev
. Каким-то образом папкаnull
всегда автоматически создается внутри этой папки. Клянусь, я понятия не имею, кто это делает. (Одна из моих любимых ошибок / функций)Ловить общие исключения:
или же
Регион чрезмерного использования
Как правило, использование слишком большого количества регионов показывает мне, что ваши классы слишком большие. Это предупреждающий флаг, который сигнализирует о том, что я должен больше изучить этот фрагмент кода.
источник
Соглашения об именах классов, которые демонстрируют плохое понимание абстракции, которую они пытаются создать. Или это не определяет абстракцию вообще.
Чрезвычайный пример приходит на ум в классе VB, который я когда-то видел и который был назван
Data
длиной более 30 000 строк ... в первом файле. Это был неполный класс, разбитый как минимум на полдюжины других файлов. Большинство методов были обертками вокруг хранимых процедур с именами вродеFindXByYWithZ()
.Даже с менее драматичными примерами, я уверен, что мы все просто «выбросили» логику в плохо продуманный класс, дали ему полностью общее название и пожалели об этом позже.
источник
Функции, которые переопределяют основные функциональные возможности языка. Например, если вы когда-нибудь увидите метод «getStringLength ()» в JavaScript вместо вызова свойства «.length» строки, вы знаете, что у вас проблемы.
источник
Конечно , без каких - либо документации и иногда вложенных
#define
систочник