Аргументы по умолчанию(мысли вслух)

Средний

7 мин

2.3K

ВНИМАНИЕ(ATTENTION)! - Хочу сразу сказать, что эта статья не о проблеме, а о моих размышлениях на тему аргументов по умолчанию.

Поэтому здесь нет стандартного сценария ПРОБЛЕМА -> РЕШЕНИЕ.

Тут сценарий такой - РАССУЖДЕНИЯ АВТОРА -> РАССУЖДЕНИЯ ЧИТАТЕЛЯ.

Поехали...

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

Стандартный поток обработки пользовательского запроса выглядит так:

Но "под капотом" он может разрастаться на большое кол-во объектов/механизмов:

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

Клиент заполняя форму оставляет пустым например поле подгруппа(subgroup) и на бек "летит" {..., subgroup: null}.
На бекенде этот json "встречает" например pydantic. Он как ни странно валидирует json и предоставляет нам объект(BaseModel) с доступом к полям через "." -> CreateServiceModel(...).subgroup == None.
CreateServiceModel мы просто "переливаем" в CreateServiceDTO, что бы в бизнес логику попал дистиллированный от всего лишнего объект.
В рамках БЛ наш CreateServiceDTO "переливается" в ServiceEntity, которую мы передаём в репозиторий для сохранения.
Репозиторий же "переливает" ServiceEntity в ServiceModel(Django).
Django перед сохранением непосредственно в персистентное хранилище "переливает" данные из ServiceModel в структуру, которая будет понятна драйверу базы данных.

Уф... Так много переливали, что прошли очередной уровень в популярной когда-то игре-рекламе😅:

Не смотря на то, что кол-во раз, которое нам пришлось "переливать" одни и те же данные велико, эта статья про другое. Прошу не фокусировать внимание на этом факторе.

А теперь давайте представим, что в текущем спринте нашей команде нужно реализовать эту фичу. Мы конечно же придерживаемся подхода изоляции слоёв и написания тестов на +- весь имеющийся код(функционал). Что в итоге у нас по задачам на спринт?

Будем считать, что разработчики +- одного уровня и слада ума.

Первый разработчик

Разработчик контроллера обсудил с фронтенд-разработчиком, что отправлять на бек несколько null(ов) нельзя - это будет медленнее и т. п. На том и порешали:

class CreateServiceModel(BaseModel):
    name: str
    group: str
    subgroup: str | None = None # -> Устанавливаем сами если поле не пришло
    service_type: str
    sub_type: str | None = None # -> Устанавливаем сами если поле не пришло
    action: ActionEnum
    period: int
    period_type: PeriodType

Это ещё не так смущает. Вот если задать значение по умолчанию для - action, period, или period_type, то кажется, что мы забираем часть ответственности клиента за чёткое понимание того, чего он хочет от нашей системы на себя, давая ему возможность не присылать ряд важных параметров, для БЛ.

Второй разработчик

На разработчике, которому нужно реализовать сценарий использования приложения лежит большая ответственность, ему нужно соблюсти много бизнес-правил и инвариантов:

Имя должно быть уникальным
Дата не должна быть в прошлом
Период не может быть больше 12
и т. д. и т. п.

Навскидку у него получилось с десяток различных тестов, что бы покрыть все варианты поведения UseCase. Уже на третьем тесте он подумал, что тратит много времени на заполнение всех полей классов(DTO и Entity) одинаковыми данными из раза в раз:

NOW = datetime(2024, 1, 1, 1, 1, 1)

@freezegun.freeze_time(NOW)
class TestCreateServiceUseCase:
    @freezegun.freeze_time()
    def test_execute(self, fake_service_repository: FakeServiceRepository) -> None:
        service_dto = CreateServiceDTO(
            name="Name",
            group="Group",
            subgroup=None,
            service_type="ServiceType",
            sub_type=None,
            action=AtionEnum.NEW,
            period=1,
            period_type=PeriodType.MONTH,
        )
        use_case = CreateServiceUseCase(fake_service_repository)
        
        got = use_case.execute(service_dto)
        
        assert fake_service_repository.get_by_name("Name") == ServiceEntity(
            name="Name",
            group="Group",
            subgroup=None,
            service_type="ServiceType",
            sub_type=None,
            action=AtionEnum.NEW,
            period=1,
            period_type=PeriodType.MONTH,
            created_time=NOW,
            updated_time=NOW,
        )

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

@dataclass
class CreateServiceDTO:
    name: str
    group: str
    subgroup: str | None = None
    service_type: str
    sub_type: str | None = None
    action: ActionEnum = ActionEnum.NEW
    period: int = 1
    period_type: PeriodType = PeriodType.MONTH


