Дубльований код. Чи завжди варто намагатися його позбутися

Підписуйтеся на Telegram-канал «DOU #tech», щоб не пропустити нові технічні статті

Всім привіт. Мене звати Олег Шастітко, я займаюсь розробкою, проєктуванням і аналізом архітектури програмних продуктів, побудованих на .NET. Свій шлях розробника я почав ще у 2001 році, з використанням технології ASP і згодом почав розробку .NET застосунків, починаючи з виходу першої версії .NET у 2002 році.

Більшість з розробленого та спроєктованого мною ПЗ було з використанням ASP.NET й інших технологій мережевої взаємодії. Тобто мій основний напрям — системи з фокусом на використання як веб зі взаємодією між компонентами системи, а не на розробку високонавантажених багатопотокових застосунків.

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

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

Вступ

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

Але чи справді дубльований код — це завжди привід бити на сполох?

Скажу одразу — ні, не завжди! Дубльований код — це лише сигнал звернути увагу на цей фрагмент, уважно проаналізувати архітектуру та задачі, які він виконує в обох (або більш ніж в обох) випадках і тільки після цього, у разі потреби, робити рефакторинг!

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

Дублювання анемічної моделі

Розгляньмо одну тривіальну задачу. Є реляційна база даних. Ми вибираємо з неї дані за допомогою сервісу, що перетворює їх на DTO-об’єкт (зрозуміло, що при проєктуванні БД як мінімум 4NF нормальної форми, цей клас, швидше за все, буде розбитий в реляційній БД на декілька класів. Наприклад, Department зберігатиметься в окремій таблиці). Нехай цей DTO-клас виглядає так:

    public class ClientDto

    {

        public string ClientName { get; set; }

        public string ClientDepartment { get; set; }

        public string CompanyName { get; set; }

        public string? Phone { get; set; }

        public string? Email { get; set; }

        public string? Website { get; set; }

        public string? MailingAddress { get; set; }

        public string? City { get; set; }

        public decimal Balance { get; set; }

        public int DistanceMeters { get; set; }

    }

Ми викликаємо цей метод з контролера ASP.NET, і метод цього контролера створює View model, передаючи її далі в безпосередньо View. В більшості випадків цей клас View model буде вражаючи схожий на наш DTO-клас або навіть ідентичний з ним. Наприклад:

    public class ClientVm

    {

        public string ClientName { get; set; }

        public string ClientDepartment { get; set; }

        public string CompanyName { get; set; }

        public string? Phone { get; set; }

        public string? Email { get; set; }

        public string? Website { get; set; }

        public string? MailingAddress { get; set; }

        public string? City { get; set; }

        public decimal Balance { get; set; }

        public int DistanceMeters { get; set; }

   }

Тут ми бачимо всі ознаки дублювання коду: якщо з’являється нове поле — ми повинні його додати в обидва класи, якщо поле буде перейменовано — важливо не забути зробити це для обох класів. Інакше на виході побачимо зовсім не те, що очікуємо. Особливо якщо використовуємо засоби для автоматичного мепінгу, наприклад Automapper, і не зобов’язані явно присвоювати кожне поле. І, дуже прошу, без холівара про автомапер і його особливості, будь ласка!)

Але не поспішайте бити на сполох, звинувачувати себе в тому, що ми мало не припустилися помилки Junior’а, і терміново кидатися створювати новий абстрактний клас (назвемо його ClientAbstract), від якого будуть успадковуватися ці обидва класи.

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

Я міг би сказати, що створення абстрактного класу для цих двох буде змішування шарів. А ви б відповіли: але ж ми можемо винести ці абстрактні класи в окрему збірку без будь-яких залежностей. Частково ви матимете рацію. Саме частково, тому що сервіс, що повертає ClientDto — це високорівневі бізнес-правила, описані передусім саме бізнесом і непорушні самі по собі. Адже вони диктують додатку, яким йому бути, а не навпаки. А ClientVm — це низькорівнева реалізація веб, яка всього лише «морда». Сьогодні вона одна, завтра — інша та бізнесу стосується опосередковано.

Але принцип єдиної відповідальності (SRP) говорить нам про те, що клас має мати одну і тільки одну причину для змін. Для сервісу бізнес-логіки — це зміна бізнес-логіки. Для вебу — те, як саме цей конкретний застосунок має показати дані. Веб, найімовірніше, буде змінюватися набагато частіше і непередбачуваніше, ніж бізнес-логіка. Кожна зміна в вебі поставить питання про зміну цього базового абстрактного класу ClientAbstract, який має бути стабільним настільки, наскільки стабільна бізнес-логіка. Принцип стійких залежностей SDP говорить нам про те, що менш стійкі компоненти, тобто ті, які більш схильні до змін, повинні залежати від більш стійких. У нашому випадку веб повинен залежати від бізнес-логіки, але не навпаки.

