Хороший код, но плохая архитектура

в 8:12, , рубрики: frontend-разработка, react.js, архитектура приложений, архитектурные паттерны

Представьте ситуацию: вы открываете какой-нибудь файл в проекте, которому уже года три (чаще всего это форма). И в голове всплывает мысль:  «А что тут вообще происходит?». Причём не в смысле «кто виноват», а в смысле «как мы вообще сюда пришли?».

Я фронтенд-разработчица, и за шесть лет в разработке такое случалось почти на каждом проекте, где мне удалось побывать. Я хочу сразу оговориться: это не статья из серии «как надо жить». Это скорее разговор по типу «смотрите, как все мы иногда делаем и даже не замечаем».

Это я смотрю на свой код спустя 2 года
Это я смотрю на свой код спустя 2 года

Сначала немного контекста

Современное фронтенд-приложение - это уже мини-бэкенд в браузере: состояние, формы, бизнес-правила, аналитика, безопасность и фичи, которые живут годами. А по правде говоря, иногда фронтенд даже сложнее, чем бэкенд. Мы давно не рисуем просто кнопочки, мы управляем системой, которая живёт в браузере и ведёт себя как самостоятельное приложение.

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

Вряд ли в какой-то момент пришёл лид и сказал: «Так, ребята, давайте сегодня сделаем плохую архитектуру». Обычно этот путь проходит тихо и с большим количеством красивых PR.

Вообще, сразу поделюсь своим главным душным тезисом:

Самая опасная архитектурная ошибка — это хорошее решение, принятое слишком рано. 

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

Откуда вообще берётся сложность

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

Сложность на фронте чаще появляется не из-за недостатка знаний, а из-за избытка старания.

Давайте признаемся честно, никто из нас не пишет плохой код (намеренно!). Мы пишем код, который кажется разумным в моменте. И иногда выходит, что мы делаем правильно, но…слишком заранее.

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

Ловушка №1: преждевременная абстракция

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

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

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

А самое подлое то, что каждый шаг был разумным. Никто не делал фигню. Но в итоге мы получили сложность, которую уже невозможно откатить потому что «от него всё зависит».

Здесь хочется задать вопрос, и я предлагаю ответить на него честно: 

Сколько раз вы делали «на будущее» и это ожидаемое будущее реально наступало?

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

Когда абстракция действительно нужна

Чтобы это не звучало как «никогда ничего не обобщайте», я вывела для себя простой критерий. Абстракция окупается, когда выполняются три условия:

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

  2. Эти кейсы стабильны и не меняются каждую неделю.

  3. Отличия между ними понятны.

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

Дублирование почти всегда дешевле неправильной абстракции.

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

Ловушка №2: компоненты, которые знают слишком много

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

Я думаю каждый знает, кто частая жертва такого подхода. Госпожа Форма. Форма начинает жизнь как «UI и submit». А потом туда добавляются:

  • валидация,

  • подготовка payload,

  • запрос,

  • нотификации,

  • работа с модалкой,

  • навигация,

  • условия «если роль такая, то покажи или спрячь»,

  • и вот этот любимый пункт: «если выбрали A, то сбрось пять полей».

И в какой-то момент ты открываешь файл и понимаешь, что тебе страшно его менять.

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

Внимание, сейчас будет кульминация!

Код становится сложным в тот момент, когда стираются границы ответственности.

Да-да, это тот самый принцип Single Responsibility из SOLID. Просто во фронте он ломается особенно тихо. На практике это выглядит так:

  • компонент начинает знать про API,

  • хук начинает знать про UI,

  • слой данных начинает знать про поведение экрана.

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

Маленький пример:

export const createUserThunk = createAsyncThunk(
  'users/create',
  async (data, { dispatch }) => {
    const response = await api.createUser(data);

    dispatch(closeModal());
    dispatch(showToast('User created'));

    return response.data;
  }
);

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

Эволюция одной формы

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

Версия 1: «Всё в одном»

