Пишемо Builder із валідацією обов’язкового виклику методів

Вітаю, шановне товариство. Мене звати Євген, я Senior C++ Engineer в компанії Intellias. У вільний від роботи час я люблю експериментувати й займатися пошуком рішень, що підвищують надійність коду.

Доволі часто в мене виникало питання стосовно того, як можна покращити усім відомий патерн проєктування Builder. На відміну від звичайного конструктора, Builder дозволяє створювати обʼєкт покроково. Це накладає свої обмеження — як зрозуміти, що необхідні методи Builder’а були викликані та наявні всі значення для конструювання?

Найбільша технічна конфа ТУТ!🤌

Хоч на перший погляд питання звучить тривіально, подумайте про випадки, коли встановлення значень у Builder розпорошені по довгому методу з купою умовних переходів або взагалі викликаються з різних методів.

Гарантія, що всі необхідні методи викликані, у цьому випадку повністю лягає на плечі розробника. Звісно, можна себе убезпечити, написавши тести, що автоматизують перевірку всіх (чи майже всіх) випадків використання Builder’а. І так треба робити, але чи є ще варіанти?

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

Перш ніж почати

Пропоную одразу працювати з чимось конкретним і ввести доволі простий клас Car, якого доля та ми обрали терпіти всі наші експерименти. Клас матиме наступний вид:

class Car {
public:
  const std::string &GetBrandName() const { return brand_name_; }
  const std::string &GetModelName() const { return model_name_; }
  unsigned GetProductionYear() const { return production_year_; }
  unsigned GetMileage() const { return mileage_; }
  unsigned GetMaxFuelLevel() const { return max_fuel_level_; }
  unsigned GetCurrentFuelLevel() const { return current_fuel_level_; };

  void SetCurrentFuelLevel(unsigned new_current_fuel_level) {
	assert(new_current_fuel_level <= max_fuel_level_ &&
       	"New current fuel level cannot exceed maximum fuel level.");
	current_fuel_level_ = new_current_fuel_level;
  }

private:
  std::string brand_name_;
  std::string model_name_;
  unsigned production_year_{0u};
  unsigned max_fuel_level_{0u};
  unsigned mileage_{0u};
  unsigned current_fuel_level_{0u};
};

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

Звичайний Builder

Звичайний Builder поступово накопичує в собі необхідні дані: чи то в окремих змінних, чи одразу в об’єкті (завдяки C++ ідіомі friend).

Додамо клас CarBuilder:

class CarBuilder {
public:
  CarBuilder &WithBrandName(const std::string &brand_name) {
	car_.brand_name_ = brand_name;
	return *this;
  }

  CarBuilder &WithModelName(const std::string &model_name) {
	car_.model_name_ = model_name;
	return *this;
  }

  CarBuilder &WithProductionYear(unsigned production_year) {
	car_.production_year_ = production_year;
	return *this;
  }

  CarBuilder &WithMileage(unsigned mileage) {
	car_.mileage_ = mileage;
	return *this;
  }

  CarBuilder &WithMaxFuelLevel(unsigned max_fuel_level) {
	assert(max_fuel_level != 0 && "max_fuel_level_ must be bigger that zero");
	car_.max_fuel_level_ = max_fuel_level;
	return *this;
  }

  CarBuilder &WithCurrentFuelLevel(unsigned current_fuel_level_) {
	car_.current_fuel_level_ = current_fuel_level_;
	return *this;
  }

  Car Build() {
	return car_;
  }

private:
  Car car_;
};

І оновимо клас <code>Car</code>:

class CarBuilder;

class Car {
public:
	// ...

private:
	friend class CarBuilder;
	Car() = default;

	std::string brand_name_;
	// ...
};

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

Ось доволі примітивна перевірка на наявність необхідних даних для коректної побудови об’єкта:

Car CarBuilder::Build() {
	assert(!car_.brand_name_.empty() && "car_.brand_name_ is not set");
	assert(!car_.model_name_.empty() && "car_.model_name_ is not set");
	assert(car_.production_year_ != 0 && "car_.production_year_ is not set");
	assert(car_.max_fuel_level_ != 0 && "car_.max_fuel_level_ is not set");

	return car_;
}

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