Уявіть, що завтра клієнти вирішать: змінна Balance, що виводиться вами як число decimal, виглядає не по фен-шуй для вебсторінки та її потрібно виводити зі знаком $ і роздільниками тисяч і дробових значень, але не більше двох цифр після коми. А DistanceMeters має виводитися або в метрах, або в кілометрах, тобто клас View Model набуває вигляду. Ну а ми на рівні контролера робимо ці перетворення. Можна звичайно нічого не чіпати й перетворити на клієнтові, але якщо він не підтримує клієнтський код, або є вимога мати його якомога менше:

    public class ClientVm

    {

        public string ClientName { get; set; }

        ......

        public string BalanceDollars { get; set; }

        public string Distance { get; set; }

   }

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

Цей банальний приклад показує, що іноді не потрібно позбуватися продубльованого коду, іноді він — норма.

Дублювання логіки й алгоритмів

Я зустрічав таку думку, що принцип DRY відноситься насамперед до логіки, а не до структур на кшталт анемічних моделей DTO. Але розгляньмо приклад, коли дубльований код в алгоритмах не повинен зазнати рефакторингу.

Уявімо, що у нас є якась бактерія, що зі стійкістю атома урану-235 ділиться навпіл. Скажімо, щохвилини. Тобто через одну хвилину (перше покоління) у нас буде 2 бактерії, через 2 хвилини (друге покоління) — 4 бактерії. Ми пишемо свій видатний і неперевершений у складності алгоритм, який рахує, скільки ж бактерій буде у Х поколінні. Виглядає він так:

        public int GetBacteriaCount(int generation)

        {

            return Math.Pow(2, generation);

        }

Все чудово. Тепер в іншому місці нашого проєкту треба порахувати, скільки предків має людина у Х поколінні. Тобто у першому поколінні у нас двоє предків (мама й тато), у другому — четверо (дві бабусі та два дідуся) тощо. Пишемо другий алгоритм:

        public int GetAncestorsCount(int generation)

        {

            return Math.Pow(2, generation);

        }

Бінго! Тепер ми дивимося на обидва методи й бачимо, що вони дивовижно схожі. Згадуємо наше чарівне Don’t repeat yourself і що робимо? Вірно, об’єднуємо обидва алгоритми в один. Все працює!

Але що поєднує ці два алгоритми? Не в площині підрахунків, а в площині бізнес-правил? Хіба процес розмноження бактерії має щось спільне із нашим родинним деревом? Хіба ці два процеси йдуть однаково за одними й тими самими законами? Ні!

Коли ми починаємо рахувати предків у, наприклад, 50 поколінні (а це лише 1000-1500 років тому), то раптово виявляємо, що отримуємо якесь астрономічне число. Порівняно з яким навіть кількість людей, що живуть сьогодні — мізер. Чому так сталося? Справа в тому, що всі люди — брати мають родинні зв’язки. Наші лінії предків перетиналися безліч разів. Тобто 1000 років тому у нас були не мільярди унікальних прапрабабусь з такими ж дідусями, а значно менше число. Їх нащадки потім створювали пари між собою (так, це спорідненість, але настільки далека, що вже не має негативного впливу на їх розвиток). Тобто ми маємо запровадити якийсь коефіцієнт на кшталт:

        public int GetAncestorsCount(int generation, double k)

        {

            return k * Math.Pow(2, generation);

        }

Або це буде зовнішній сервіс, що впроваджений у наш клас, який повертає цей коефіцієнт залежно від покоління. Наприклад:

        public async Task<int> GetAncestorsCountAsync(int generation)

        {

                var k = await _extService.GetCoefficientAsync(generation);

                return k * Math.Pow(2, generation);

        }

Або ще якийсь спосіб підрахунку корекції.

А що з розмножуванням бактерій? Вони з часом стають обмежені середовищем, в якому живуть, поживними речовинами та іншими факторами. Ми можемо це описати як:

        public int GetBacteriaCount(int generation, int minutes)

        {

              .....

        }

Або, знову ж таки, з використанням якогось зовнішнього сервісу. У будь-якому випадку ми бачимо, що шляхи розвитку цих алгоритмів розійшлися, хоч спочатку вони були ідентичні! Тому що вони підкоряються різним бізнес-правилам. Мають різні причини для зміни.

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

public virtual int GetEntityCount(int generation) {

        return Math.Pow(2, generation);

}

А параметри, специфічні для конкретної реалізації, передавати іншим способом (наприклад, через конструктори класів-нащадків). Але це буде шлях, коли замість того, щоб відрубати непотрібне вже зараз, ми тягнемо його далі. Чи мають спільну основу два алгоритми? Залежно від цього і треба приймати рішення, повинні вони мати щось спільне при проєктуванні чи ні! В нашому випадку — не мають. І ніщо не гарантує, що нові дослідження завтра не скажуть нам, що бактерія ділиться не за законом степеня двійки, а за степеня трійки чи ще якось.

