Сім ознак, які свідчать про потребу у перегляді та можуть призвести до дорогих змін у майбутньому

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

Що насправді говорить про проблеми в коді

Досвідчений рецензент рідко потребує читати весь модуль, щоб виявити проблему. Булевий аргумент, мета якого залишається незрозумілою там, де викликається функція, утиліта з десятком необов’язкових функцій, блок catch, який перетворює кожну помилку на однакове, здавалося б, невинне значення — усе це сьогодні не призводить до жодних проблем. Проблема полягає у тому, що це може створити завтра.

Цей інстинкт походить від досвіду, а не від суворішого посібника зі стилю. Люди, які роками підтримують системи, бачили, як одне скопійоване правило перетворюється на три суперечливі версії, як зручна абстракція стає місцем для кожного особливого випадку, а стандартний варіант роботи призводить до приховування проблем у виробництві протягом тижнів. Вони навчилися розпізнавати моменти, коли код накопичує невизначеність.

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

• Хто саме чи який модуль відповідає за це рішення?
• Як це буде функціонувати, якщо зміняться вимоги?
• Чи буде помилка все ще помітною під час її виникнення?
• Чи зможе наступний розробник зрозуміти, чому код сконструйований саме так, не з’ясовуючи, хто його написав?

Сім сигналів, наведених нижче, є тими, які найнадійніше призводять до збитків, якщо ніхто не поставить цих запитань на ранній стадії.

1. Одна функція виконує кілька непов’язаних завдань

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

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

Корисним ознакою є назва, яка не може чесно описати функцію. Щось на кшталт processOrder може перевіряти інформацію про клієнта, обчислювати ціни, контролювати наявність товару, створювати інтент оплати, зберігати інформацію про замовлення та надсилати підтвердження електронною поштою. Назва залишається нечіткою, тому що точна назва зайняла б цілий абзац.

Чому механічне вилучення — не рішення

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

• Де приймається бізнес-рішення?
• Яка частина є побічним ефектом для зовнішньої системи?
• Яка частина — це суто перетворення даних?
• Яка частина відповідає за транзакцію?

Як тільки ці шви стають видимими, код можна організувати відповідно до того, як насправді працює процес. Основною проблемою ніколи не була кількість операцій, які виконує функція. Проблема полягала у тому, що ніхто не міг змінити жодне рішення, не враховуючи одночасно кілька непов’язаних між собою факторів.

2. Булеві параметри, які увімкнюють приховані режими

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

await updateAccount(accountId, true, false);

Щоб зрозуміти це, потрібно відкрити реалізацію та з’ясувати, чи перший true змушує до оновлення, викликає сповіщення, обходить перевірку, зберігає історію чи активує обліковий запис. Той, хто написав функцію, може пам’ятати це, але всі інші мусять відновити сенс з позицій аргументів, причому поміняна пара літералів пройде будь-яку перевірку типу.

Комбінації, які ніхто не проектував

Проблема посилюється з накопиченням флагів. Три булеві значення створюють вісім можливих комбінацій, кілька з яких ніхто навіть не розглядав. Якщо функція приймає параметри forceUpdate, skipValidation та sendNotification, що має відбутися, коли примусове оновлення обходить перевірку, але все одно надсилає сповіщення? Система типів з радістю дозволяє такий виклик, навіть якщо це не повинно бути допустимо в контексті бізнес-логіки.

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

Вибір правильної оптимізації

У деяких випадках достатньо перейти на об’єкт з одним аргументом, властивості якого мають імена, оскільки updateAccount(id, { notify: true }) принаймні сам по собі зрозумілий у місці виклику. У інших випадках окремі функції, такі як verifyAccount та overrideAccount, роблять бізнес-намір явним та повністю усувають неможливі комбінації. Вибір підходу залежить від того, чи є ці дії справжніми варіаціями однієї операції, чи окремими операціями, які випадково мають спільний код.

Булеві значення не є за своєю природою поганими. Опція з простим ім’ям, така як includeArchived, є цілком доцільною. Попереджувальним знаком є флаг, який змінює значення функції, а не просто коригує якусь добре зрозумілу деталь.

3. Та сама бізнес-правило, виражене у різних формах

Текст, скопійований та вставлений, легко розпізнати. Копійовані знання є ще небезпечнішими, адже кожна копія може виглядати зовсім інакше.

Візьмемо правило дозволу. Воно може проявлятися як прихована кнопка в інтерфейсі користувача, перевірка ролі в контролері, умова статусу в сервісі чи оператор WHERE у запиті. Кожен шар, здається, захищає систему, але кожен з них може містити трохи інше розуміння того, хто має право діяти.

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

Досвідчені рецензенти помічають повторювані комбінації статусів, ролей, прапорців та дат, оскільки такі умови часто відображають однакове бізнес-значення, описане кілька разів. Вони запитують, де знаходиться остаточне рішення, та чи інші шари його дотримуються чи тихо переопредмечують.

