Pull to refresh

Comments 23

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

Уважаю создателей rust, что сходу добавили фп возможности в язык. Хоть rust и не фп сам по себе, но задача из статьи будет выглядеть как на F#

Автор книги Роберт Мартин, а не Мартин Фаулер, это разные люди)

Действительно, грубая ошибка.

Примеры из жизни.

Первый. Клиент Вася выбирает место 25 для бронирования, клиент Петя выбирает место 25 для бронирования - оба видят экземпляры класса FreeSeat. Вася бронирует и получает подтверждение, Петя бронирует и тоже получает подтверждение. Как объяснить Васе, что в кино он сегодня не идёт?

Второй. Приложение работает в проде, но клиенту надо то же самое, только не для кинотеатра на 150 мест, а для стадиона тысяч на 15 человек. Логика бронирования и покупки аналогичная. Завели в БД места, но вот незадача - теперь для бронирования одного места мы выкачиваем из БД 15к строк. DBA заходит в наш офис, у него в руке дробовик.

Решения из жизни.

Первое. На каждый запрос открывается транзакция. Кто первый закоммитил - тот и будет смотреть. Поймали ConcurrencyException - говорим Васе, что в кино не пойдет.

Второе. Да, код не оптимален. Его основная задача не решение бизнес-задач, а существование в качестве примера. DBA не придет, можно спать спокойно.

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

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

Я думаю, что тут стоит признать, что независимо от того, что именно реализует хранилище (бд или не бд), загружать в память коллекции неизвестного размера полностью - моветон, из чего можно сделать вывод, что ошибка заключается именно в *неоптимальном алгоритме*, нежели в выборе способа проектирования.

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

А вы точно специалист?

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

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

По второму мне видится так. Будем сохранять не каждое место в строке, а каждый сектор с рядом (стадион по-любому делится на сектора и в каждом секторе присутствует ряд). 1ая колонка - номер/ид сектора, 2ая колонка - номер/ид ряда сектора. В 3ью колонку в формате json для удобного хранения и передачи в приложение храним список номеров мест в ряду и признак занято/не занято. Как только все места в ряду имеют признак занятости, на весь ряд в этой строке в 4ую колонку ставим признак полной занятости ряда, тогда даже json не придётся парить, чтобы понять, что все места заняты. Если представить, что в каждом ряду в среднем 10 мест, то кол-во строк сокращается с 15к до 1.5к, а с учётом того, что к моменту полной продажи билетов, большое кол-во мест будет распроданы (особенно в передних рядах), по общему признаку занятости просто закрасим в приложении весь ряд в, условно, красный.

Protected Internal (familyOrAssembly) таки даёт возможность унаследоваться из другой сборки. Для вашего случая нужен private protected (familyAndAssembly).

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

Имхо разница между анемичной и толстой моделью в глазах смотрящего :)

Если мы вынесли логику в некие условные сервисы, юзкейсы и прочее (манипулирующие само собой только доменными же интерфейсами и ентитями ) - они частью домена быть не перестают. Ну или самой моделью в широком смысле этого слова. Так что анемичная модель это такой Гомер Симпсон с защипами на спине. Мне, кстати, с ней удобнее - выстроить иерархию сервисных классов манипулирующих дата-ентити, зацепив их на интерфейсы друг друга. Тогда у нас не нарушается буковка S из нашего любимого SOLID и у нас не возникает необходимости лезть в один класс дважды - за изменением формата данных и протоколом их обработки в бизнес-процессе.

Хорошо, что поправили

Я ведь правильно понял, что вся бизнес-логика сводится к двум запросам вида:

update Seats
set ClientId = @clientId
where SessionId = @seesionId and Number = @number and Type = @type

Так?

Для этого точно надо делать три проекта, не считая тестов? В чем преимущество от трех проектов?

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

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

Также ещё одно отличие - на левом рисунке в голубом слое справа есть пункт DB (база данных), а в рисунке из книги нет такого пункта. В англоязычном варианте книги есть опция пункт DB в этом рисунке. Это очень странно - помещать визуальный интерфейс и базу данных в один слой. Визуальный интерфейс и функционал доступа к базе данных находятся в противоположных слоях приложения.

Мне это тоже показалось странным, но потом пришел к выводу, что, в общем, это логично:

  1. База данных, как и пользовательский интерфейс, - это плохой внешний мир. Функциональщики такое называют side effect, даже оборачивают в монады (привет IO из haskell)

  2. Можно сделать логическое разграничение на 2 множества:

    • Веб, UI - это взаимодействие С нами

    • БД, внешние сервисы, устройства - это уже МЫ взаимодействуем

    И эти множества расположены близко друг к други на противоположных краях слоя, как бы вход и выход.

    Такое разграничение явно не проговаривалось, но лично я это так интерпретирую.

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

