Перепишіть це негайно!

або покрокова інструкція до того, як врівноважувати здоровий ґлузд та перфекціонізм при рефакторингу

Про мене

  • В розробці з 2011 року
  • Виступаю на конференціях з 2017 року
  • Люблю верстати
  • Вирощую огірки

Дисклеймер

Усі приклади коду в доповіді висмоктані з пальця і можуть не відображати реальних проблем, а самі розглянуті проблеми притягнуті за вуха. Принципи й підходи, розглянуті в доповіді, не є вичерпним списком, а обрані для ілюстрації основних ідей, що я їх намагатимусь вам пояснити з перемінним успіхом. Меми взагалі вставлені аби було, а не тому шо смішно і підходять. Ще мені сказали, шо не можна тепер матюкатись. Срав пес (. І кажуть, що не треба більше зі сцени казати, що я доробляв слайди вночі в готельному номері після препаті. Тому я вам не скажу, де, коли і в якому стані доробляв слайди. Усі думки, висловлені під час доповіді, є виключно моїм субʼєктивним сприйняттям реальності і не можуть розглядатися як абсолютна істина, а навпаки мають бути піддані сумніву в дискусіях після виступу.

Переписувати чи рефакторити?

Переписування Рефакторинг
Усунення технічного боргу раз і назавжди. І створення нового, чого вже. Поступове усунення технічного боргу без breaking changes та з небагато меншими затратами часу на ітерацію.
Використання сучасних технологій. Осучаснення кодової бази відбувається так само ітеративно, без переривання процесу розробки.
Покращення продуктивності та оптимізація архітектури. Покращення продуктивності та оптимізація архітектури. Тільки потроху.

Наша мета — 2002!

  • Підтримуваність та гнучкість коду: Код повинен бути легким для розуміння та готовим до швидких змін
  • Мінімізація технічного боргу: Постійне вдосконалення коду без переривання процесу розробки
  • Масштабованість: Готовність коду до майбутніх змін та вимог без ускладнень

Уявний приклад поганого коду

import React, { useState, useMemo, useCallback } from "react";

const Dashboard = () => {
  const [users, setUsers] = useState([]);
  const [orders, setOrders] = useState([]);
  const [formErrors, setFormErrors] = useState({});
  const currencyFilter = "USD";

  const fetchUsers = async () => {
    const data = await fetch("/api/users").then((res) => res.json());
    setUsers(data);
  };

  const fetchOrders = async () => {
    const data = await fetch("/api/orders").then((res) => res.json());
    setOrders(data);
  };

  const activeUsers = users.filter((user) => user.status === "active");
  const activeOrders = orders.filter(
    (order) => order.status === "active" && order.currency === currencyFilter
  );

  const validateOrder = useCallback((order) => {
    if (order.amount <= 0) {
      setFormErrors((prev) => ({ ...prev, [order.id]: "Invalid amount" }));
    } else {
      setFormErrors((prev) => ({ ...prev, [order.id]: "" }));
    }
  }, []);

  const handleValidation = useCallback(() => {
    activeOrders.forEach(validateOrder);
  }, [activeOrders, validateOrder]);

  const orderSummary = useMemo(
    () =>
      activeOrders.length > 0
        ? activeOrders.length > 10
          ? activeOrders.every((order) => order.priority === "high")
            ? activeOrders.some((order) => order.currency === "USD")
              ? "Many high-priority USD orders"
              : "Many high-priority non-USD orders"
            : "Many orders with mixed priorities"
          : activeOrders.length > 5
          ? "Few active orders"
          : "Very few orders"
        : "No active orders",
    [activeOrders]
  );

  return (
    <div>
      <button onClick={fetchUsers}>Load Users</button>
      <button onClick={fetchOrders}>Load Orders</button>
      <button onClick={handleValidation}>Validate Orders</button>

      <h2>Users</h2>
      <ul>
        {activeUsers.map((user) => (
          <li key={user.id}>{user.name}</li>
        ))}
      </ul>

      <h2>Orders</h2>
      <ul>
        {activeOrders.map((order) => (
          <li key={order.id}>
            {order.product} - {order.currency}
            {formErrors[order.id] && <span> - {formErrors[order.id]}</span>}
          </li>
        ))}
      </ul>

      <p>{orderSummary}</p>
    </div>
  );
};

export default Dashboard;

Я два рази
не повторюю
не повторюю

DRY — Don’t Repeat Yourself

Поганий код

const Dashboard = () => {
  …
  const activeUsers = users
    .filter(user => user.status === 'active');
  const activeOrders = orders
    .filter(order => order.status === 'active');
  …
}
  • Проблема підтримки: Дублювання коду ускладнює його підтримку, адже будь-які зміни в логіці потрібно повторювати у кількох місцях, що збільшує ризик помилок
  • Зниження читабельності: Повторювана логіка робить код менш прозорим і важким для розуміння, що ускладнює роботу з ним іншим розробникам
  • Складність масштабування: У разі зміни вимог або додавання нових функцій, необхідно оновлювати кожен дублікат коду, що сповільнює процес розробки та збільшує ризик помилок

Хороший(?) код

function useFilterData(data, status) {
  return data.filter((item) => item.status === status);
}

const activeUsers = useFilterData(users, "active");
const activeOrders = useFilterData(orders, "active");
  • Не повторюємся: Уникли дублювання логіки, оскільки фільтрація даних тепер відбувається через одну функцію, яку легко перевикористовувати
  • Легка підтримка: Всі зміни робляться в одному місці, що спрощує обслуговування коду і знижує ризик помилок
  • Гнучкість до змін: Нові умови фільтрації або набори даних легко додати без зміни основної структури коду
const useFilterData = (data, type, status, currency) => {
  if (type === 'order') {
    data = data.map(order => ({
      ...order,
      currency: order.currency === 'USD' ? 'US Dollar' : order.currency,
    }));
  }

  return data.filter(item => {
    if (type === 'order') {
      return item.status === status && item.currency === currency;
    }
    return item.status === status;
  });
};
…
const curr = 'US Dollar';
const activeUsers = useFilter(users, 'user', 'active');
const activeOrders = useFilter(orders, 'order', 'active', curr);

Ну а згодом? Про наслідки можливі не подумать? Один лиш тільки раз і вже крадеться піздець з своєю усмішкою хижой

Можливі наслідки

  • Зайва деталізація: Надмірне подрібнення функцій або компонентів може призвести до надмірного утворення дрібних сутностей, що ускладнює підтримку й навігацію по коду
  • Затрати часу: Виділення окремих функцій і хуків для кожної задачі потребує більше часу на розробку й тестування, що може бути зайвим для дрібних проєктів
  • Надмірна абстракція: Надто велика абстракція може зробити код важким для розуміння, оскільки логіка розподіляється між багатьма частинами, ускладнюючи роботу з ним
  • Не завжди виправдано для малих проєктів: У простих або малих проєктах надмірне дотримання певних підходів може зробити код важчим для читання та підтримки без суттєвих переваг

З великою силою приходить велика відповідальність

Але одна

Поганий код

const fetchUsers = async () => {
  const data = await fetch("/api/users").then((res) => res.json());
  setUsers(data);
};

const fetchOrders = async () => {
  const data = await fetch("/api/orders").then((res) => res.json());
  setOrders(data);
};
…
const [formErrors, setFormErrors] = useState({});
const validateOrder = useCallback((order) => {
  if (order.amount <= 0) {
    setFormErrors((prev) => ({ ...prev, [order.id]: "Invalid amount" }));
  } else {
    setFormErrors((prev) => ({ ...prev, [order.id]: "" }));
  }
}, []);
  • Зайві обов’язки: Компонент відповідає за кілька різних задач, що ускладнює його підтримку та тестування
  • Ускладнене тестування: Потрібно враховувати багато сценаріїв та станів компонента, що збільшує кількість тестів і ускладнює їх написання
  • Погана масштабованість: Коли змінюються вимоги, доводиться змінювати весь компонент, що робить його менш гнучким і збільшує ризик помилок при змінах
  • Обмежене повторне використання: Логіка компонента тісно пов’язана з конкретними сценаріями
  • Заплутаний код: Втрачається читабельність, зростають затрати часу на підтримку та внесення змін

Хороший(?) код

import { useUsers } from "./useUsers";
import { useOrders } from "./useOrders";
import { validateOrder } from "./validation";
import { useFormErrors } from "./useFormErrors";

const Dashboard = () => {
  const { users, fetchUsers } = useUsers();
  const { orders, fetchOrders } = useOrders();
  const { errors, validate } = useFormErrors();

  const handleValidation = (order) => {
    activeOrders.forEach(validate);
  };
  …
}
  • Чітке розділення обов’язків: Кожен модуль виконує тільки свою задачу, наприклад, завантаження даних, валідація чи обробка помилок, що робить код зрозумілішим та легшим для підтримки
  • Полегшене тестування: Оскільки функції й хуки ізольовані, їх можна тестувати незалежно, що спрощує написання тестів і полегшує виявлення проблем
  • Масштабованість і гнучкість: Додавання нових функцій або зміна існуючих не вимагає змін у всьому компоненті, що підвищує гнучкість і знижує ризик помилок під час масштабування

Можливі наслідки

  • Важко знайти єдину відповідальність: Компоненти часто мають поєднувати логіку й відображення, тому суворе дотримання SRP може бути недоцільним і вимагати зайвих зусиль у реальних проєктах
  • Сповільнення розробки: Поділ на дрібні модулі та хуки збільшує час на розробку, тестування й інтеграцію, що може негативно вплинути на швидкість проєкту
  • Підвищена складність коду: Надмірне розбиття на окремі частини може зробити код заплутаним, складним для розуміння та підтримки через велику кількість взаємозалежних компонентів
  • Ризик наплодити клонів: Поділ відповідальності може призвести до створення занадто багатьох подібних компонентів, що збільшить дублювання коду і ускладнить його підтримку

Застав дурня богу молиться, то він і лоб розібʼє

Народна мудрість. Не про KISS

Поганий код

const orderSummary = useMemo(
  () =>
    activeOrders.length > 0
      ? activeOrders.length > 10
        ? activeOrders.every((order) => order.priority === "high")
          ? activeOrders.some((order) => order.currency === "USD")
            ? "Many high-priority USD orders"
            : "Many high-priority non-USD orders"
          : "Many orders with mixed priorities"
        : activeOrders.length > 5
        ? "Few active orders"
        : "Very few orders"
      : "No active orders",
  [activeOrders]
);
  • Зниження зрозумілості: Складні або “езотеричні” конструкції ускладнюють читання та розуміння коду, що може призвести до виникнення помилок і зниження загальної прозорості
  • Труднощі з підтримкою: Складна структура коду ускладнює внесення змін, адже будь-яка модифікація вимагає більше часу на розуміння та може зачепити декілька частин системи, підвищуючи ризик помилок
  • Ускладнення тестування: Через складний, заплутаний або занадто “мудрий” код тестування стає важчим, оскільки такі конструкції важче ізолювати для модульного тестування та перевірити на коректність
KISS — I Was Made For Lovin' You

Хороший(?) код

const orderSummary = useMemo(() => {
  if (activeOrders.length === 0) {
    return "No active orders";
  }

  if (activeOrders.length > 10) {
    const allHighPriority = activeOrders.every(
      (order) => order.priority === "high"
    );
    const hasUsdOrders = activeOrders.some((order) => order.currency === "USD");

    if (allHighPriority) {
      if (hasUsdOrders) {
        return "Many high-priority USD orders";
      } else {
        return "Many high-priority non-USD orders";
      }
    } else {
      return "Many orders with mixed priorities";
    }
  }

  if (activeOrders.length > 5) {
    return "Few active orders";
  }

  return "Very few orders";
}, [activeOrders]);
  • Простота полегшує підтримку: Код стає більш зрозумілим і легким для читання, що зменшує ризик виникнення помилок та спрощує його модифікацію
  • Легкість тестування: Чітка та проста логіка дозволяє легко тестувати окремі частини коду, знижуючи складність написання тестів
  • Швидке внесення змін: Простий код легше змінювати або розширювати, що зменшує ризик зламу функціональності при модифікаціях
  • Зменшення ризику помилок: Менша кількість складних конструкцій знижує ймовірність помилок та забезпечує стабільність коду при внесенні змін

Можливі наслідки

  • Може знизити продуктивність: Надмірна простота може ігнорувати можливості для оптимізації, що негативно вплине на швидкодію
  • Підвищує ризик дублювання коду: Спрощення може призвести до повторення логіки в різних частинах програми замість її перевикористання
  • Може обмежити гнучкість: Спрощений код часто важче адаптувати до нових вимог або умов, що обмежує можливість масштабування
  • Неповне вирішення складних проблем: Занадто простий підхід може призвести до втрати важливих деталей або функціональності, особливо в складних системах

і шо тепер, не рефакторить?

  • DRY прагне уникнути дублювання, але може ускладнити модулі, що порушить SRP і KISS
  • SRP забезпечує чітку відповідальність кожної частини коду, але може призвести до дублювання логіки або створення занадто багатьох компонентів
  • KISS зберігає код простим, але може призвести до дублювання або порушення SRP

Балансуючи ці три принципи, важливо враховувати контекст проєкту та конкретні вимоги. Іноді варто порушити один із принципів, щоб забезпечити простоту чи гнучкість, не жертвуючи зручністю підтримки.

Висновки

  • Зберігайте розумний баланс між принципами: Не всі принципи та підходи працюють у кожному контексті. Важливо адаптувати їх до специфіки проєкту, не жертвуючи підтримуваністю чи швидкодією
  • Компроміси неминучі: Під час рефакторингу доведеться балансувати між різними вимогами, такими як простота, гнучкість і продуктивність
  • Контекст диктує рішення: Вибір підходів залежить від реальних потреб проєкту, обмежень команди та термінів, а також вартості підтримки в майбутньому

Домашнє завдання: спробуйте відрефакторити код з цієї презентації, але обовʼязково прислухайтеся до свого внутрішнього маленького януковича

— Астанавітєсь!

Що почитати?

Не то шоб це прямо по темі доповіді, але цікаво і корисно буде.

DataRobot наймає!

  • Frontend Engineer
  • Senior Backend Engineer

Дякую, приходьте ще