Шари з різними завданнями — це нормально

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

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

4. Спільний помічник із об’єктом опцій, який постійно розширюється

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

Тоді двоє користувачів починають вимагати різних речей. Один потребує повторних спроб; інший не повинен повторювати свій запит. Один отримує перевагу від кешування; інший не повинен бачити застарілих значень. Один повертає повний набір даних; інший передає їх у потоці. Замість того, щоб розділяти ці процеси, допоміжний модуль створює новий варіант для кожної відмінності.

Незабаром об’єкт опцій охоплює переклад помилок, форматування результатів, налаштування кешування, кількість повторних спроб, обробку автентифікації, функції-зачепи та кілька булевих перемикачів. Більшість користувачів використовують два-три з цих елементів, але будь-які зміни можуть вплинути на всі з них.

Це є проявом певного запаху — абстракція припинила усувати складність та почала її концентрувати. Інтерфейси виглядають простими, але розуміння системи тепер означає знання того, як кожен варіант взаємодіє з усіма іншими. Велика кількість викликів не є доказом того, що абстракція є правильною; іноді це лише свідчить про те, що використання допоміжних елементів стало неминучим.

Схожа синтаксис проти спільної поведінки

Ключове питання полягає у тому, чи мають ці елементи стабільну поведінку, чи просто схожий код. Дві операції можуть обидві надсилати HTTP-запити, проте по-різному поводитися з помилками та спробами їх усунення. Пара форм може мати однакові поля, але різну логіку перевірки, надсилання та обробки проєктів. Їх об’єднання усуває видиме дублювання, але ускладнює вираження кожної справжньої різниці.

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

5. Обробка помилок, яка приховує невдачу під успіх

Блок catch може створювати враження надійності додатку, при цьому тихо підриваючи довіру до нього.

Уявіть собі шар доступу до даних, який у випадку помилки бази даних повертає []. Або сервіс, чия HTTP-запитування зазнає невдачі та просто повертає null. Фронтенд перетворює кожну відхилену запит на стан порожнечі. Нічого не зламується, але система більше не може відрізнити справжню відсутність даних від невдалих спроб їх отримання.

Рев’юери швидко помічають цю проблему, оскільки знають, як вона проявляється під час інциденту. Панель керування повідомляє про відсутність записів, поки база даних несправна. Клієнт бачить повідомлення „Замовлень не знайдено“ після того, як його сесія мовчки закінчилась. Фонове завдання позначає елемент як виконаний, оскільки залежність не повернула нічого корисного. Інтерфейс залишається спокійним, тому що замінив правду на правдоподібну відповідь.

Питання, які потрібно поставити до кожного блоку обробки помилок

Під час перевірки обробки помилок перевірте три речі:

• Чи зберігається первинна причина, наприклад як cause нової помилки, або принаймні у журналах?
• Чи може той, хто викликає функцію, розрізнити очікуваний бізнес-результат від технічної несправності?
• Чи належить шар, який ловить помилку, безпосередньо до прийняття наступного рішення?

Якщо код на цьому рівні не має можливості відновитися, спробувати знову, скасувати часткові дії або надати розумну заміну, ловлення помилки може нічого не дати, окрім знищення доказів.

Усе це не означає, що первинні винятки мають потрапляти до користувачів. Повідомлення, які відображаються людям, мають бути безпечними та зрозумілими, а внутрішні помилки часто потребують перекладу у стабільні категорії домену. Вимога полягає у тому, щоб переклад зберігав достатню кількість значення, щоб як додаток, так і розробники могли правильно відреагувати.

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

6. Невелика вимога зі значними, непов’язаними змінами

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

Додавання одного поля може закономірно торкнутися механізмів зберігання даних, сервісів, відповідей API та користувацького інтерфейсу; це є послідовною зміною у всіх шарах системи. Проблема виникає, коли той самий pull request також переписує спільні утиліти, змінює назви несуміжних типів, модифікує механізми обробки помилок, переміщує модулі та оновлює великі файли з копіями даних.

Кожна з цих змін може бути покращенням, але разом вони об’єднують кілька причин змін у одну структуру. Рецензенти повинні одночасно оцінювати функціонал, процес рефакторингу, зміни в контракті та очищення коду. Якщо виникає регресія, знайти конкретну причину стає складніше. Якщо функціонал доводиться скасувати, разом із ним йдуть і несуміжні покращення.

Чому кодування за допомогою ШІ робить це більш поширеним

Інструменти генерації коду сприяють тому, що люди частіше потрапляють у цю патерну, оскільки повне переписування коштує майже нічого. Якщо ви попросите виправити лише одну помилку, ви можете отримати оптимізовану функцію-замінник із новими назвами, іншою структурою керування, сучасними абстракціями та зміненою поведінкою під час виникнення помилок. Створення такої функції коштувало майже нічого; проте її перевірка все одно вимагає від команди тих самих зусиль, що й завжди.

