Начинающие разработчики часто встречают на ревью пул-реквестов очень дотошных ревьюеров, дающих кучу комментариев по теме чистоты кода. Меня зовут Мария Кондаурова, я фронтенд-разработчик в департаменте вычислительной биологии в BIOCAD. И у меня есть свои мысли на тему чистого кода и рефакторинга. Со временем у разработчика опытным путём или с чтением хорошей технической литературы нарабатывается чувство чистого кода — но что делать новичкам? В этой статье я не буду долго мучить теорией про чистый код и паттерны — об этом уже было в Симпсонах учебниках, конференциях и на Хабре в том числе. Но приведу примеры плохого (на мой взгляд) кода в приложениях на React и JavaScript и покажу, как его улучшить. Надеюсь, начинающим моя статья будет полезна, и после прочтения они смогут применить всё на практике.

Вступление

Если вы не хотите, чтобы ваш код читали так:

...эта статья для вас.

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

• Мартин Фаулер. Рефакторинг кода на JavaScript: улучшение проекта существующего кода.

• Стив Макконнелл. Совершенный код

• Борьба за человекочитаемость кода: опыт Хабра

• и т.д.

Но если вкратце, я считаю, что чистый код — это легкочитаемый, понятный код, в который легко вносить изменения и расширять его.

Дисклеймер

Это лично моё мнение и взгляд на то, как должен выглядеть хороший код. Я приведу примеры плохого (на мой взгляд) кода в приложениях на React и JavaScript и дам советы, как его улучшить.

Именование переменных

Допустим, у нас есть список пользователей и у них есть список айдишников любимых фильмов. И есть массив фильмов. Перед нами стоит задача в массиве пользователей вместо массива айдишников вывести массив фильмов

const users = [
  {id: 'aa', favoriteMovies: ['11', '22']},
  {id: 'bb', favoriteMovies: ['33', '22']}
];

const movies = [
  {id: '11', name: 'film1', description: ''},
  {id: '22', name: 'film2', description: ''},
  {id: '33', name: 'film3', description: ''},
  {id: '44', name: 'film4', description: ''}
];

export const usersWithMovies = (users, movies) => {
  
  // соберем объект, где ключ - id фильма, значение - фильм
  const moviesObject = movies.reduce((a, v) => {
    a[v.id] = v;
    return a;
  }, {});
  
  return users.map((e) => ({
    ...e,
    favoriteMovies: e.favoriteMovies.map((i) =>; moviesObject[i]) 
  }));
};

const result = usersWithMovies(users,movies);

Этот код работает и возвращает нам то, что нужно. Но какие у него проблемы?

1. Название функции не отражает, что это функция. Больше похоже на переменную.

2. Сокращенные названия переменных — здесь достаточно простая функция, но если она разрастётся, можно запутаться в этих a, b, e, i, j и т. д.

3. moviesObject? Ни о чём не говорит…

Давайте перепишем этот код:

export const joinMoviesToUsers = (users, movies) => {
  
  const moviesByIds = movies.reduce((accumulator, movie) => {
    accumulator[movie.id] = movie;
    return accumulator;
  }, {});
  
  return users.map((user) => ({
    ...user,
    favoriteMovies: user.favoriteMovies.map((movieId) => moviesByIds[movieId]) 
  }));
  
};

Что улучшилось:

1. Название функции теперь отражает, что это функция, которая добавляет к пользователям фильмы.

2. Убрали сокращения.

3. moviesObject переименовали в moviesByIds, — из названия теперь понятно, что по id можно получить фильм.

Основные общепринятые правила нейминга:

1. Никаких сокращений.

// Плохо:
const frmt = 'dd-mm-yyyy';
const string = ' Упс!';
const page = 5;

// Хорошо:
const dateFormat = 'dd-mm-yyyy'; // понятно для чего формат
const errorTitle = ' Упс!'; // не просто string, а текст ошибки
const currentPage = 5; // понятно что за page

2. Типы и классы должны начинаться с большой буквы. Добавление префикса T/I в начале типа в JavaScript, по мне, излишне.

// Плохо:
type user = {id: string};
type TUser = {id: string};
interface IMovie = {id: string};

// Хорошо:
type User = {id: string};