Декілька слів про принцип DRY

До речі, додам щодо принципу DRY: якщо вдуматися у сенс виразу «Don’t repeat yourself», він закликає «не повторюватися». Що не означає явно «дубльований код». «Не повторюватися» означає не створювати саме такий код. Це може бути як дубльований код там, де його можна і потрібно відрефакторити, так і абсолютно різний код, який при цьому виконує те саме! Наприклад, два різних алгоритми, які виконують одну й ту саму задачу. Це якраз порушення принципу DRY, на відміну від описаних вище випадків.

Висновок

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

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

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

PS. Дякую Nikola Utiuzhnikov за зауваження щодо помилки в моєму тривіальному коді :)

👍ПодобаєтьсяСподобалось11
До обраногоВ обраному2
LinkedIn
Дозволені теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter
Дозволені теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter

Трохи дев-філософського задротства:

Як ми знаємо, є два види рівності (equality) обʼєктів: structural equality та reference equality.
Тобто, у першому випадку ми порівнюємо, умовно, суттєві данні обʼєкта, у іншому випадку — маємо два посилання на один і той самий обʼєкт.

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

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

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

тому що потрібно завжди дивитися оченятами, що ми мержимо :)

При великому обʼємі можна не зрозуміти, що не так. І Git сам по собі не надає зручних засобів для цього (як там в графічних мордах — не дивився).

При великому обʼємі можна не зрозуміти, що не так.

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

ділов то

Счастливые часов не наблюдают ©

Спробуйте графічну морду, у мене проблем з візуалізацією того, що мержимо, немає

1. Поправка: я не испытывал граф. морды на особо крупных объёмах (считаем, от 10k строк). С какими объёмами у вас нет проблем с визуализацией — без подробного примера не понять.
(Вопрос, зачем мержить сразу сотнями тысяч миллионами, не рассматриваем, тут могут быть административные ограничения. Я, естественно, стараюсь этого избегать.)
2. Даже в случае средних объёмов сравнивать по каждому чиху «а тут точно наложился патч для process_foo() именно на process_foo(), а не отличающуюся от неё в 5 строках 50-строчную process_moo()?» - задалбывает даже на таких размерах, а если это будет 200+ строк, то тем более. Это уже более-менее надёжно ловится скорее тестами (главное, чтобы они были).

1. Поправка: я не испытывал граф. морды на особо крупных объёмах (считаем, от 10k строк). С какими объёмами у вас нет проблем с визуализацией — без подробного примера не понять.

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

не понял, разница при каждом мержинге 10 тыс строк?

Откуда идея про «каждом»?

Извините, тут наверное ничего не поможет...

Ну если так фантазийно читать, то да...

у меня был проект с сотнями тысяч строк (и не спрашивайте, почему всё пихали в один монолит), но я не пойму, в чём проблема мержить те 10-20 изменений

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

Приклади:
* Логіка розрахування цін
* Логіка валідації/модерації контенту
* модуль нотифікацій в слаке (це канонічний приклад)
* модуль якій розраховує як показувати твіттер таймлайн

Что бы код не дублировался, попробуй запаковать его в zip или rar архив, дублирования не останется и следа, да ещё и место под btc хеши освободиться!

Щоб вирішити чи дублювати код чи ні, можна задати питання — чи будь-які потенційні зміни в одній частині однозначно повинні бути застосовані до іншої.
Якщо відповідь так, створюємо абстракцію, чемно застосовуємо DRY
Якщо відповідь ні, дублюємо.
Я це називаю семантикою коду, інколи 2 ідентичні блоки коду мають різну семантку, передають різну суть, як в прикладі з DTO та View. Тоді слід дублювати.

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

Приклад із DTO не є прикладом дублювання коду.
Тут ви спочатку вирішуєте завдання mapping вашої бази даних на ваші класи, а потім вирішуєте завдання mapping полів для вашого клієнтського UI, виходячи з тих вимог, які він пред’являє. Тобто те, що поля можуть бути однакові, свідчить про схожість вимог, а не про їхнє копіювання.
А дублювання коду — це коли є певний код, який виконує певне завдання. І його копіюють в інше місце, де він вирішуватиме таке саме завдання.

Саме цю думку я і хотів пронести через всю статтю! Що це не дублювання як такове, це схожі вимоги :)

Якщо код хороший — гріх не копіпаснути

Хорошего кода должно быть много

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

і інколи матюкатись, але краще матюкатись і отримувати $ ніж мати гарний код і не отримувати $ :D

