Источник картинки

Работающий код может иметь изъяны — например, быть недостаточно простым, лаконичным и понятным. Это может быть признаком более глубоких проблем в коде, а избавиться от них можно с помощью рефакторинга. В этой статье разберем наиболее популярные недостатки кода. Материал адаптирован на русский язык вместе с Глебом Михеевым, директором по технологиям Skillbox и спикером профессии «Frontend-разработчик PRO» и преподавателем курса Frontend-разработки в Skillbox Борзуновым Игорем.

Непонятный выбор имени

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

Общие принципы выбора имени:

  • Стратегия выбора последовательна.

  • Из имени понятно назначение переменной.

  • Имена легко различимы между собой.

  • Имя переменной можно произнести вслух.

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

const a = 'Frontend course';

const b = 1;

const c = '100$'

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

const courseName = 'Frontend course';

const courseId = 1;

const coursePrice = '100$'

Длинный метод

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

async function makeAnOrder() {

  const form = document.querySelector<HTMLFormElement>('form#basket');

  const basket = new FormData(form);

  if (!basket.get('name')) {

      throw new Error('Вы не указали имя');

  }

  if (!basket.get('address')) {

      throw new Error('Вы не указали адрес доставки');

  }

  const user = await fetch('/api/user').then(results => results.json())

  basket.set('user_id', user.id);

  basket.set('discount', user.discount);

  fetch('/api/order', { method: 'POST', body: basket })

      .then(results => results.json())

      .then(() => {

          alert('Заказ удачно отправлен')

      })

}

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

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

async function makeAnOrder() {

  const basket = getBasket();

  vaildateBasket(basket);

  await applyDiscount(basket);

  createOrder(basket).then(() => {

    alert('Заказ удачно отправлен')

  })

}

function getBasket() {

  const form = document.querySelector('form#basket');

  return new FormData(form);

}

function vaildateBasket(basket) {

  if (!basket.get('name')) {

    throw new Error('Вы не указали имя');

  }

  if (!basket.get('address')) {

    throw new Error('Вы не указали адрес доставки');

  }

}

function createOrder(basket) {

  fetch('/api/order', { method: 'POST', body: basket })

    .then(results => results.json())

}

async function applyDiscount() {

  const user = await fetch('/api/user').then(results => results.json())

  basket.set('user_id', user.id);

  basket.set('discount', user.discount);

}

Объект бога

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

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

Глеб Михеев: «Часто бывает такая ситуация: лаконичный код по ходу решения задачи обрастает дополнительной логикой. Например, функция записи начинает проверять загружаемый файл, права доступа к нему, конвертировать его в нужный формат, складывать в систему хранения, сбрасывать и прогревать кэш. 

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

Вот почему важно не оставлять такой код как есть, а после разработки функционала проводить рефакторинг. Упрощать код логически, разнося на маленькие простые блоки, которые легко читать, изменять и тестировать».

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

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

let button = document.getElementById("button");

let anotherButton = document.getElementById("anotherButton");

button.onclick = function() {

    modal.style.display = "block";

    modal.style.height = "100%";

    modal.style.width = "50%";

    modal.style.position = "fixed";

}

anotherButton.onclick = function() {

    modal.style.display = "block";

    modal.style.height = "100%";

    modal.style.width = "50%";

    modal.style.position = "fixed";

}

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

function showModal() {

  modal.style.display = "block";

  modal.style.display = "block";

  modal.style.height = "100%";

  modal.style.width = "50%";

  modal.style.position = "fixed";

}

button.onclick = showModal

anotherButton.onclick = showModal

Слишком много параметров

Большое число аргументов функции затрудняет чтение и тестирование кода. Разработчик Роберт Мартин в книге «Чистый код. Создание анализ и рефакторинг» назвал ноль идеальным числом параметров:

Глеб Михеев: «В идеальном случае количество аргументов функции равно нулю. Далее следуют функции с одним аргументом и с двумя аргументами. Функций с тремя аргументами следует по возможности избегать. Необходимость функций с большим количеством аргументов должна быть подкреплена очень вескими доводами».

Надуманная сложность

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

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

Стрельба дробью

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

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

Мутации переменной