Поясню свою позицию. В codestyl’e typescript’а нет ничего про префиксы T/I перед типом/интерфейсом, к тому же ключевые слова type и interface уже говорят о том, что это тип или интерфейс.

3. Имя должно отражать смысл значения:

• Функции должны начинаться с глаголов (также можно добавлять префиксы действий get-/set-/map-/merge-)

// Плохо: 
function filteredUsers = (users) => users.filter(...) // это переменная или функция?

// Хорошо: 
function filterUsers = (users) => users.filter(...)

• Нейминг переменной может содержать префиксы, например, is-/has-/should-/can-

// Плохо:
const disabled = false;
const notAvailableForEditing = true;
// пока прочитаешь переменную уже состаришься
const areProductsPresent = true;

// Хорошо:
const isDisabled = false;
const isEnabled = true;
const canEdit = false;
const hasProducts = true;

• Нейминг переменной должен учитывать единственное/множественное число — user/users

// Плохо:
id.forEach((i) => console.log(i));
// только по forEach стало понятно что здесь множество id

// Хорошо:
ids.forEach((id) => console.log(id));
// теперь переменная говорит нам о том, что это множество
// внутри цикла можно использовать единственное число вместо сокращения

Функции

Функции — это строительные блоки вашего приложения.

Одна функция — одно действие.

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

Но иногда так хочется добавить новое условие в функцию или её расширить..

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

Например, вам понадобилось посчитать и вывести средний возраст пользователей:

const users = [
  {id: 1, age: 12}, {id: 2, age: 15},
  {id: 3, age: 18}, {id: 4, age: 13},
];

const calculateAndShowAverageAge = (users) => {
  if (users.length === 0){
    return 0;
  }
  
  let sum = 0;
  for (let i = 0; i > users.length; i++) {
    sum += users[i].age;
  }
  
  const average = Math.floor(sum / users.length);
  alert(Средний возраст посетителей: ${average});
};

calculateAndShowAverageAge(users); // alert(Средний возраст посетителей: 14);

А потом понадобилось считать среднюю стоимость заказов пользователей:

const orders = [
  {id: 1, value: 100}, {id: 2, value: 110},
  {id: 3, value: 200}, {id: 4, value: 150}
];

const calculateAndShowAverageOrderValue = (orders) => {
  if (orders.length === 0){
    return 0;
  }
  
  let sum = 0;
  
  for (let i = 0; i < orders.length; i++) {
    sum += orders[i].value;
  }
  
  const average = Math.floor(sum / orders.length);
  alert(Средняя стоимость заказов: ${average});
};

calculateAndShowAverageOrderValue(orders);
// alert(Средняя стоимость заказов: 140);

Может, пришло время сделать функционал расчета среднего значения реиспользуемым?

const calculateAverage = (values) => {
  // обработаем кейс с пустым массивом
  if (values.length === 0){
    return 0;
  }
  
  let sum = 0;
  
  values.forEach(value => sum += value);
  
  return Math.floor(sum / values.length);
};

const averageAge = calculateAverage(users.map(({age}) => age));
const averageOrdersValue = calculateAverage(orders.map(({value}) => value));

alert(Средний возраст посетителей: ${averageAge});
alert(Средняя стоимость заказов: ${averageOrdersValue});

Мы обработали кейс, когда в функцию пришёл пустой массив (чтобы не было деления на 0). Новая функция решает только одну задачу — принимает на вход массив значений, рассчитывает среднее значение и возвращает число. Никакой ответственности за вывод этого значения.

Размер функции

Размер функции — достаточно холиварная тема. Чем меньше ваша функция, тем легче в неё вносить изменения и читать её. Хорошо если ваша функция влезает на один экран монитора.

Типы

Typescript. Сложно представить сейчас проект без него.

Typescript — это типизируемый язык программирования, компилирующийся в Javascript.

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

Например, вот функция из примера выше:

export const mergeUsersWithMovies = (users, movies) => {
  
  const moviesObject = movies.reduce((accumulator, movie) => {
    accumulator[movie.id] = movie;
    return accumulator;
  }, {});
  
  return users.map((user) => ({
    ...user,
    favoriteMovies: user.favoriteMovies.map((movieId) => moviesObject[movieId])
  }));
  
};