class Car {
// ...
private:
	bool was_ever_used_{false};
};

class CarBuilder {
public:
	// ...
	CarBuilder& WithWasEverUsed(bool was_ever_used){
    	car_.was_ever_used_ = was_ever_used;
    	return *this;
	}
};

У цьому випадку, в разі, коли ми конструюємо об’єкти, що відображають вживані автомобілі, як зрозуміти, чи був викликаний метод WithWasEverUsed(bool), чи збереглося значення за замовчуванням {false} з декларації класу? Для цього доведеться в CarBuilder додавати проміжну змінну типу bool, яка відображатиме розуміння, чи встановлювалось необхідне значення:

class CarBuilder {
public:
	// ...
	CarBuilder& WithWasEverUsed(bool was_ever_used){
    	car_.was_ever_used_ = was_ever_used;
    	is_set_was_ever_used_ = true;
    	return *this;
	}

private:
	// ...
	bool is_set_was_ever_used_{false};
};

Тоді можемо зрозуміти консистентність накопичених значень у Builder’і:

Car CarBuilder::Build() {
	// ...
	assert(is_set_was_ever_used_ != false && "car_.was_ever_used_ is not set");

	return car_;
}

Виглядає перенавантажено. Спробуємо інший підхід.

Покращений Builder

Цей підхід являє собою комбінацію звичайного Builder’а з enum та bitset. Така зв’язка допомагає автоматизувати перевірку на встановлення необхідного значення.

Додаймо enum з назвою MandatoryValues, в якому вкажемо обов’язкові значення, відсутність яких не дозволяє правильно сконструювати об’єкт:

class CarBuilder {
// ...
private:
  enum MandatoryValues {
	kBrandName = 0,
	kModelName,
	kProductionYear,
	kMileage,
	kMaxFuelLevel,

	Count
  };
  std::bitset<MandatoryValues::Count> mandatory_values_;
// ...
};

Можна побачити, що останнім елементом енума MandatoryValues є Count. Він відповідає за кількість необхідних значень і використовується при ініціалізації бітсету mandatory_values_. Під час перевірки наявності всіх параметрів Count не враховується, головне завжди тримати його останнім.

Подивимось, як нові зміни впливають на CarBuilder. До кожного методу With...(), який встановлює обов’язкове значення, треба додати встановлення в бітсет відповідного значення енума, а в методі Build() викликати вбудовану перевірку встановлених значень: std::bitset<>::all().

class CarBuilder {
public:
  CarBuilder &WithBrandName(const std::string &brand_name) {
	car_.brand_name_ = brand_name;
	mandatory_values_.set(MandatoryValues::kBrandName);

	return *this;
  }

  CarBuilder &WithModelName(const std::string &model_name) {
	car_.model_name_ = model_name;
	mandatory_values_.set(MandatoryValues::kModelName);

	return *this;
  }

  CarBuilder &WithProductionYear(unsigned production_year) {
	car_.production_year_ = production_year;
	mandatory_values_.set(MandatoryValues::kProductionYear);

	return *this;
  }

  CarBuilder &WithMileage(unsigned mileage) {
	car_.mileage_ = mileage;
	mandatory_values_.set(MandatoryValues::kMileage);


	return *this;
  }

  CarBuilder &WithMaxFuelLevel(unsigned max_fuel_level) {
	assert(max_fuel_level != 0 && "max_fuel_level_ must be bigger that zero");

	car_.max_fuel_level_ = max_fuel_level;
	mandatory_values_.set(MandatoryValues::kMaxFuelLevel);
    
	return *this;
  }

  CarBuilder &WithCurrentFuelLevel(unsigned current_fuel_level_) {
	car_.current_fuel_level_ = current_fuel_level_;
	return *this;
  }

  Car Build() {
	assert(mandatory_values_.all() &&
     	"Not all mandatory values were set for car creation.");

	mandatory_values_.reset();
 
	return car_;
  }

private:
  enum MandatoryValues {
	kBrandName = 0,
	kModelName,
	kProductionYear,
	kMileage,
	kMaxFuelLevel,

