Искусство рефакторинга: 5 советов по написанию лучшего кода

1
873
views

Перевод статьи «The Art of Refactoring: 5 tips to Write Better Code».

Рефакторинг и улучшение кода
Image by Free-Photos from Pixabay

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

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

Решение этой (невероятно) распространенной проблемы состоит в том, что нам нужно делать свой код как можно более понятным и информативным. Я бы сказал, что код должен быть понятен даже не-технарям. Пришла пора отбросить все наши самооправдания и улучшить качество нашего кода.

Писать чистый код не так уж и сложно. В этом руководстве вы найдете 5 простых подходов к улучшению кода (с практическими примерами).

Избавляемся от конструкций switch

Обычно мы используем switch, чтобы избежать создания слишком больших блоков if else if. Но конструкции switch слишком многословны, их сложно поддерживать и еще сложнее заниматься их отладкой. Они загромождают наш код и, по моему скромному мнению, имеют странный, неудобный синтаксис. При добавлении большего количества cases нам приходится вручную добавлять каждый блок case и break, а это способствует появлению ошибок.

Давайте рассмотрим пример конструкции switch:

function getPokemon(type) {
  let pokemon;
  switch (type) {
    case 'Water':
      pokemon = 'Squirtle';
      break;
    case 'Fire':
      pokemon = 'Charmander';
      break;
    case 'Plant':
      pokemon = 'Bulbasur';
      break;
    case 'Electric':
      pokemon = 'Pikachu';
      break;
    default:
      pokemon = 'Mew';
  }
  return pokemon;
}

console.log(getPokemon('Fire')); // Result: Charmander

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

Так как же избежать использования конструкций switch? Это можно сделать, используя объектные литералы. Объектный литерал прост, его легко писать, читать и поддерживать. Все мы привыкли к управлению объектами в JavaScript, и этот синтаксис намного чище, чем конструкция switch. Вот пример:

const pokemon = {
    Water: 'Squirtle',
    Fire: 'Charmander',
    Plant: 'Bulbasur',
    Electric: 'Pikachu'
  };

function getPokemon(type) {
  return pokemon[type] || 'Mew';
}
console.log(getPokemon('Fire')); // Result: Charmander

// If the type isn't found in the pokemon object, the function will return the default value 'Mew'
console.log(getPokemon('unknown')); // Result: Mew

Как видите, используя оператор ||, мы можем добавить значение по умолчанию. Если тип не будет найден в объекте pokemon, функция getPokemon вернет «Mew» в качестве значения по умолчанию.

Примечание: как вы, наверное, заметили, мы объявляем объект pokemon вне функции, а не внутри. Это делается, чтобы объект не создавался каждый раз при запуске функции.

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

Вот как можно использовать map:

const pokemon = new Map([
    ['Water', 'Squirtle'],
    ['Fire', 'Charmander'],
    ['Plant', 'Bulbasur'],
    ['Electric', 'Pikachu']
  ]);

function getPokemon(type) {
  return pokemon.get(type) || 'Mew';
}

console.log(getPokemon('Fire')); // Result: Charmander
console.log(getPokemon('unknown')); // Result: Mew

Как видите, при замене конструкций switch на объектный литерал или map наш код выглядит гораздо чище.

Делайте ваши условия наглядными

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

Взгляните на следующую конструкцию:

function checkGameStatus() {
  if (
    remaining === 0 ||
    (remaining === 1 && remainingPlayers === 1) ||
    remainingPlayers === 0
  ) {
    quitGame();
  }
}

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

Как нам улучшить наше условие? Путем выделения его в функцию. Вот так:

function isGameLost() {
  return (
    remaining === 0 ||
    (remaining === 1 && remainingPlayers === 1) ||
    remainingPlayers === 0
  );
}

// Our function is now much easier to understand:
function checkGameStatus() {
  if (isGameLost()) {
    quitGame();
  }
}

Теперь, когда мы выделили условие в функцию с описательным именем isGameLost(), наша функция checkGameStatus стала понятной с первого взгляда. Почему? Потому что наш код информативный, он сообщает нам, что в нем происходит, а это именно то, к чему мы всегда должны стремиться.

Заменяйте вложенные условные операторы граничными операторами

Вложенные if-конструкции это одна из наихудших вещей, с которыми можно столкнуться в коде. Мне попадались if-ы, идущие на 10 уровней вглубь… Поверьте мне на слово: попытки разобраться в таком коде это просто кошмар. Вот пример вложенных условий if (только на три уровня в глубину, я ж не монстр):