Хорошим правилом є підтримувати розмір змін у пропорції до того, наскільки чітко їх можна пояснити. Вузький діагноз зазвичай вимагає вузького рішення, а більш масштабні зміни можна включити у окремий pull request, де їх можна буде розглянути та протестувати окремо. Це не означає, що потрібні мініатюрні pull request у кожному випадку; деяка архітектурна робота за своєю природою є масштабною. Питання полягає у тому, чи кожна значуща зміна сприяє досягненню єдиного цілеспрямованого результату. Коли різниця між версіями більша за саму проблему, зазвичай одночасно вирішуються кілька питань без їх розділення, через що стає складніше визначити причину та наслідок.

7. Код, який працює, але ускладнює перевірку

Деякі реалізації створюються швидко саме тому, що їхню поведінку важко спостерігати чи контролювати. Функція безпосередньо зчитує системний годинник, створює власні залежності, змінює глобальний стан, виконує кілька зовнішніх викликів, приховує власні помилки та майже нічого не повертає у відповідь. У успішному випадку все може бути гаразд, але демонстрація його невдачі стає складною та дорогою.

Тестованість зазвичай відображає якість меж. Якщо бізнес-правило неможливо протестувати без запуску всього додатку, заповнення рядків бази даних, виклику реальних сервісів та очікування на реальні таймери, то це правило, ймовірно, пов’язане з занадто багатьма побічними ефектами.

Асинхронні робочі потоки виявляють цю прогалину

Проблема найбільш гостра у багатокрокових асинхронних процесах. Уявіть функцію, яка зберігає зміни, генерує подію домену, сповіщає користувача та додає рядок до журналу аудиту. Якщо третій крок зазнає невдачі, чи може тест визначити, які кроки вже були виконані? Чи можна змусити залежність завершитися через таймаут? Чи можна симулювати двократне виконання функції? Чи можна відтворити застарілу оновлення без покладання на випадковості часу?

Код, який легко написати, але важко перевірити, створює надмірну впевненість у ручних тестах. Хтось пройде успішний шлях, побачить правильний результат та дійде висновку, що функція готова. У продакшені ж з’являється конкурентність, повторні спроби, дивні дані та незавершені операції, які ніхто не тестував локально.

Перевіряйте на рівні, де існує ризик

Чистота всюди та використання моків для всього також не є рішенням. Тести, побудовані переважно на моках, схильні зафіксувати внутрішні деталі на місці, ігноруючи те, як насправді поводяться база даних, мережа чи транзакції. Кращим підходом є визначити, які гарантії мають значення, та перевірити кожну з них у місці, де насправді існує ризик:

• Чисті трансформації можна тестувати як окремі одиниці.
• Інваріанти бази даних часто потребують інтеграційних тестів з реальною базою даних.
• Для визначення меж сервісів можуть знадобитися тести контрактів.
• Розподілена поведінка може залежати від структурованих журналів та ідентифікаторів кореляції, оскільки не кожну умову продакшену можна точно відтворити.

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

Основні висновки

Жоден з цих сигналів не є автоматичним вердиктом. Довга функція може представляти одну чітку операцію, булеве значення може бути цілком очевидним, спільний помічник може відображати справді стабільну поведінку, а масштабна зміна може бути неминучою. Розглядайте цей список як спосіб визначити, де потрібно глибше дослідити, а не як чек-лист для механічного дотримання.

Під час перегляду виникають такі основні запитання:

• Чи змішані обов’язки, через що одна зміна тягне за собою нерелевантні рішення?
• Чи можуть користувачі зрозуміти функцію без розшифровки позиційних флагів?

Чи є у кожного бізнес-правила один чіткий власник?

Чи приховує спільна абстракція справжні відмінності за допомогою опцій?

Чи зберігає обробка помилок інформацію про те, що саме пішло не так?

Чи є різниця пропорційною вимозі, яку вона задовольняє?

Чи можна перевірити важливу поведінку, коли щось йде не так?

Те, що насправді помічають досвідчені рецензенти, — це відкладена невизначеність: ймовірність того, що наступна вимога змусить когось розуміти непов’язану логіку, що наступний інцидент залишить занадто мало доказів для розслідування, що наступний розробник не знатиме, яка поведінка була навмисною, або що наступний користувач додасть ще один варіант до абстракції, яка вже несе занадто багато. Вони швидко розпізнають ці ознаки, тому що вже стикалися з ними раніше — розбираючи перевантажені функції, відстежуючи приховані винятки, узгоджуючи суперечливі правила та переглядаючи „невелику“ функцію, яка впливає на половину кодбази. Мета не у скасуванні коду, який виглядає невпорядкованим; мета — виявити код, який у майбутньому ускладнить роботу більше, ніж це необхідно.