Похудеть на 5 кг за 5 секунд? Легкий! Просто отрежь себе руку.

Большинство из нас знакомы с концепцией целей SMART: конкретных, измеримых, достижимых, реалистичных и своевременных. Вы можете возразить, что отрубить себе руку на самом деле нереально, но по типичному определению SMART это определенно так. В этом заключается недостаток SMART-целей для инженеров; Я бы сказал, что устойчивость необходимо добавить в смесь. Есть разница между простым достижением вашего результата и достижением его устойчивым или, когда дело доходит до кода, поддерживаемым способом.

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

Вот старая версия кода:

Хотя код выполняет свою работу — в том смысле, что он активно не ломается, — его можно было бы немного отполировать в нескольких местах, что сильно изменило бы его читабельность и удобство сопровождения. .

Первая проблема

Название на самом деле не говорит мне, что происходит. `getsWhatsRequired` — это имя, которое применимо практически к любой функции, которая получает и/или форматирует данные. Он не говорит мне, почему эта функция живет сама по себе и что она делает особенно полезного.

Вторая проблема

Внутри URI происходит целый ряд повторений. Этот код повторяется 6 раз внутри этой одной функции. Повторение 6 раз затрудняет рассуждение, потому что вам нужно либо прочитать каждое повторение и решить, делают ли они точно то же самое, ИЛИ вы в конечном итоге пропускаете это и предполагаете, что оно делает то же самое. вещь каждый раз. В данном случае это проделывает одно и то же каждый из шести раз, но вам нужно приложить много умственной энергии, чтобы понять это.

Третья проблема

В повторяющемся коде есть намерение и цель, которые теряются. Мы не обрезаем строки и не добавляем косые черты и прочее просто так, мы делаем это с определенной целью! Но почему? Из этого кода вообще ничего не понятно.

Четвертая проблема

Эти двойные равны? Они несущие! Во время некоторой незначительной очистки кода один из моих коллег пошел и преобразовал их в оператор строгого равенства ===, как предложил ESLint, потому что не было никаких причин думать, что это должна быть свободная проверка равенства.

Оказывается, проходящие значения были не 1, они были на самом деле «1». Это совсем другое, и это вызвало инцидент, потому что мы не отправляли файлы в течение определенного периода времени.

Это можно улучшить одним из двух способов:

  1. Если проходящее значение всегда является строкой, вместо этого используйте строковое представление с ===.
  2. Если значение передается либо в виде строки, либо в виде числа, и есть действительно веская причина для того, чтобы это продолжалось, используйте двойное равенство и задокументируйте это конкретное поведение, чтобы Будущее Вы / ваш коллега знает, что вы зависите от этого очень специфического поведения.

И пятый (и, безусловно, самый простой выбор)

Если вы собираетесь определить переменную, а затем немедленно вернуть ее, вы можете просто немедленно вернуть это значение, не присваивая его сначала переменной. Это небольшая деталь, но она делает функцию короче.

Вот новый код:

Большинство изменений, которые я сделал выше, довольно хорошо соответствуют возможностям, которые я только что упомянул, но есть одна область, на объяснение которой я хочу потратить немного больше времени:

Это две строки, которые я ввел вместо кода, который повторялся шесть раз.

Вы могли бы легко утверждать, что эти две строки могут быть одной строкой, которая читается так:

Это было бы еще более эффективно, не так ли? Верно! Это будет 100% правильно! И вот тут в игру вступает контекст. Вы бы не узнали этого, прочитав этот код самостоятельно, но все активы ордера? Они загружаются в сервис активов заказов, причем папка представляет собой идентификатор заказа без префикса.

Зная, что идентификатор заказа без префикса имеет особое значение вне этого контекста, я думаю, что лучше отделить нарезку префикса идентификатора от композиции пути.

Итак, подробно раскритиковав этот фрагмент кода, я хочу прояснить несколько моментов:

Инженер не не виноват в том, что этот код, который был объединен, был неотшлифован. Говоря от имени EM и штатных инженеров, мы обязаны устанавливать стандарты и поддерживать наших коллег в написании кода, который хорошо отполирован и удобен в сопровождении, и здесь мы этого не сделали. Усвоен ценный урок.

Кроме того,ничего из этого не относится к коду, который вы не готовы объединять. Код, который я пишу, когда впервые начинаю что-то решать, часто сильно отличается от кода, который я получаю в итоге. Не тратьте время на полировку кода, который вы выбросите через 15 минут.

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

«Вместо того, чтобы впасть в паралич анализа, когда я пытаюсь найти «лучший» способ написать кусок кода, я часто просто начинаю писать самый дрянной, самый грубый, что бы ни пришло в голову, сначала вещь, которая работает. Затем я напишу свои тесты (если я еще не написал их с помощью TDD, что я, конечно, рекомендую). Теперь, когда у меня есть подстраховка, что код делает то, что нужно, теперь я могу очистить его и быть уверенным, что он все еще работает». — Эрин Дойл, штатный инженер-программист

И я оставлю вас с прекрасным резюме вышеизложенного от Росса Мартина, главного инженера:

‍"Заставьте это работать, сделайте это правильно, сделайте это быстро - в этом порядке".

Первоначально опубликовано на https://www.lob.com штатным инженером Rich Seviora.