1

Вопрос на инспекцию кода.

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

public class Order
{
    public int Id { get; set; }

    public OrderStatus ; { get; set; }

    public virtual ICollection<OrderPosition> OrderPositions { get; set; }

    // ... и прочие данные заказа

    public bool CanDeleteOrder(int userId)
    {
        if (this.Status == OrderStatus.Open && this.IsManagerOrParticipant(userId))
            return true;

        if (this.Status == OrderStatus.Locked && this.IsManager(userId))
            return true;

        return false;
    }

    public bool IsOrderPositionsReadOnly(int userId)
    {
        if (this.Status == OrderStatus.Archieved)
            return true;

        if (this.Status == OrderStatus.Open && this.IsParticipant(userId))
            return false;

        if (this.IsManager(userId))
            return false;

        return true;
    }

    public bool CanCreateOrderPositions(int userId)
    {
        return !this.IsOrderPositionsReadOnly(userId);
    }

    public bool CanUpdateOrderPositions(int userId)
    {
        return !this.IsOrderPositionsReadOnly(userId);
    }

    public bool CanDeleteOrderPositions(int userId)
    {
        return !this.IsOrderPositionsReadOnly(userId);
    }

    private bool IsManager(int userId)
    {
        return this.GroupBuying.ManagerId == userId;
    }

    private bool IsManagerOrParticipant(int userId)
    {
    }
}

При этом собственно действия вынесены в сервисы:

public class OrderService
{
    public OrderService(IOrderRepository orderRepository)
    {
        this.OrderRepository = orderRepository ?? throw new ArgumentNullException(nameof(orderRepository));
    }

    private readonly IOrderRepository OrderRepository;

    public async Task<bool> DeleteAsync(int orderId, int userId)
    {
        var order = await this.OrderRepository.FindAsync(orderId);

        if (order == null)
            throw new ApplicationException($"Unable to find Order with Id '{orderId}'.");

        if (!order.CanDeleteOrder(userId))
            throw new ApplicationException("CanDeleteOrder is false.");

        return await this.OrderRepository.DeleteAsync(order);
    }

    ....
}

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

Я имею ввиду, что снаружи класса Order может появиться некоторый класс, который захочет вызвать например удаление при этом не вызвав проверки CanDeleteOrder (или вызвав какие-то свои проверки, свою логику проверок) -- а хочется, что вся логика по определению возможно ли удаление осталась в одном центральном месте.

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

У меня самого есть два варианта, как это можно сделать и оба содержат какие-то изьяны.

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

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

3
  • третий вариант: оставить как есть, если кто-то пользуется недокументированными возможностями - его проблемы
    – Grundy
    22 июл 2018 в 10:17
  • как вариант, сделать статический класс, который принимает на вход экземпляр объекта и проверяет на корректность. И всю проверку вынести туда. Таким образом централизация проверок будет и можно вызывать необходимые методы где угодно (например контроллеры, viewModelы.. сервисы)
    – Vein
    22 июл 2018 в 10:35
  • @SultanovShamil Цель не "централизовать проверки", а "централизовать проверки подближе к действиям": в языке нет конструкций, которые явно позволяют указать, что некий метод обязан вызываться только после проверок -- поэтому хоть как-то снизить возможность выстрелить себе в ногу можно только собрав связанную логику поближе друг к другу, чтобы она была понятной.
    – A K
    22 июл 2018 в 10:53

1 ответ 1

1

Если рассудить логически, то CanDeleteOrder не должен находиться в сущности Order, так как не понятно, откуда эту заявку удалить. Если я создал var orders = new List<Order>(); отсюда можно удалять? Все эти методы необходимы для работы сервиса, означают, что через сервис нельзя удалить заявку, следовательно у сервиса и должен находиться метод для проверки можно ли заявку удалить через него. И да, по хорошему было бы сделать еще интерфейс IOrderService, чтобы SOLID и все такое, вдруг вы захотите использовать другую реализацию сервиса.

public class OrderService : IOrderService
{
    public OrderService(IOrderRepository orderRepository)
    {
        this.OrderRepository = orderRepository ?? throw new ArgumentNullException(nameof(orderRepository));
    }

    private readonly IOrderRepository OrderRepository;

    public bool CanDeleteOrder(Order order, int userId) //Возможно просто CanDelete, тут то понятно, что речь об Order, но для понимания, перенес как было 
    {
        if (order.Status == OrderStatus.Open && order.IsManagerOrParticipant(userId))
            return true;

        if (order.Status == OrderStatus.Locked && order.IsManager(userId))
            return true;

        return false;
    }

    public async Task<bool> DeleteAsync(int orderId, int userId)
    {
        var order = await this.OrderRepository.FindAsync(orderId);

        if (order == null)
            throw new ApplicationException($"Unable to find Order with Id '{orderId}'.");

        if (!CanDeleteOrder(order, userId))
            throw new ApplicationException("Order can not be deleted");

        return await this.OrderRepository.DeleteAsync(order);
    }
}

Ваш ответ

Нажимая на кнопку «Отправить ответ», вы соглашаетесь с нашими пользовательским соглашением, политикой конфиденциальности и политикой о куки

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.