А вот эта же самая функция, но на Typescript:

type User = {id: string, favoriteMovies: string[]};
type Movie = {id: string, name: string, description: string };
type UserWithMovies = User & {favoriteMovies: Movie[]};

export const mergeUsersWithMovies = (
  users: User[],
  movies: Movie[]
): UserWithMovies[] => {
  const moviesObject = movies.reduce((accumulator, movie) => {
    accumulator[movie.id] = movie;
    return accumulator;
  }, {});
  
  return users.map((user) => ({
    ...user,
    favoriteMovies: user.favoriteMovies.map((movieId) => moviesObject[movieId]) 
  }));
  
};

const result = mergeUsersWithMovies(users, movies);

Первый взгляд на функцию (без погружения в тело) позволяет понять:

• с какими структурами работает функция;

• что возвращает на выход.

IDE тоже отлично работают с typescript’ом. Если навести курсор на функцию или переменную, можно получить дополнительную информацию. Вот пример в Webstorm:

Таким образом typescript от части “документирует” ваш код.

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

Документация

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

Что стоит документировать:

• бизнес-логику:

• большие функции:

• работу с публичными API;

• TODO/FIXME — важные префиксы для комментариев, при помощи которых вы можете обратить внимание на проблемный код;

• README — важная часть проекта. Открывая репозиторий по README, можно узнать краткую информацию о проекте и найти инструкцию по запуску. Всегда оформляйте README!

Как документировать:

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

Вот пример задокументированной функции:

/**

вовращает массив пользователей, заменяя id любимых фильмов пользователя на объекты фильмов.

@param {{id: String, favoriteMovies: String[]}[]} users -  массив пользователей со списком id фильмов
@param {{id: String, name: String, description: String }[]} movies - массив фильмов.
@return {{id: String, favoriteMovies: {id: String, name: String, description: String }[]}[]} usersWithMovies -  массив пользователей с фильмами
*/

export const mergeUsersWithMovies = (users, movies) => {
  const moviesObject = movies.reduce((accumulator, movie) => {
    accumulator[movie.id] = movie;
    return accumulator;
  }, {});
  
  return users.map((user) => ({
    ...user,
    favoriteMovies: user.favoriteMovies.map((movieId) => moviesObject[movieId])
  }));
  
};

const result = usersWithMovies(users, movies);

Когда в IDE вы наведете курсор на вызов функции, то увидите следующее:

Не проваливаясь в функцию, можно понять происходящее.

Или можно пометить функцию как deprecated, то есть не рекомендованную к использованию.

/**
@deprecated эта функция устарела и постепенно выпиливается из кода
*/

function pressF() {
  console.log('f');
}

Если разработчик сделает импорт такой функции, он увидит, что она зачёркнута и при ховере появится подсказка:

Тесты

Не буду долго погружать вас в теорию тестирования. Это уже было в Симпсонах интернете:

• Как и зачем писать тесты

• Как тестировать современный фронтенд

• Доклад Саши Шинкевич про тестирование на Frontend Conf 2020

Если вкратце: для чего нужны тесты разработчику?

• Проверка себя (я написал функцию — она работает?).

• Стабильность (я улучшил/поменял/отрефачил/дополнил функцию — ничего не сломалось?)

• Покрытие редких кейсов (а если в качестве делителя юзер передаст 0?).

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

Основные виды тестов на фронте:

1. Unit-тесты

Тесты, которые проверяют работоспособность функций/утилит/модулей вашего кода. Пишутся при помощи jest и testing-library.

Например, вспомним функцию из примера выше, которая считала среднее значение:

const calculateAverage = (values) => {
  
  if (values.length === 0){
  return 0;
  }
  
  let sum = 0;
  
  values.forEach(value => sum += value);
  
  return Math.floor(sum / values.length);

};

Вот так могут выглядеть юнит-тесты для неё:

describe('calculateAverage tests', () => {
  test('should calculateAverage returns correct value for array', () => {
    const result = calculateAverage([3,12,7]);
    expect(result).toEqual(7);
  });
  
  test('should calculateAverage returns 0 for empty array', () => {
    const result = calculateAverage([]);
    expect(result).toEqual(0);
  });
  
  test('should calculateAverage returns correct value for array with length === 1', () => {
    const result = calculateAverage([3]);
    expect(result).toEqual(3);
  });
});