export const CreateUserForm = () => {
  const dispatch = useDispatch();
  const [name, setName] = useState('');
  const [email, setEmail] = useState('');
  const [error, setError] = useState(null);

  const handleSubmit = async () => {
    if (!name || !email) {
      setError('Fill all fields');
      return;
    }

    try {
      const response = await api.createUser({ name, email });
      dispatch(addUser(response.data));
      dispatch(closeModal());
    } catch (e) {
      setError('Something went wrong');
    }
  };

  return (
    <form>
      <input value={name} onChange={e => setName(e.target.value)} />
      <input value={email} onChange={e => setEmail(e.target.value)} />
      {error && <span>{error}</span>}
      <button onClick={handleSubmit}>Create</button>
    </form>
  );
};

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

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

Версия 2: «Нормально, даже красиво»

export const useCreateUser = () => {
  const dispatch = useDispatch();
  const [error, setError] = useState(null);

  const createUser = async (data) => {
    try {
      const response = await api.createUser(data);
      dispatch(addUser(response.data));
      dispatch(closeModal());
    } catch {
      setError('Something went wrong');
    }
  };

  return { createUser, error };
};

export const CreateUserForm = () => {
  const { createUser, error } = useCreateUser();
  const [name, setName] = useState('');
  const [email, setEmail] = useState('');

  return (
    <form onSubmit={() => createUser({ name, email })}>
      ...
    </form>
  );
};

Вот это уже похоже на то, что многие из нас пишут. Компонент становится почти чистым: хранит только name и email, вызывает createUser, показывает error. Логика уехала в хук. Всё аккуратно, всё читаемо.

Но давайте подумаем: «Красивый код» и «простая архитектура» это одно и тоже? 

Надеюсь, что вы ответили «Нет» (это правильный ответ!)

Посмотрите, что осталось внутри хука: сайд-эффекты, знание про Redux, знание про закрытие модалки, привязка к конкретному сценарию. Такой хук сложно тестировать изолированно, так как он завязан на Redux. Его сложно переиспользовать, ведь он знает про конкретный UI-сценарий. И он становится точкой, через которую проходят и данные и сайд-эффекты.

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

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

Версия 3: границы по уровням

export const createUser = (data) => api.createUser(data);

// thunk / RTK Query
export const createUserThunk = createAsyncThunk(
  'users/create',
  async (data) => {
    const response = await createUser(data);
    return response.data;
  }
);

// компонент
export const CreateUserForm = () => {
  const dispatch = useDispatch();

  const handleSubmit = (data) => {
    dispatch(createUserThunk(data));
  };

  return <Form onSubmit={handleSubmit} />;
};

Здесь мы делаем простую вещь — разделяем уровни ответственности. Не прячем всё в один хук, а раскладываем по слоям (главное не лениться это делать):

  • API-функция не знает про Redux. Её можно дёрнуть откуда угодно.

  • thunk/RTK Query не знает про модалку, тосты и навигацию. Он знает только про данные.

  • Компонент единственный, кто знает сценарий экрана. Закрытие модалки, тосты, навигация живут здесь, по fulfilled.

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

Конечно, такой подход не серебряная пуля и Thunk тоже можно превратить в монстра. Но здесь хотя бы разделены уровни ответственности, а это уже снижает связность. А ещё отсюда следует простой принцип: UI-эффекты должны жить рядом с UI.

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

Ускорители сложности

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

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

Бизнес-правила, размазанные по JSX

Сначала одно условие в JSX. Потом второе. Потом десятое. И в какой-то момент никто в команде не может ответить: «По каким правилам эта форма вообще работает?».

{mode !== 'view' && isVip && canEdit && (
  <Controller name="vipComment" control={control} render={({ field }) => <Input {...field} />} />
)}

<Button disabled={!isDirty || isSubmitting || (type === 'A' && !hasDocs) || !canEdit}>
  Сохранить
</Button>

Формально тут всё просто, — это обычное логическое И: не view-режим, клиент VIP, есть право редактировать. Но это уже полноценное бизнес-правило: в каких режимах и при каких ролях поле вообще существует. Чтобы понять форму, ты начинаешь читать JSX как спецификацию. Но эта спецификация разорвана на куски: одно правило тут, второе ниже, третье в disabled кнопки.

Вообще это частая история, когда правила лежат кусками: в JSX, в хелперах, в эффекте, в yup-схеме, в маппере payload.