	Count
  };
  std::bitset<MandatoryValues::Count> mandatory_values_;

  Car car_;
};

Тепер ми можемо бути певні, що перед викликом методу Build() всі значення встановлені. У разі, якщо це не так — спрацює assert.

Додамо додатково вивід усіх обов’язкових значень і замінимо assert на більш стандартний підхід з std::exception:

Car Build()
{
	if (!mandatory_values_.all())
	{
    	std::string mandatory_values_as_str = mandatory_values_.to_string();
    	std::reverse(mandatory_values_as_str.begin(), mandatory_values_as_str.end());

    	mandatory_values_.reset();

    	throw std::runtime_error(
        	"Not all mandatory values were set for car creation. Mandatory values are: " +
        	mandatory_values_as_str);
	}

	mandatory_values_.reset();
	return car_;
}

Висновок

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

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

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

Дякую за ваш час та увагу.

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

👍ПодобаєтьсяСподобалось4
До обраногоВ обраному1
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

Спробую використати цей коментар аби пояснити мотивацію написання статті і свою точку зору.

Ідея написання статті виникла під час роботи з legacy кодом, у якому доступ до конструкторів класів є закритим і сконструювати об’єкт можливо тільки за допомогою відповідних Builder’ів. Ба більше, модифікувати Builder’и можливо, а класи — ні, бо вони є частиною зовнішнього API. У статті я розповідаю про підхід, який, на мою думку, полегшує перевірку правильності ініціалізації всіх необхідних значень.

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

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

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

Гарного дня! 😊

Щось ніхто не може знайти приклад, де саме потрібен білдер. Ось приклад: API функція відкриття файлу приймає 15 параметрів, з них зазвичай потрібно 4-5:

NTSTATUS FLTAPI FltCreateFileEx(
  [in]           PFLT_FILTER        Filter,
  [in, optional] PFLT_INSTANCE      Instance,
  [out]          PHANDLE            FileHandle,
  [out]          PFILE_OBJECT       *FileObject,
  [in]           ACCESS_MASK        DesiredAccess,
  [in]           POBJECT_ATTRIBUTES ObjectAttributes,
  [out]          PIO_STATUS_BLOCK   IoStatusBlock,
  [in, optional] PLARGE_INTEGER     AllocationSize,
  [in]           ULONG              FileAttributes,
  [in]           ULONG              ShareAccess,
  [in]           ULONG              CreateDisposition,
  [in]           ULONG              CreateOptions,
  [in, optional] PVOID              EaBuffer,
  [in]           ULONG              EaLength,
  [in]           ULONG              Flags
);
З білдером це виглядає наступним чином:
status = FileUtil::openFile()
    .withFilter(filter)
    .withPath(path)
    .withDesiredAccess(FILE_READ_ATTRIBUTES)
    .withFileObject(newFileObject)
    .execute();

Ваш приклад не надає мотивації, чому ви це не можете зробити в конструкторі.

Я можу зробити, скажімо 10 конструкторів, що покриють більшість кейсів. Але не впевнений, що це буде зручніше. Треба вибрати правильний конструктор (а може і немає такого варіанту, який зараз треба — але я це дізнаюсь тільки якщо переберу всі наявні конструктори) та передати параметри у встановленому порядку. І ще добре, коли параметри мають різні типи. А коли це прапорці в вигляді ULONG — їх легко переплутати місцями. Та й читати код важче: оцей ноль в FltCreateFileEx(..., 0, ...) - це ShareAccess чи CreateOptions чи ще щось?

Плюс білдер дозволяє зробити наступне:

auto openFileBuilder = FileUtil::openFile()
    .withFilter(filter)
    .withPath(path)
    .withDesiredAccess(FILE_READ_ATTRIBUTES)
    .withFileObject(newFileObject);
    
if (...)
{
    openFileBuilder
        .withEaBuffer(buffer)
        .withEaLength(bufferLength);
}

status = openFileBuilder.execute();