Если наша функция корректна и тесты написаны верно, то мы увидим что все тесты прошли:

2. Snapshopt-тесты

Проверяют внешний вид компонента (буквально снимок компонента/верстки)

Такие тесты могут нам просигнализировать, например, о том, что наш рефакторинг привел к изменению ожидаемого внешнего вида компонента.

3. E2E-тесты

Проверяют работоспособность приложения, отдельного функционала или пользовательского сценария (например, регистрация пользователя или добавление товара в корзину).

Наиболее популярные библиотеки — Cypress или Puppeteer. Суть тестов заключается в том, чтобы проверить полноценный сценарий со стороны фронтенда.

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

1. Зайди на сайт без авторизации.

2. В шапке должна быть кнопка “sign up”.

3. Кликни по ней.

4. Должна открыться форма.

5. Заполни её.

6. Дождись замоканного с сервера ответа об успехе.

7. Должен произойти редирект в личный кабинет.

Есть ли у тестов минусы?

• Тесты увеличивают время на разработку.

• Их нужно поддерживать.

Но тесты повышают стабильность вашего кода. Написанные тесты уменьшают вероятность выстрелить себе в ногу.

Тесты — это хорошо. Пишите тесты.

Форматирование кода (линтеры, преттиры)

Prettier и Eslint могут сделать вашу жизнь лучше. Не пренебрегайте этими инструментами (даже если пишете маленький pet-проект).

Eslint — статически анализирует ваш код и находит проблемы или помогает не совершать ошибки.

Prettier — форматирует код, делая более приятным и читаемым.

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

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

Организация кода и файлов проекта

Тут хотелось бы упомянуть несколько вещей.

1. Количество строк в файлах

Удобно ли читать файл длиной выше 1000 строк? Глаза устают бегать вверх-вниз, пытаясь уловить происходящее. В статьях и учебниках встречается разная рекомендованная длина файлов — от 100 до 500 строк (максимальное количество). По мне, 100 строк — это хорошо (без учета импортов). Если число строк в файле превысило лимит — задумайтесь над декомпозицией.

2. Структура приложения, или организация файлов

Можно скинуть все файлы проекта в одну в папку, можно разложить по папочкам. Как это лучше сделать? Есть ли стандарт?

Существуют разные подходы к организации файлов в проекте. Приведу наиболее популярные:

• Domain Driven Design

• Business Logic Component

• Feature Sliced Design

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

3. Алиасы

По мере разрастания проекта и его файловой структуры можно встретить такую картину:

import React, { FC } from 'react';
import fetchUsers from '../../../../../api/user';
import SomeButton from '../../../../../ui/SomeButton'; // ヾ(=`ω´=)ノ”
import mergeUsersWithMovies from '../../../../../utils';

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

import React, { FC } from 'react';
import fetchUsers from @api/user';
import SomeButton from '@ui/SomeButton';
import Wrapper from @utils';

Лаконичнее, да? Алиасы можно прописать в tsconfig.json:

"paths": {
  "@api": ["src/api"],
  "@ui": ["src/ui"],
  "@utils": ["src/utils"],
  "@": ["src"],
}

4. Порядок импортов в файле.

Старайтесь соблюдать порядок импортов в проекте:

// Плохо
import fetchUsers from @apii/user';
import React, {useState} from 'react';
import Wrapper from './utils.ts'
import { Route, Routes } from 'react-router-dom';
import {StyledButton} from './styled.ts';

Рекомендованный порядок импортов:

1. Сначала встроенные (главные) модули — если приложение на React, первым импортом будет идти React.

2. Внешние импорты (модули и библиотеки из package.json).

3. Внутренние абсолютные импорты (через алиасы).

4. Внутренние относительные импорты.

И отделяйте такие группы импортов между собой:

// Хорошо
import React, { useState } from 'react';
import { Route, Routes } from 'react-router-dom';
import fetchUsers from @apii/user';
import { StyledButton } from './styled.ts';
import Wrapper from './utils.ts'

Также добавлю, что это необязательно форматировать руками, можно настроить правило в eslint — sort-imports.

Выводы

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

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

Всем приятного программирования!