function writeTweet() {
  const tweet = writeSomething();

  if (isLoggedIn()) {
    if (tweet) {
      if (isTweetDoubleChecked()) {
        tweetIt();
      } else {
        throw new Error('Dont publish without double checking your tweet');
      }
    } else {
      throw new Error("Your tweet is empty, can't publish it");
    }
  } else {
    throw new Error('You need to log in before tweeting');
  }
}

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

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

function writeTweet() {
  const tweet = writeSomething();

  if (!isLoggedIn()) {
    throw new Error('You need to log in before tweeting');
  }
  if (!tweet) {
    throw new Error("Your tweet is empty, can't publish it");
  }
  if (!isTweetDoubleChecked()) {
    throw new Error('Dont publish without double checking your tweet');
  }

  tweetIt();
}

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

Избегайте дублирования кода

Дублирование кода никогда не приводит ни к чему хорошему. Из-за него вы получаете ситуации типа «Я уже исправлял этот баг, но забыл исправить еще и здесь» или «Мне нужно внести изменения в фичу (или добавить ее), причем сделать это придется в пяти разных местах». Как говорит нам принцип DRY,

«Каждая часть знания должна иметь единственное, непротиворечивое и авторитетное представление в рамках системы».

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

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

function getJavascriptNews() {
    const allNews = getNewsFromWeb();
    const news = [];
 
    for (let i = allNews.length - 1; i >= 0; i--){
        if (allNews[i].type === "javascript") {
            news.push(allNews[i]);
        }
    }
 
    return news;
}
 
function getRustNews() {
    const allNews = getNewsFromWeb();
    const news = [];
 
    for (let i = allNews.length - 1; i >= 0; i--){
        if (allNews[i].type === "rust") {
            news.push(allNews[i]);
        }
    }
 
    return news;
}

function getGolangNews() {
  const news = [];
  const allNews = getNewsFromWeb();

  for (let i = allNews.length - 1; i >= 0; i--) {
    if (allNews[i].type === 'golang') {
      news.push(allNews[i]);
    }
  }

  return news;
}

Вы, вероятно, заметили, что цикл for совершенно одинаков для всех трех функций, за исключением одной маленькой детали: типа новостей, которые нам нужны (в первом случае это новости JavaScript, во втором — Rust, в третьем — Golang). Чтобы избежать дублирования, мы можем выделить цикл for в функцию, которую сможем затем вызывать из функций getJavascriptNews, getRustNews и getGolangNews. Вот так:

function getJavascriptNews() {
  const allNews = getNewsFromWeb();
  return getNewsContent(allNews, 'javascript');
}

function getRustNews() {
  const allNews = getNewsFromWeb();
  return getNewsContent(allNews, 'rust');
}

function getGolangNews() {
  const allNews = getNewsFromWeb();
  return getNewsContent(allNews, 'golang');
}

function getNewsContent(newsList, type) {
  const news = [];
  for (let i = newsList.length - 1; i >= 0; i--) {
    if (newsList[i].type === type) {
      news.push(newsList[i].content);
    }
  }
  return news;
}

После выделения цикла for в функцию getNewsContent наши функции getJavascriptNews, getRustNews и getGolangNews превратились в простые и понятные однострочники.

Дальнейший рефакторинг

Вы наверняка заметили, что наши три функции совершенно одинаковы, за исключением типа строки, которую мы передаем в функцию getNewsContent. Такое бывает при рефакторинге. Зачастую (и чаще да, чем нет) одно изменение влечет за собой другое и так далее, аж пока код не уменьшится вдвое по сравнению с исходным. Позвольте вашему коду рассказать вам, в чем он нуждается:

function getNews(type) {
  const allNews = getNewsFromWeb();
  return getNewsContent(allNews, type);
}

function getNewsContent(newsList, type) {
  const news = [];
  for (let i = newsList.length - 1; i >= 0; i--) {
    if (newsList[i].type === type) {
      news.push(newsList[i].content);
    }
  }
  return news;
}

Куда делись наши getJavascriptNews, getRustNews и getGolangNews? Мы заменили их функцией getNews, получающей вид новостей в качестве аргумента. Таким образом, не имеет значения, сколько еще видов новостей мы добавим, — мы в любом случае сможем пользоваться этой функцией. Это называется абстракцией и позволяет нам использовать функции повторно, а это чрезвычайно полезно. Абстрагирование это один из подходов, которые я чаще всего применяю в моем коде.

Бонус: сделайте цикл for более читаемым при помощи свойств ES6

Это последнее изменение, клянусь!

Циклы for не отличаются совершенной читабельностью. Но после введения функций массивов в ES6 мы можем в 95% случаев избежать использования циклов for. В нашем случае мы можем заменить исходный цикл при помощи Array.filter в комбинации с Array.map:

function getNews(type) {
  const allNews = getNewsFromWeb();
  return getNewsContent(allNews, type);
}

function getNewsContent(newsList, type) {
  return newsList
    .filter(newsItem => newsItem.type === type)
    .map(newsItem => newsItem.content);
}
  • При помощи Array.filter мы возвращаем только те элементы, чьи типы соответствуют типу, переданному в качестве аргумента.
  • При помощи Array.map мы возвращаем только свойство content объекта Item, а не весь Item.

Поздравляю! После внесения трех небольших изменений наши исходные три функции сократились до двух, более понятных и простых в поддержке. Также, благодаря абстракции, мы сделали функцию getNews пригодной для многократного использования.

Функции должны делать только что-то одно

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

function startProgram() {
  if (!window.indexedDB) {
    window.alert("Browser not support indexeDB");
  } else {
    let openRequest = indexedDB.open("store", 1);
 
    openRequest.onupgradeneeded = () => {};
 
    openRequest.onerror = () => {
      console.error("Error", openRequest.error);
    };
 
    openRequest.onsuccess = () => {
      let db = openRequest.result;
    };
 
    document.getElementById('stat-op').addEventListener('click', () => {});
    document.getElementById('pre2456').addEventListener('click', () => {});
    document.getElementById('cpTagList100').addEventListener('change', () => {});
    document.getElementById('cpTagList101').addEventListener('click', () => {});
    document.getElementById('gototop').addEventListener('click', () => {});
    document.getElementById('asp10').addEventListener('click', () => {});
 
    fetch("employeList.json")
      .then(res => res.json())
      .then(employes => {
        document.getElementById("employesSelect").innerHTML = employes.reduce(
          (content, employe) => employe.name + "<br>",
          ""
        );
      });
 
    document.getElementById("usernameLoged").innerHTML = `Welcome, ${username}`;
  }
}

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

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

function startProgram() {
  if (!window.indexedDB) {
    throw new Error("Browser doesn't support indexedDB");
  }

  initDatabase();
  setListeners();
  printEmployeeList();
}

function initDatabase() {
  let openRequest = indexedDB.open('store', 1);

  openRequest.onerror = () => {
    console.error('Error', openRequest.error);
  };
  openRequest.onsuccess = () => {
    let db = openRequest.result;
  };
}

function setListeners() {
  document.getElementById('stat-op').addEventListener('click', () => {});
  document.getElementById('pre2456').addEventListener('click', () => {});
  document.getElementById('cpTagList100').addEventListener('change', () => {});
  document.getElementById('cpTagList101').addEventListener('click', () => {});
  document.getElementById('gototop').addEventListener('click', () => {});
  document.getElementById('asp10').addEventListener('click', () => {});
}

async function printEmployeeList() {
  const employees = await getEmployeeList();

  document.getElementById('employeeSelect').innerHTML = formatEmployeeList(employees);
}

function formatEmployeeList(employees) {
  return employees.reduce(
    (content, employee) => content + employee.name + '<br>',
    ''
  );
}

function getEmployeeList() {
  return fetch('employeeList.json').then(res => res.json());
}

Давайте пройдемся по внесенным изменениям.

Для начала, мы избавились от блока if else при помощи граничного оператора. Затем мы выделили логику, необходимую для старта базы данных, в функцию initDatabase, а логику добавления прослушивателей событий — в функцию setListeners.

Логика вывода списка сотрудников немного более сложная, так что мы создали три функции: printEmployeeList, formatEmployeeList и getEmployeeList.

Функция getEmployeeList ответственна за осуществление запроса GET к employeeList.json и возврат ответа в формате json.

Затем он вызывается функцией printEmployeeList, которая принимает список сотрудников и передает его функции formatEmployeeList, которая форматирует и возвращает его. После этого список выводится.

Как видите, каждая из функций ответственна за что-то одно.

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

Заключение

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

Применяя 5 простых подходов, показанных в этом туториале, вы существенно повысите качество своего кода и, опосредованно, вашу продуктивность.

1 КОММЕНТАРИЙ

  1. > Дальнейший рефакторинг

    Плохой пример. Точнее недостаточно разобран. Сейчас функция выполняет несколько действий: 1) получение списка 2) фильтрация данных. Логичнее разделить эту функцию на две, каждая из которых будет выполнять одно действие (получение данных, фильтрация). Потому что нет смысла получать данные в цикле — достаточно получить эти данные один раз.

ОСТАВЬТЕ ОТВЕТ

Please enter your comment!
Please enter your name here