Чомусь згадалося. Якось було таке, що мені один колега під час code review запропонував об’єднати два юніт теста в один, бо в них були однакові чаcтини Arrange, а відрізнялися лише Act і Assert. Тобто сказав, що треба об’єднати два суттєво різних юніт-теста в один. Типу, це зменшить дублювання коду, а менше коду — це завжди краще. Зараз смішно, а тоді не було, бо на той момент я не міг нічого мьоржити в мастер без його апрува.
Різні люди пропонують багато різної дурні. Ба, більше, часто існує більше одного способу зробити одну й ту саму річ, і кожен із способів має як плюси, так і недоліки. Сперечатися про один найкращий спосіб організації data layers у веб-аплікації можна нескінченно і ні до чого не прийти, бо ідеала не існує.
Колись я в одному з проектів активно використовував Nuget на ім’я AutoMapper і мені здавалося до певного моменту, що він трохи полегшує життя. Потім усвідомив, що навпаки, додає зайвої складності і зменшує контроль коду компілятором. Якщо забув зареєструвати відповідний профайл у відповідному класі, дізнаєшся про це лише коли отримаєш exception. Набагато краще робити щось на кшталт окремого конвертера під кожний мапінг і реєсрувати його як окремий інтерфейс, принаймні на відносно невеликих проектах. Тоді маппінг або є, або, якщо його нема, буде сompile time error при першому намаганні його використати.
Чим довше займаюся розробкою, тим більше погоджуюся з думкою, що срібних куль на всі випадки життя тупо не існує. Кожен проект має свої особливості і недоліки, і кожне рішення треба приймати з урахуванням всього наявного зоопарку.

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

Ось тут повністю згоден.

Ну, це — доволі тривіально думка. :)

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

Якщо забув зареєструвати відповідний профайл у відповідному класі, дізнаєшся про це лише коли отримаєш exception.

а юніт-тести навіщо? :)

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

Я також не прихильник покриття тестами 100% коду (як той же «дядюшка Боб» пропонує), але тут же мова йде не про те, що контролюється компілятором. Якщо вилітає ексепшн в рантаймі (незареєстрований профайл мапінга), то значить програма пройшла етап компіляції. Чи я Вас не так зрозумів :)

Тут мова йде про те, що AutoMapper виводить маппінги з під контролю компілятора. А вони, взагалі-то — дуже проста річ, просто купка присвоювань. Якщо просто зробити щось типу свого інтерфейсу типу IMapper<TSource, TDest>, реєструвати всі маппінги таким чином і просто передавати в конструктори там, де вони необхідні. Все буде чисто, прозоро, зрозуміло і набагато краще контролюватися компайлером. Тобто в принципі, AutoMapper нічого додатково не дає крім геморою. :)

Ось саме цю частину кода я, навпаки, намагаюся покрити тестами :)

Покриття тестами і контроль статичної типізації компілятором — це дві речі, яка одна одній ніяк не заважають. Звісно, можна підміняти друге першим, але трошки затратно. Є мови, де взагалі немає статичної типізації, наприклад, Java Script. Але мені їх використовувати вкрай некомфортно. Бо кожен додатковий тест треба спочатку написати, а потім ще й підтримувати. Геть знижує швидкість розробки.
Відмовлятися від статичної типізації є сенс лише в дуже специфічних випадках. Маппінг ДТО-шек точно таким не є.

Ми щось трохи о різном. Контроль статичної типізації компілятором є «by default», на те вона і статична типізація

Я ось про що. Коли викликаєш AutoMaper для копіювання даних з одного типу в інший, компілятор гадки не має, чи є в конфігурації викликаного мапера потрібний для цього маппінг. AutoMapper навмисно, або через погано продуману архітектуру приховує цю інформацію від компілятора. Тобто коли я роблю виклик типу:
OrderDto dto = mapper.Map<OrderDto>(order);
компайлер може перевірити якого типу змінна order, також може перевірити наявність типу OrderDto, але він гадки не має, чи здатний цей виклик перетворити order на тип OrderDto.
І це, як на мене, геть суттєвий недолік, який зводить нанівець всі плюси AutoMapper’а. Деякі подробиці можна подивитись тут docs.automapper.org/...​test/Getting-started.html
В останньому своєму проекті я не використовую AutoMapper, просто роблю необхідні методи або lambda-expressions в допоміжних класах і інжектю їх у конструктори класів, де вони використовуються у звичайний спосіб. За допомогою generic інтерфейсів. Як на мене, такий підхід працює набагато краще, бо тепер компілятор бачить все що потрібно, і єдине, що я можу забути — це зареєструвати відповідний інтерфейс. Але це вилезе при першому запуску аплікації, а не в момент виклику маппінгу.

Я не зрозумів, чому ти кажеш про статичну типізацію, тому що Automapper це не про приведення типів, а про копіювання даних (тільки що поліз в його сорси, щоб впевнитися, швидко оглянув — схоже він працює на рефлексії, тобто це насправді про копіювання). Тепер впевнився, що ти тільки про реєстрацію мапінгів і їх перевірки на етапі компілювання наі що я і відповів, що перевіряю юніт тестами). Але ти насправді прав в тому, що винести цю перевірку на етап компіляції (а не в юніт-тести, які по суті той же рантайм) — це класно. А можеш показати трохи кода, як це робиш? Це для кожного мапінга треба реалізовувати цей дженерік-інтерфейс, описувати там мапінг і його реєструвати, так?