Дозвольте додам свої п’ять копійок з Java боку.
Про Lombok @Builder.

TL; DR.

Q: How to distinguish a good builder and a bad builder?

A: It’s easy. The good one should return a fully initialised object in any case. Assume we provide a carBuilder for a rental agency. If a customer just wants to drive and doesn’t have any special requests, the carBuilder.build() should provide them some car that is ready to go, i.e. the car.drive() does its job and doesn’t throw any exception.

Якщо білдер не гарантує compile check що обʼєкт буде повністю ініціалізовано — не треба такий білдер. Є конструктор(и).

Трохи нижче пропонувалась така ідея зі сторони Rust. Дійсно треба спробувати імплементувати, дякую за влучну підказку!

Так з боку Rust якась крута compile магія пропонувалась. Нажаль, я не знаю, як таке в Java зробити. Боюсь, що ніяк.

Тому мій поінт набагато простіший — не можна білдер використовувати. Ну, окрім хіба що випадків request-а чи fluent filter-a, про що я намагався пояснити в статті.

Ну, з тим, що не можна використовувати Builder я не погоджуюсь, адже це давно відомий паттерн проектування і багато матеріалу існує, який доводить його користь.
Але Ваш коментар надихнув мене покопати глибше в сторону compile-time check’ів, тому, скоріше за все, буде друга частина цієї статті :)

Дозвольте я тоді так посилання додам gist.github.com/...​c0ec5112a226e83e633d90200

Ще так вас спробую переконати.

З одну боку тяжіє

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

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

Але захисту від дурня, нажаль, немає.
Яким чином ви би не робили валідацію в методі build() — чи перевіряючи усі поля, чи якось за допомогую bitmap-и, чи ще якось — в любому випадку
1) це прикласти нормально так зусиль
2) а головне — це runtime check.

Якщо в клас додати нове обов’язкове поле, те щось десь ініціалізація об’єкту десь зламалась — ви, в кращому випадку, це тільки на тестах дізнаєтесь (а в гіршому — на продакшені).

А я хочу compile check — щоб одразу проект переставав збиратися. Бо це єдина перевірка в яку я вірю.

Якщо Rust дозволяє якусь compile магію — дуже радий за цю мову.
Для всіх інших пересічних — тільки конструктори.

C++ теж дозволяє щось подібне, копаю в цю сторону далі для нової реалізації та нової статті

Звісно що, стосуватиметься вона C++, я не сильний в Java

Окей, допустимо у нас немає варіанту міняти сам Сar клас, щоб зробити необхідні поля аргументами конструктора і не дати створити невалідний клас.

Що ж ми зробимо?

Варіант 1:
зробити один assert на кожну умову в конструкторі білдера перед тим як віддати зібраний клас.

Ні, це складно і незрозуміло!

Варіант 2:
ввести паралельний список необхідних полів в вигляді еnum, на рівні контракту білдера вимагати логувати додавання полів в окремій колекції в відповідних методах білдера, навернути маніпуляцій з паралельними даними на третину екрану щоб вивести фенсі лог, в процесі загубити перевірку current_fuel_level_ <= max_fuel_level_ бо замість сетера використав плюсові shenanigans

Ось це вже краще!

Варіант 1:
зробити один assert на кожну умову в конструкторі білдера перед тим як віддати зібраний клас.

Це ж пропонується у розділі «Звичайний Builder» і далі поступово описується мотивація чому перевірки у методі Build() можуть вилитись у комбінаторний хаос.

на рівні контракту білдера вимагати логувати додавання полів в окремій колекції в відповідних методах білдера

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

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

Тут немає маніпуляцій з паралельними даними, Builder напряму працює з полями класу завдяки ідіомі

friend
в процесі загубити перевірку current_fuel_level_ <= max_fuel_level_

Дякую, виправлю :)