Это специфическая проблема JavaScript, которая указывает на изменения переменной. Но мутация отличается от изменения тем, что происходит непредсказуемо. Из-за изменения одной переменной в другом месте меняется и другая, связанная с ней.

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

Хорошая практика в JavaScript — использование так называемого иммутабельного подхода: запрета мутирования переменных. Он позволяет избавиться от этой проблемы. Пример реализации –– библиотека ImmutableJs.

Если воспользоваться ключевым словом let, то значение переменной действительно можно переопределить 

let age = 25;

age = "Двадцать пять";

В случае с const, будет выведена ошибка, так как ключевое слово const неизменно в данном примере 

const age = 25;

age = 'Ягодка опять';

> Uncaught TypeError: Assignment to constant variable.

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


  1. doomguy49
    01.07.2022 02:57
    +4

    Пожалуй, самое отвратительное – большая вложенность if в функциях, часто ещё и совместно с else, вместо проверок от противного с возвратом из функции в случае несоответствия


    1. MyraJKee
      02.07.2022 11:54
      +1

      ) а кто-то считает отвратительным множество возвратов в методе


    1. gscdlr
      03.07.2022 00:21
      -2

      С расширениями подсветки синтаксиса и форматирования структуры даже несколько if () {} else {} смотрятся отлично и отлично читаются, если ветвление умещается на экран.


  1. IvanSTV
    01.07.2022 09:32
    +1

    Все рекомендации не абсолютны. Читал китайский код, в котором на 40 строчек кода было написано 20 выночных функций. Я бы предпочел хорошо оформленную иерархию вложенных операторов if


    1. CrazyElf
      01.07.2022 10:10

      А вот предположим, что вам нужно было бы покрыть этот код тестами. Вы всё ещё предпочли бы иерархию if-ов? )


      1. dopusteam
        01.07.2022 11:07
        +1

        Методы то эти приватные, скорее всего, в тестировании все равно ничего не изменится


      1. IvanSTV
        01.07.2022 12:23

        в функциях там было как правило, по несколько строчек. Я бы это пережил.

        В первую очередь тут надо сверяться с целесообразностью. Я полагаю, что у китайца был набор функций, написанный для других вещей, и он не стал заморачиваться и накопипастил. Себе время сэкономил.

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


  1. dimti
    02.07.2022 15:44

    Хорошо, доступно, понятно.

    Когда тему разжевывают со всех сторон - это выглядет куда лучше, чем просто один пример или одно предложение на одну ситуацию.

    А у вас можно заказывать разработку документации? (Не все же обучать могут, я вот например не учитель, но документация по style guide и best practices в команде нужна)

    Ещё накину про переменные:

    • Соблюдать наименьшее расстояние между объявлением переменной и местом ее использования

    • Следить за тем, чтобы в коде не было неиспользуемых переменных или ни на что не влияющих присваиваний значений.

    • Названия переменных всегда с маленькой буквы - названия классов с большой (может это конечно уже вкусовщина)


    1. gscdlr
      03.07.2022 00:30

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

      gcc -std=* -pedantic -Wall 
      -Wextra -Werror

      Есть ещё много других полезных штук, например, -Wstack-protector


      1. dimti
        03.07.2022 08:29

        Спасибо, интересно. Я до этого в серьезное программирование ещё не влезал. Все только веб, да веб, где вот эта вот динамическая типизация и на память оперативную по большому никто о ней даже не знает ничего. Там такого нет, чтобы в интерпретаторе были встроены линтеры и проверки пременных, там либо только отдельные opensource либы с гитхаб, причем разные и например новичку, чтобы разобраться какая что делает нужно съесть сапог, либо все у JetBrains.

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

        Не историю коммитов в GIT полную посмотреть из cli, ни линтер по-человечески к PHP подключить (пробовали несколько раз со студентом подключить - но это все не то). И GUI для GIT тоже не видел ещё отдельного удобного инструмента.


    1. Moraiatw
      03.07.2022 17:36

      • Следить за тем, чтобы в коде не было неиспользуемых переменных или ни на что не влияющих присваиваний значений.

      Об этом по хорошему, должен предупреждать компилятор/IDE.