«Вынесением в хелперы» это не лечится (так как это просто перекладывание мозаики в другую папку). Здесь можно, например, создать одну точку правды policy:

export function buildDealFormPolicy(ctx: DealFormCtx) {
  return {
    fields: {
      vipComment: { visible: ctx.isVip && ctx.mode !== 'view' && ctx.canEdit },
    },
    submit: {
      disabled:
        !ctx.isDirty ||
        ctx.isSubmitting ||
        (ctx.type === 'A' && !ctx.hasDocs) ||
        !ctx.canEdit,
    },
  };
}

const policy = buildDealFormPolicy(ctx);

{policy.fields.vipComment.visible && <VipCommentField />}

<Button disabled={policy.submit.disabled}>Сохранить</Button>

Кажется избыточным и сложным? Но это не «сложная архитектура». Просто один объект, где видно все правила целиком. Хочешь понять правила — открываешь policy, а если хочешь понять разметку — открываешь JSX. И не нужно собирать пазл из десяти файлов.

Boolean-prop hell

Есть ещё одна вещь, которая незаметно размывает архитектуру, это когда компонент начинает принимать всё больше флагов:

<Card
  isEditable
  showFooter
  withActions
/>

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

И начинается самое интересное: нужен ли withActions, если showFooter = false? Подразумевает ли isEditable наличие withActions? В компоненте почти неизбежно появляется внутреннее ветвление, скрытое от потребителя.

Как альтернативу можно использовать композицию. Вместо <Card isEditable showFooter /> мы собираем структуру явно:

// просто контент
<Card>
  <CardContent />
</Card>

// добавился футер и это видно прямо из JSX
<Card>
  <CardContent />
  <CardFooter />
</Card>

// действия живут внутри футера
<Card>
  <CardContent />
  <CardFooter>
    <SaveAction />
  </CardFooter>
</Card>

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

Но это тоже не универсальное решение всех проблем. Любой паттерн, применённый слишком рано, начнёт работать против нас. Если у компонента реально 1–2 состояния и они стабильны, то флаги могут быть нормой.

Моя мысль не о том, что флаги это по дефолту плохо. Я хочу сказать другое: когда вы чувствуете, что добавляете уже третий-четвёртый boolean, стоит остановиться и спросить себя: мы сейчас делаем универсальность или размываем границы?

useEffect как контроллер приложения

Если у вас цепочки «A → эффект → B → эффект → C → грузим D», поведение стало неявным.

useEffect задумывался как синхронизация с внешним миром, а не как способ вычислять состояние. Простой тест: «Если я удалю этот useEffect, перестанет ли что-то работать вне компонента?». Если ответ: «Запрос не уйдёт» или «Подписка не создастся», то эффект оправдан. А если «Не обновится состояние», то эффекту здесь не место.

Мемоизация как религия

useMemo и useCallback в большинстве UI-кода не решают реальных проблем, зато добавляют когнитивную нагрузку. Оптимизация без замеров почти всегда делает поддержку хуже. С приходом React Compiler многие вещи начинают оптимизироваться под капотом. И это значит, что мемоизация снова становится тем, чем должна быть: инструментом, а не базовым синтаксисом.

Как сложность рождается незаметно

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

const { disabled, disableReason, canView } = useTariffDisabledCondition({
  tariff,
  disabledProp,
  client,
  statusCode,
});

Когда смотришь на использование, всё выглядит отлично. Читается понятно: хук, который говорит, доступен ли тариф и почему. В целом, очень удобно.

Но давайте посмотрим, что внутри. Не буду показывать весь код: там два RTK Query со skip и selectFromResult и хелпер на 40 строк. Общий силуэт такой:

  • запрос за аккаунтами клиента (условный),

  • запрос в словари за бизнес-правилом «Скрыт ли тариф для смены при таком статусе» (условный),

  • хелпер getTariffVisibilityProps, в который передаётся почти вся вселенная: тариф, аккаунты, сегменты, коды клиента, флаги, результат словаря.

const { data: accounts = [] } = useClientAccountsListQuery(
  { idClient: client?.idClient ?? '' },
  { skip: !client?.idClient },
);