>>

А можеш показати трохи кода, як це робиш

Приклад інтерфейса:


/// <summary>
/// Create/Update sub-request converter 
/// </summary>
/// <typeparam name="TRequest">Sub-request data type</typeparam>
/// <typeparam name="TModel">Appropriate domain sub-model data type to convert to</typeparam>
public interface IRequestConverter<TRequest, TModel>
{
    TModel ToDomainEntity(TRequest request);
}
Приклад власно конвертера.

public class JobDefinitionSearchRequestConverter : IRequestConverter<JobDefinitionSearchRequest, JobDefinitionSearchConditions>
{
     public JobDefinitionSearchConditions ToDomainEntity(JobDefinitionSearchRequest request) => new JobDefinitionSearchConditions
    {
        TenantId = Guid.Parse(request.TenantId),
        TypeId = string.IsNullOrEmpty(request.TypeId) ? null : Guid.Parse(request.TypeId),
        States = request.States,
        HasParent = request.HasParent,
        ParentId = string.IsNullOrEmpty(request.ParentId) ? null : Guid.Parse(request.ParentId),
        Query = request.Query,
        OrderBy = request.OrderBy,
        Ascending = request.Ascending,
        Size = request.Size ?? 10,
        Page = request.Page ?? 1
     };
}
Реєструється як Singletone. Все. Automapper не потрібен. Звісно, бувають випадки коли мапити треба під час фетчу з IQueriable, тоді замість метода використовується проперть типу Expression<Func<TSource, TDest>>. Принцип той самий. І все типобезпечно.

