Уголовный кодекс разработчика


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

Я сейчас не говорю про «Административный кодекс», куда я как раз и отношу неправильное применение шаблонов, неиспользование тестов, неоптимизированный код, даже харкодинг каких-нибудь настроек и «магические числа» (хотя уже на грани). В этих случаях разная правоприменительная практика. Например оптимизированный код часто сложнее для понимания, чем неоптимизированный. Неоптимальный алгоритм зачастую легче воспринимается при чтении кода, а ведь разработчик 95% времени читает свой или чужой код и только 5% пишет. Или если вы пишите скрипт для друга забесплатно, побыстрее и заходкодили пару настроек, вы скорее всего правильно поступили. Решив, что интеграция туда логики извлечения настроек (и ее тестирования) из отдельных конфигов потребует намного большего времени, чем хардкод.

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

Статья 1. Закомментированный код


Как часто в проектах любого уровня это встречается, человек как бы говорит вам: я немного не уверен, может быть это еще понадобится, а может и нет я — хз, просто оставлю это пока тут. Типичный кодомусорщик. Есть даже целые закомменченные модули. Это опять же очень сильно влияет на читаемость кода, даже несмотря на то, что он идет другим цветом. Решение есть — просто удалить, и все. Но если вы такой хламушник, можете сделать в git спец ветку с названием например warehouse-of-old-trash и ваша душа будет спокойна.

Статья 2. Мертвый код. Мертвые зависимости


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

Сюда же можно добавить бестолковые заглушки. Например вчера увидел такое:

if (true){
  someFunction();
}

На вопрос: «зачем ты это написал?», последовал ответ: «чтобы не забыть, что там должна быть проверка». Логично да? Ну напиши ты комментарий:
// нужно проверить на то-то, 
someFunction();

Тоже не очень, но хоть понятно, что тебя не троллят т.к. данный проект скинулся мне со словами «очень хорошо написанный». Или посреди нормального рабочего кода идет Promise.delay (4000) — типа эмуляция какого-то долгого процесса, который еще не написан. Ну сделай ты моку, сразу будет понятно, что это класс фейковый и вопросов не будет.

Статья 3. Копипаст


Это не про бездумный копипаст со StackOverflow в проект, что тоже грех, а про то, что любой повторяющийся код содержащий более двух операторов должен быть убран в функцию. Это прям болезнь какая-то, в основном свойственная новичкам и любителям вышеприведенного копипаста с so. Лютое нежелание подумать над тем, что пишешь. Есть правда исключения. Например повторная логика, которую написал через какое-то время, забыв, что ее где-то уже писал или когда пишут разные люди, плохо читая остальной код. В любом случае при рефакторинге все должно убираться.

Статья 4. Гигантские методы


О да, этим страдают даже более-менее приличные проекты. По моему убеждению, метод должен быть не больше экрана компьютера (± усредненного конечно). Иногда на том же github сидишь разбираешься в чьем-то коде и понимаешь, что ты уже 3й экран пролистал, а идет та же самая функция, и мозг уже отказывается что-то понимать. Ну разбей ты по некоторым признакам код на функции, ну понятней в сто раз будет да еще и возможно переиспользовать какие-нибудь можно будет. Даже экран это много, в такой размер как исключения могут попадать функции с имплементацией математических выражений, где разбиение на мелкие части понимания может не добавить, иногда лучше чтобы все математическое выражение было в одной функции, но опять же не больше экрана.

Отягчающие обстоятельства


А. Множественность. Один раз, как говорится, можно. Ну например срочно нужно было отдать в релиз и быстро чего-нибудь там закомментили, отключили и тд. Но когда, это по всему проекту и тянется не один год, то это — вдвойне криминал.

Б. По предварительному сговору группой лиц. Если вы пишите один, или свой изолированный модуль это одно, но когда несколько человек отвечают за код содержащий выше описанное это — втройне криминал.

P.S. Чтобы не быть ханжой скажу, что у самого иногда в проектах проскакивают признаки состава преступлений из выше приведенных статей, но я никогда не назову такой свой код хорошим. Просто последнее время часто по работе вижу людей, «надувающих щеки» и говорящих, что вот у них код идеальный и 100% профессиональный. А при первом взгляде, когда еще не видно ошибок высокого уровня (в паттернах там и тд) вылезает вот это вот все.

Комментарии (6)

  • 16 марта 2017 в 16:55

    +1

    // нужно проверить на то-то,
    someFunction ();

    Минутка рекламы — JetBrains IDE умеют распознавать // TODO: … и составлять из них списки (и напоминать при каждом удобном случае, например, останавливая коммит).
    Подозреваю, что похожим трюкам можно научить и более прочие IDE.

    • 16 марта 2017 в 17:15

      0

      В принципе по всем четырем пунктам можно придумать автоматизацию, которая будет перед комитом предупреждать разработчика.
  • 16 марта 2017 в 17:20

    0

    Да, спасибо автору. Очень полезно!
  • 16 марта 2017 в 17:20

    +1

    Resharper тоже отлично распознает тудушки :)
  • 16 марта 2017 в 17:24

    +1

    Пункт 4 тоже прекрасно определяется плагинами в упомянутых выше JetBrains IDE, даже есть функция extract method для рефакторинга сразу по месту. Мне очень помогло выставление всем инспекциям оформления кода одинакового уровня подсветки на уровне синтаксических. Если использовать вашу терминологию — за любую оплошность IDE мне дает максимальный срок, начинаешь серьезно относиться к любым даже самым мелким недочетам, а за пару месяцев это доходит до полного автоматизма.
  • 16 марта 2017 в 18:01

    +1

    А самое грустное — когда приходишь на новое место работы, а там в коде есть сразу все эти проблемы, и причем везде. Тратишь неделю на то, чтобы объяснить, почему так нельзя, и не достигаешь успеха, ведь «мы всегда так делали, мы так привыкли».

© Habrahabr.ru