const { isHiddenForChangeTariffByStatus } = useGetRelatedDictionariesQuery(
  {
    relationCode: RELATION_CODE,
    itemCode: tariff?.bcode ?? '',
    reverse: true,
  },
  {
    skip: !tariff || !tariff.replacePermanentDisabled || !statusCode,
    selectFromResult: ({ data }) => ({
      isHiddenForChangeTariffByStatus: Boolean(
        data?.find((el) => el.code === statusCode),
      ),
    }),
  },
);

const disabledConditionResult = getTariffVisibilityProps({
  tariff,
  disabled: disabledProp,
  accounts,
  clientId: client?.idClient,
  tariffSegmentRelations,
  isHiddenForChangeTariffByStatus,
});

return {
  ...disabledConditionResult,
  loading: isFetching,
};

А внутри хелпера нас ждут последовательные if, в которых порядок строк определяет приоритет правил, который нигде явно не описан.

То есть внутри хук делает несколько вещей сразу: ходит в сеть (дважды), знает бизнес-правила, знает связи в словарях, знает про статусы сделки, строит UI-модель. И в итоге мы получили маленький движок правил.

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

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

И снова повторюсь: это не антипример. Это нормальный код, который дожил до возраста, в котором становится видна цена решений, принятых раньше.

Признаки, что сложность вышла из-под контроля

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

  1. Страшно менять.

  2. Чтобы понять поведение нужно открыть много файлов.

  3. Тесты писать сложнее, чем сам код.

  4. Новичок не разбирается за разумное время.

  5. Маленькое изменение вызывает каскад правок.

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

Архитектура хороша не когда красивая, а когда дешёвая в изменении

А что именно делать? Я свела это для себя к одному вопросу. Каждый раз, когда я добавляю абстракцию, спрашиваю себя: (что я вообще делаю?) «Я сейчас уменьшаю стоимость следующего изменения или уменьшаю стоимость этого изменения за счёт всех будущих?» Если второе, то это плохая абстракция, какой бы красивой она ни была.

И ещё одна контринтуитивная мысль, которая мне очень помогает:

Хорошая архитектура — та, которую легко удалить. А не та, которую легко расширить.

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

Отсюда же правило, которое спасает меня от преждевременного выноса:

Код, который меняется вместе, должен лежать вместе.

Когда чешутся руки вынести функцию в utils/, спросите себя: «Эта функция действительно используется где-то ещё? Или я выношу её, потому что "так положено"?»

Если она живёт только в одном экране, пусть лежит рядом с экраном. Появится второй вызов из другого места тогда и переедет.

Вместо заключения: что стареет, а что нет

Напоследок небольшое лирическое отступление про Clean Code.

Я люблю «Чистый код» Мартина, но он писался для мира синхронных операций, процедурной логики и долгоживущих объектов. А наш мир, это мир фронта и React, декларативный, асинхронный, с состоянием, которое живёт отдельно от функций. Если переносить идеи буквально, можно случайно усложнить фронт, пытаясь сделать его «архитектурно правильным».

На фронте читаемость часто важнее архитектурной правильности.

Но что точно не стареет:

  • одна ответственность (особенно для хуков),

  • меньше аргументов (props с десятью флагами — это уже конфиг, случайно ставший JSX),

  • читаемость важнее микрооптимизаций,

  • дублирование дешевле неправильной абстракции.

И последний пункт стоит держать в голове особенно крепко: большинство монстров вроде UniversalModal, SmartFormEngine и SuperFlexibleTable родились ровно из страха дублирования.

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

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

Самый опасный код — это тот, который выглядит разумным.

Если после этой статьи вы хотя бы пару раз поймаете себя на мысли «А это решение нужно прямо сейчас?» значит, она написана не зря.

А как у вас? Узнали свои проекты в этих примерах? Расскажите, какие «ускорители сложности» встречали вы, особенно интересно про хуки, которые незаметно превратились в мини-приложения.


Подписывайтесь на Телеграм-канал Alfa Digital — там рассказываем о работе в IT, делимся новостями, анонсами митапов и квартирников, рассказываем о технологиях, делимся советами наших экспертов, вакансиями и стажировками, иногда шутим.

Читайте также:

Автор: Viktoria_Arturovna

Источник

* - обязательные к заполнению поля


https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js