Но опять же не вижу каких-то преимуществ в таком виде представления перед стандартным вариантом многослойной архитектуры. Моя версия описания многослойной архитектуры здесь https://habr.com/ru/articles/667922/.

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

Когда стремление к чистой архитектуре приводит к грязному коду, нарушению базовых принципов кодирования в угоду красивой картинке. Бизнес-исключения, которые исключениями не являются, а это вполне ожидаемое, логичное поведение, определённое правилами бизнеса, это очень и очень плохо. Не делайте так пожалуйста.

Многословно и запутанно, много дублирования. Seat и DatabaseSeat, методы Book() и Buy() в каждом наследнике Seat, методы для Visitor. Из-за конструкторов разрешается покупка одним и тем же клиентом 2 раза, хотя для другого клиента будет исключение. Доменное исключение в репозитории.

Не рассмотрены вопросы валидации и возврата человекопонятных ошибок, особенно если есть требование возвращать все сразу, и защиты от одновременного бронирования. Если это добавлять, код будет еще более запутанным.

Лучше работать с сущностью Seat без наследования, так код будет гораздо проще. Session в этом варианте для описанной логики тоже не нужен. Будет как-то так. Я не специалист в C#, и использовал такой подход в другом языке, возможно где-то синтаксис неправильный.

Код
[ApiController]
[Route("sessions")]
public class SessionsController: ControllerBase
{
    private readonly ISeatService _seatService;
    private readonly ISessionRepository _sessionRepository;

    [HttpPut("seat/{seatId:int}/book")]
    public async Task<IActionResult> BookSeat(int seatId, [FromQuery][Required] Client client, CancellationToken token = default)
    {
        var seat = this.findEntity(seatId);
        
        ValidationResult validationResult = _seatService.IsBookingAllowed(seat);
        if (validationResult.hasErrors()) {
          return ValidationError(validationResult.getErrors());
        }
        
        await _seatService.BookSeatAsync(seat, client, token);
        
        return Ok();
    }
    
    private async Task<Seat> findEntity(int seatId)
    {
        var seat = await _sessionRepository.findSeatById(seatId, true, token);
        if (seat === null) {
            throw new NotFoundHttpException('Seat not found');
        }
        
        return seat;
    }
}

public class PostgresSessionRepository: ISessionRepository
{
    private readonly SessionDbContext _context;
    private readonly SessionDbContext _lockService;
    
    public async Task<Seat|null> findSeatById(int seatId, bool needLock, CancellationToken token = default)
    {
        // блокировать от изменений в базе надо до загрузки данных в приложение
        if (needLock) this._lockService.Lock('Seat', seatId);
        
        var found = await _context.Seats.AsNoTracking()
            .Include(s => s.Session)
            .FirstOrDefaultAsync(s => s.Id == seatId, token);
            
        return found;
    }
    
    public async Task UpdateSeatAsync(Seat seat, CancellationToken token = default)
    {
        await _context.Seats
            .Where(s => s.seatId == seat.Id)
            .ExecuteUpdateAsync(calls =>
                calls.SetProperty(s => s.ClientId, seat.ClientId)
                  .SetProperty(s => s.Type, seat.Type),
                token
            );
    }
}

class ValidationResult {
  private new Array<string> errors;
}

class SuccessValidationResult<type T>: ValidationResult {
  private T validData;
}

class SeatService
{
    private readonly ISessionRepository _sessionRepository;

    public ValidationResult IsBookingAllowed(Seat seat, Client client)
    {
      var errors = new Array<string>;
      if (seat.Type !== SeatType.Free) {
        errors.add('Seat is not free');
      }
      if (seat.ClientId !== client.Id) {
        errors.add('Seat is already busy');
      }
      
      return errors.isEmpty() ? 
        new SuccessValidationResult(errors, seat);
        new ValidationResult(errors);
    }
    
    public async Task<Seat> BookSeatAsync(SuccessValidationResult<Seat> validationResult, Client client, CancellationToken token = default)
    {
        Seat seat = validationResult.validData;
        seat.Book(client);
        await _sessionRepository.UpdateSeatAsync(seat, token);
        
        return seat;
    }
}

enum SeatType
{
    Free = 0,
    Booked = 1,
    Bought = 2,
}

// ORM entity
class Seat
{
    public int Id { get; set; }
    public SeatType Type { get; set; }
    public int SessionId { get; set; }
    public Session Session { get; set; }
    public int Number { get; set; }
    public int? ClientId { get; set; }
    
    public void Book(int clientId)
    {
      this.Type = SeatType.Booked;
      this.ClientId = SeatType.clientId;
    }
    
    public void Buy(int clientId)
    {
      this.Type = SeatType.Bought;
      this.ClientId = SeatType.clientId;
    }
}
Sign up to leave a comment.

Articles