@dataclass
class ServiceEntity:
    name: str
    group: str
    subgroup: str | None = None
    service_type: str
    sub_type: str | None = None
    action: ActionEnum = ActionEnum.NEW
    period: int = 1
    period_type: PeriodType = PeriodType.MONTH
    created_time=field(default_factory=datetime.now))
    updated_time=field(default_factory=datetime.now))

В итоге он добился своей цели - его тесты стали короче.

NOW = datetime(2024, 1, 1, 1, 1, 1)

@freezegun.freeze_time(NOW)
class TestCreateServiceUseCase:
    @freezegun.freeze_time()
    def test_execute(self, fake_service_repository: FakeServiceRepository) -> None:
        service_dto = CreateServiceDTO(
            name="Name",
            group="Group",
            service_type="ServiceType",
        )
        use_case = CreateServiceUseCase(fake_service_repository)
        
        got = use_case.execute(service_dto)
        
        assert fake_service_repository.get_by_name("Name") == ServiceEntity(
            name="Name",
            group="Group",
            service_type="ServiceType",
        )

Но какой ценой? Теперь объекты приложения меняются для тестов.

Третий разработчик

Третий разработчик, который делал репозиторий пошёл дальше чем второй, и решил предусмотреть любой вариант развития событий для модели django😄:

class ServiceModel(Model):
    name = CharField()
    group = CharField()
    subgroup = CharField(null=True, default=None)
    service_type: str
    sub_type = CharField(null=True, default=None)
    action = CharField(choises=..., default=ActionEnum.NEW)
    period = IntegerField(default=1)
    period_type = CharField(choises=..., default=PeriodType.MONTH)
    created_time=DateTimeField(auto_now_add=True)
    updated_time=DateTimeField(auto_now=True)

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

Итог

Все разработчики закончили свою работу, фича работает корректно, поздравляю!

Но есть одно но... Всегда есть одно "но" )

Теперь сквозь всю фичу начиная с заполнения пользовательской формы и заканчивая попаданием услуги в базу данных мы имеем несколько опциональных параметров.

Контекст получен! Теперь по делу.

Что же меня смущает в параметрах по умолчанию, которые пронизывают всё приложение?

1. В боевом режиме они никогда не понадобятся.

Давайте согласимся с тем, что если наше приложение подразумевает БОЛЬШУЮ нагрузку по кол-ву запросов в секунду, то не посылать на бек поля с null наверное может немного помочь нам(в объекте может быть и сто полей).

Например как только в рамках валидации будут проставлены в None subgroup и subtype(если они не придут с фронта), то больше опциональность нигде не нужна.

2. Тесты портят дизайн приложения?

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

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

def make_service(
    name: str,
    group: str,
    subgroup: str | None = None,
    service_type: str,
    sub_type: str | None = None,
    action: ActionEnum = ActionEnum.NEW,
    period: int = 1,
    period_type: PeriodType = PeriodType.MONTH,
    created_time: datetime | None = None,
    updated_time: datetime | None = None,
) -> Service:
    return Service(
        name=name,
        group=group,
        subgroup=subgroup,
        service_type=service_type,
        sub_type=sub_type,
        action=action,
        period=period,
        period_type=period_type,
        created_time=created_time or datetime.now(),
        updated_time=updated_time or datetime.now(),
    )

3. DRY и SRP ушли... Не обещали вернуться...

Мы имеем Фичу для создания услуги. Кто решает какую услугу мы хотим создать? Ооо...

Клиент - отправит нам данные(может все может нет).
Валидатор - если часть данных не прилетела, то проставит None там где это необходимо.
DTO - Проставит None если вдруг не передали из валидатора. Также установит по умолчанию period и т. п.
Сущность - Проставит None если вдруг не передали из DTO. Также установит по умолчанию period и т. п. А ещё либо сущность либо ORM установят даты по умолчанию.
ORM - Проставит None если вдруг не передали из сущности. Также установит по умолчанию period и т. п. А ещё либо сущность либо ORM установят даты по умолчанию.
СУБД - Ну вы поняли)

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

По поводу SRP - притянул за уши конечно, но всё же каждый из уровней вашего приложения теперь имеет дополнительного актора(даже если это будет разработчик, а не менеджер), который может захотеть что-то поменять в этих параметрах. Ну и каждый уровень имеет доп. ответственность за эти параметры, если они не пришли с клиентской стороны. Поменять в одном месте точно не удастся :)

4. Ошибки разработчиков.

Что если кто-то при очередном "переливании" полей из одного объекта в другой не укажет например поле perod? Ничего страшного, ведь он проставится по умолчанию в значение 1. А тут уже надеемся что есть хоть один тест где период больше чем один, что бы заметить это до продакшена )

ServiceEntity(
    сreate_service_dto.service_type,
    # сreate_service_dto.subtype -> Проглядели и не добавили строку
    ...
)