Головна мета оцих різних маперів — це зменшити цей об’єм коду, який вони мапять автоматично. В цілому, звичайно, можна всунути якраз мапінг automapper`а в цей метод JobDefinitionSearchRequestConverter  і таким чином все не будемо стикатися з випадками незареєстрованого профайла автомапера в рантаймі.

Головна мета оцих різних маперів

Я нічого не казав про «оці різні мапери», тільки про AutoMapper, бо я ним активно користувався більше року на доволі великому проекті і маю певний досвід

— це зменшити цей об’єм коду, який вони мапять автоматично.

Саме так. І це не таке вже просте питання, чого від цього автоматичного мапінгу більше, зиску чи геморою. Наприклад у разі використання автоматичного мапінгу через reflection можна легко внести помилку в код просто перейменуванням проперті, або зміною її типу. Причому виявиться она в рантаймі, і не факт, що під час мапінгу. Може у довільному місці вилезти щось на кшталт null reference exception. Стикався кілька разів, як на мене, воно того не варто. Тобто якщо треба швидко створити MVP, тоді можливо й є сенс, хоча, з появою GitHub Copilot, навіть для MVP дуже і дуже сумнівно.

Це звісно моя особиста суб’єктивна думка, я нікому нічого не нав’язую. З мого досвіда використання AutoMapper і взагалі бездумне використання reflection заради «економії кода» — це антипатерн. Звісно можна зробити те саме меншою кількістю коду, але платня за це — його крихкість. Як на мене, будь-що, що суттєво зменшує maintenability не варто використання.

Я нічого не казав про «оці різні мапери», тільки про AutoMapper, бо я ним активно користувався більше року на доволі великому проекті і маю певний досвід

Це не ти казав, це я так підсумував, що ціль усіх маперів — уникнути кода на кшалт:

        Query = request.Query,         OrderBy = request.OrderBy,         Ascending = request.Ascending,

тобто того, що можна замапити автоматично.

Звісно можна зробити те саме меншою кількістю коду, але платня за це — його крихкість. Як на мене, будь-що, що суттєво зменшує maintenability не варто використання.

чесно сказати, я і сам дотримуюсь «філософії», щоб «писати більше коду, але підвищити стабилити коду», а на практиці цього коду реально стає дуже багато... І я знову повертаюсь до цього автомапера і пишу юніт-тести за допомогою чатжпт (він з цим справляється на ура). Але все одно, ці тести треба підтримувати. Вон може й захищають від перейменування тільки в одній сущності з тих, що мапляться, але не захищає від додавання нової. Тобто треба лізти в код юніт-теста і додавати поле, не зробив це — тест все одно зелений :(
В цілому, проблема все одно має місце бути...

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

повністю згоден! Буває (і не так уж і рідко) навіть навпаки — чим менше коду, тим вище ризик.

Дублювати код це круто, завжди дублюйте код, але тільки там де треба.

Там де не треба, не дублюйте.

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

мене здається що ви паритесь по мелочам

не зрозумів, по яким дрібницям я парюсь, ну да ладно :)

Якщо чесно, то це якісь два дуже дивні приклади.
Дублювання як на мене, це про ідентичні частини коду які виконують і один і той самий фукціонал.

Чому дивні? Хіба приклад про алгоритми поділу бактерій та предків не про ідентичні частини коду? Дуже прості приклади і треба було вивалити якийсь шматок кода, в який треба півдня врубатись? :)

Це не алгоритм, а просто формула. Людина яка вміє у common sense таке точно не буде рефакторити.

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

Приклад, насправді, дивний. У реалізації алгоритмів ідентичні частини коду часто корисно виділяти у функції. Нагадаю, що в .NET немає операції зведення в ступінь. Оператор ^ виконує виключну диз’юнкцію. Тому коректний приклад виглядатиме так:

public int GetBacteriaCount(int generation)
{
            return Math.Pow(2, generation);
}
public int GetAncestorsCount(int generation, double k)
{
            return k * Math.Pow(2, generation);
}
public async Task<int> GetAncestorsCountAsync(int generation)
{
                var k = await _extService.GetCoefficientAsync(generation);
                return k * Math.Pow(2, generation);
}
Цей приклад наочно показує, що всі ідентичні частини зведені якраз у бібліотечній функції Math.Pow.

ось за це дякую, писав начебто псевдокод, але потім лохматив під C#, в результаті вийшла каша :) Зараз підправлю!

Дякую за допис! Часто спілкуюсь про DRY на співбесідах і обговорюю з кандидатами їхнє розуміння цього принципу. Як підмітив один з авторів, який сформулював цей принцип, Dave Thomas, що «DRY немає нічого спільного з кодом, він відноситься до знань». І якщо поглянути на принцип з цієї точки зору, що «не повторюй знання», то все стає на свої місця, і конфлікт «об’єднувати цей код чи ні» майже зникає.

Дякую за оцінку. «не повторюй знання» реально звучить влучно для опису DRY

О, щасливі люди, у яких дто і вьюмодел в одній екосистемі і під повним контролем команди. У нас, гівнокодерів, таких питань нема. Де виклик віддаленого сервісу-там межа домену. Перетворення на межі домену — окрема модель, бо треба відділити серіалізацію від логіки. Виходить, в типовій трубі даних від трьох до безкінечності майже однакових моделей: серіалізація на вході(ах), доменна модель, серіалізація на виходах. Зазвичай стогін апологетів дядька боба закінчується після зміни контракту одним з десятка консьюмерів / продьюсерів

Зазвичай стогін апологетів дядька боба закінчується після зміни контракту одним з десятка консьюмерів / продьюсерів

А до чого тут це? В clean architecture від того ж Мартіна так й написано, в нас одна модель для домену й купа дто для взаємодії з клієнтами, консьюмерами, продюсерами. Як раз на випадок зміни контракту, версіонування, відокремлення формату даних від доменної моделі.

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

Під фанатами клін коду мав на увазі любителів розгалуженого наслідування дата класів щоб позбавитися повторення коду

Але оце

розгалуженого наслідування дата класів щоб позбавитися повторення коду

Немає жодного відношення до оцього:

Перетворення на межі домену — окрема модель, бо треба відділити серіалізацію від логік

В принципі, можна використати доменні моделі в інфраструктурному/транспортному рівні, для того щоб зекономити час, або просто влом писати ДТО, по факту просто відкласти написання ДТО до часу коли це дійсно знадобиться, тобто будуть зміни в моделі. Й це теоретично все ще буде clean architecture, бо напрямок залежності domain<-service-<infrastructure зберігається.

Але наслідувати транспортну ДТО модель від доменної моделі — це моветон, й точно не клін код. Фанати клін коду по яких ви судите — не розбираються в цьому клін коді, ось й все )

Але наслідувати транспортну ДТО модель від доменної моделі — це моветон

я навІть би ще додав, що ДТО — взагалі не об’єкт ООП по суті. Це просто структура даних. Тому вона не може наслідуватися в повному розумінні цього слова взагалі і особливо від доменної (яка може мати поведінку і стан, ДТО нічого не має в принципі)

ЗІ. Хоча використання наслідування щоб тільки уникнути дублювання пропертів для ДТО — як на мене це ок. Але не це наслідування поведінки, це не використання поліморфізму в наслідок наслідування і т.д.

В принципі, можна використати доменні моделі в інфраструктурному/транспортному рівні, для того щоб зекономити час, або просто влом писати ДТО, по факту просто відкласти написання ДТО до часу коли це дійсно знадобиться, тобто будуть зміни в моделі. Й це теоретично все ще буде clean architecture,

Ну, як мінімум тут буде порушення SRP (Single Responsibility Principle), так що вже не клінкод

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

тут би я посперечався, як на мене, він один з найважливіших (після, хіба що Dependency Inversion Principle) :)

Я згоден що він один з найважливіших, але це не робить його не розмитим ) Усі визначення які є для нього не дозволяють написати код який однозначно б не порушував би цей принципі. Завжди можна найти таке формування responsibility та/або actors при яких принцип не буде виконуватись, або буде. Це досить часто обговорювалось в коментарях й на редіті, й на доу, та усюди. В нашому прикладі про ДТО перше що в голову лізе, два варіанти:
1. Контроллер забезпечує формат даних, версіонування та мапінг з доменної моделі. Тоді так, при відсутності ДТО принцип порушується, бо зміна в домені одразу прокидується в формат даних.
2. Контролер віддає моделі as is, а серіалізація робиться за допомогою JSON фреймворка й його налаштувань. Будь які зміни в домені мають одразу відображатись в API (наприклад в нас просто CRUD), тобто для нас це бажана поведінка — й з такої точці зору вже нічого не порушується.

останні кілька разів у нашій тімці просто були використанні protobuff-models всюди, окрім бази. Для невеликих і сервісів працювало як треба. Головняка по підтримці в рази менше при зміні контракту аніж купа ДТОшек

Протобуф також дто для мене. Глобально — опис транспортного контракту на межі домену. Мова — вторинна.

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

Панове, а чому би не використовувати кодогенерацію + partial класи для розширення потрібних view models?

1. Не потрібно буде вручну дублювати код.
2. Не буде змішування відповідальності між dto та view models.

Не зрозумів, як це кодогенерацію + partial класи? А якщо щось треба змінити в view models, а не розширити? Лізти в кодогенерацію і partial класи і там змінювати?

Хіба Mapster не спрощує цю проблему?

Спрощує. Але Євген казав про кодогенерацію і паршіал класи, а не про автомапінг :) Ось мені цікаво, як перші 2 вирішують проблему.

А чому не можна щоб ClientVm приймав єдиний параметр ClientDto?

тому що, по-перше, ClientVm починає залежити від ClientDto, по-друге, як це вирішує проблему з дублюванням полів? Чи Ви хочете взагалі брати поля з ClientDto, в такому випадку спитайте себе, навіщо Вам взагалі потрібен тоді ClientVm, може, тоді напряму використовувати ClientDto (що, на мій погляд, порушує все і вся)

Чи повинна бути заборона на те, щоб Vm залежав від Dto? Можливо, але не очевидно що поганого у цьому буде, та чи порушує це якісь принципи. Dto — дата трансфер об’єкт — ідеальна назва щоб жбурляти купку даних поміж шарами коду, і куди завгодно.

Вирішує проблему дублювання так, що його не буде. Навіщо Vm — він каже від яких даних залежимо — від такого-то Dto, їх може бути декілька, плюс додаткові параметри. Якщо уявити собі Vm з 5’ма Dto, по 10 полів у кожному, ніби стає очевидно наскільки це буде легше читати, бачити що звідки береться, та підтримувати, ніж такий Vm з 50ма полями.

Можливо я поза контекстом, не c#-ер, міркую із загального розуміння. Та й Vm ніколи не використовував) Тому цікаво навіщо в c# копіюють з одного класу всі поля в інший, коли можна сам об’єкт передати.

Чи повинна бути заборона на те, щоб Vm залежав від Dto? Можливо, але не очевидно що поганого у цьому буде, та чи порушує це якісь принципи.

В теорії тому що одне від одного повинно залежити тоді, коли воно насправді має зв’язок. Vm використовується для presentation і, по-хорошому, не повинно залежити від чогось там ще. Йому прийшли якійсь дані, воно їх тільки «показує». І фіолетово, прийшли з цього шару бізнес-логіки або ще відкіля.
На практиці це буде неможливість використати Vm без Dto, неможливість написати тести без залежності від сборок, де описані ці Dto, це створює нестабільність Vm, тому що зміни в Dto можуть призвести до того, що Vm поламається, тобто нам потрібно проводити регресивне тестування кожен раз (і мати 100% покриття тестами), коли ми змінюємо сборки з Dto або суміжні з ними. Хоча, чесно кажучи, ця залежність (Vm від бізнес-логіки) лайтова і, в принципі, дозволена (принцип Принцип стійких залежностей SDP каже нам, що менш стійкі, якими є presentation, повинні залежити від більш стійких, яким є бізнес-логіка, тобто тут цей принцип виконується)

Можливо я поза контекстом, не c#-ер, міркую із загального розуміння. Та й Vm ніколи не використовував) Тому цікаво навіщо в c# копіюють з одного класу всі поля в інший, коли можна сам об’єкт передати.

C# тут взагалі ні до чого.

ЗІ. Повторюсь, я прихильник стійкої архітектури, тому із двух зол (дубльовані поля чи перемішані кордони між зонами відповідальності) я обираю дубльовані поля :)

Дякую за роз’яснення, бачу ідею.

Можливо, воно і так і так правильно.
Коли presentation дійсно залежить від конкретної сутності, то можна передавати об’єктом.
А якщо це presentation для перевикористання у різних контекстах, то ні до чого конкретного не чепляємо, лише що такі-то дані очікує.
ClientVm звучить як спеціальний одноразовий випадок для сутності клієнта, я б не став його перевикористовувати для чогось іншого.

Вони в різних сборках? Я так уявив що класи с такими подібними іменами лежать поруч і немає проблем щоб зробити Dto у тесті.

Якщо Dto несумісно зміниться, то воно у будь-якому випадку десь зламається.

Можливо, воно і так і так правильно.

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

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

Вибачте, не зрозумів. Чому якщо залежить від конкретної сутності, то можна передавати об’єктом?
Причому тут перевикористання presentation у різних контекстах? Це коли таке буває?

ClientVm звучить як спеціальний одноразовий випадок для сутності клієнта, я б не став його перевикористовувати для чогось іншого.

А які ще класи Ви перевикористовуєте для чогось іншого? Клас повинен використовуватися тільки для однієї мети, через що він буде мати тільки одну причину для змін (Single Responsible Principle)

Вони в різних сборках? Я так уявив що класи с такими подібними іменами лежать поруч і немає проблем щоб зробити Dto у тесті.

Як на мене, це взагалі Must Have. Я можу привести мабудь з десяток аргументів, чому Must Have, але найважніший (і з якого всі інші так чи інакше «витікають») — бізнес-логіка набагато більш стабільна, ніж presentation і її не повинно ніяк чіпати те, що коїться і змінюється в presentation

Якщо Dto несумісно зміниться, то воно у будь-якому випадку десь зламається.

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

так ClientDto — бізнес-сутність. Хіба не логічно його передавати у в’юху і використовувати де треба?

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

Дякую за статтю !! Згодний що не завжди треба позбуватися дублікатів. Як раз нещодавно мав кейс, де вважав що краще той кусок коду продублювати. Не хотілося розвивати дискусію за то, тому післе декількох спроб переконати ревьювера більше не став і таки виніс дублюючу логіку в окремий метод )).

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

А чому б не засунути DTO у abstract

ClientBaseModel<T> where T: BaseDTO { T Dto { get; set; } }
? На моїй практиці це був найзручніший варіант:
  • Зменшення дублювання коду: Немає потреби дублювати усі поля або частину з них у клієнтських моделях, що значно спрощує підтримку коду.
  • Доступ до оригінальних даних: Завжди є доступ до оригінальних даних з DTO, що може бути корисно, наприклад, у системах редагування даних. Це дозволяє автоматично реалізувати функцію скасування (cancel) та перевіряти, чи були внесені зміни.
  • Гнучкість у створенні UI моделей: Поля в клієнтській моделі створюються лише ті, які потрібні для відображення в UI, що дозволяє застосовувати обробку даних, потрібну для інтерфейсу користувача.
  • Автоматична валідація під час компіляції: Під час компіляції відбувається автоматична валідація, що знижує ризик помилок при зміні моделі. У випадку сліпого маппінгу, якщо не налаштовані жорсткі правила або додаткові засоби валідації, можна легко прогавити зміни.
  • Перевикористання існуючих класів: Існуючі інстанси класів, які генеруються під час парсингу об’єкту будь-якого формату, можуть бути вбудовані у відповідний інстанс клієнтської моделі, що наслідується від абстрактного ClientBaseModel<T>. Це підвищує продуктивність і забезпечує централізоване керування логікою.
  • Використання DTO напряму: У випадку, якщо деякі моделі дозволяють, їх можна показувати напряму у інтерфейсі/кліенті без створення проміжного класу. Наприклад, умовна метадата медіафайлу.

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

Щось я трохи не зрозумів, що таке BaseDto ? Він базовий і для бізнес-моделі, і для веб? Якщо так, то в чому різниця, в Вашому випадку наслідування (начебто так краще звучить, ніж «спадщина»? :) ) просто замінена на асоціацію. Я бачу тут ті самі проблеми, щр і описані мною в статті. Також якщо Ви захочете відв’язати фронт від цієї бізнес-логіки і прив’язати до іншої (що, правда, доволі рідко буває потрібно :) ), то у Вас це не вийде.
В цілому в статті, я хотів сказати, що на мою думку, краще зв’язувати ті речі, які повинні бути зв’язані і навпаки, відв’язувати ті, які можуть існувати окремо одна від одної. Так, це може тягнути за собою якійсь додаткові затрати, але це не порушує логіку взаємодії. А як показує мій доволі глибокий досвід, це — проблема всіх проблем, особливо в майбутньому, що призводить до майже неможливості відкорегувати архітектуру без значних змін.
А взагалі, покажіть трохи більше кода будь-ласка, я не впевнений, що я правильно зрозумів Ваш підхід. Дякую

Підписатись на коментарі