Я бы сказал, что это анти-паттерн. За всю карьеру ни разу не было необходимости городить что-то подобное.
У класса Car должен быть единственный конструктор, который принимает класс или структуру CarParams, в которой уже и описываются все необходимые танцы с бубнами.
Плюс такого подхода — у вас не будет существовать объекта Car в недоинициализированном, не готовом к использованию, состоянии. Ибо это крайне хреновый инвариант, который, кстати, порождается еще одним анти-паттерном — двухфазным конструированием.

У класса Car должен быть единственный конструктор, который принимает класс или структуру CarParams, в которой уже и описываются все необходимые танцы с бубнами.

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

Car car_;

если что, вот это и есть объект в нерабочем состоянии

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

пффф... к нему прекрасно есть доступ из билдера
и у тебя там сейчас с десяток public методов для инициализации
и рядом будет еще десяток для работы с «готовым» объектом
и ты прекрасно можешь написать код, который полезет в билдере не в тот метод

Це вже якісь фантазії, якщо чесно :)
Задача Builder’а допомогти сконструювати обʼєкт і валідувати його правильність. Навіщо Builder розширювати методами, котрі відповідають за якусь іншу логіку?

еще раз, на пальцах
— Car имеет public методы, которые отвечают за его приведение в готовность и методы, которые корректно использовать, только в состоянии готовности
— Car находится в классе Builder, где можно вызывать и те и другие методы и нет никакой защиты от дурака

если бы такого рода проблем не существовало, в языке не было бы защиты в виде protected/private, потому что все умные, никогда не будут ошибаться и всегда вызывать правильные методы в правильных состояниях

Наскільки я Вас зрозумів, Ви стверджуєте, що, умовно, ніхто ж не забороняє десь в Builder’і викликати метод Drive(), або LoadCargo() або ще якийсь. І це дійсно так.
Але якщо керуватись цією логікою, то так само треба переживати і за те, що в CarBuilder програміст напише

rocket.FlyToTheMoon()
Я розумію Ваші побоювання, проте не підтримую їх.
Builder покликаний для того аби сконструювати обʼєкт, не для того, аби викликати всі публічні поля та методи класу який він конструює.

Стаття не каже що замість конструктора чи звичайного Builder’а треба використовувати виключно описаний підхід. Вона описує ситуацію, коли конструйований обʼєкт містить багато полів які тісно між собою повʼязані і пропонує зручний механізм перевірки наявності значень у цих полів.
Я не пропоную у статті повністю перейти на описаний механізм. Здається, це цілком очевидно, що у більшості випадків такий підхід надмірний і для класу з 2-3 полів Builder не потрібен.

Я бы сказал, что это анти-паттерн. За всю карьеру ни разу не было необходимости городить что-то подобное.

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

Более продвинутая тема это builder с использованием typestate pattern, где компилятор сам проверяет, чтоб нужные поля были инициализированы, и если они не инициализированы, то будет ошибка компиляции (вместо проверки в ратнайме).

blog.ediri.io/...​ing-the-typestate-pattern

Цікаве рішення, треба буде спробувати імплементувати на C++

Проблема ассерту з бітсетом в тому що він не скаже які саме поля не були ініціалізовані. Можливо в вашому випадку це не критично, тому що функціі будуть викликатися поряд, але в загальному випадку, для дебага, особливо на проді коли steps to reproduce невідомі, краще бачити яке саме поле не було ініціалізовано, хоча б перше з них.

У методі

Build()
 є прінт всіх полів бітсету в ексепшн, це найпростіший і водночас не найдієвіший спосіб, проте інформації він додасть.
Я також додавав тести на солюшн, їх можна побачити [отут](github.com/...​ob/main/test.hpp#L99-L101)

Звісно, можна перегрузити

operator<< 
для цього енума, або використовувати [BetterEnum](aantron.github.io/better-enums), які з під капоту мають
to_string()
, але я подумав, що для прикладу поточного рішення буде достатньо.
й займатися пошуком рішень, що підвищують надійність коду.

👏

На відміну від звичайного конструктора, Builder дозволяє створювати обʼєкт покроково.

У вашому дописі я не побачив мотивації: а чому ви не можете зробити це у конструкторі? Перевірку усіх необхідних параметрів зробить компілятор, а чи там допустимі дані — assert.

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

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