Информация об изменениях

Сообщение Re[4]: Option vs ? - критика Rust от 24.10.2023 2:51

Изменено 24.10.2023 2:52 Sinclair

Старый текст
Новый текст
Разница

Re[4]: Option vs ? - критика Rust

Здравствуйте, Быдлокодер, Вы писали:

Б>Здравствуйте, Sinclair, Вы писали:

S>>Ужасно. Просто ужасно. Чувак берёт плохой код, и лёгким движением руки.... делает его ещё хуже!
S>>Пипец какой-то.

Б>А можете рассказать, что не так в преобразованном коде?
1. В репозиторий вносится метод GetLast3YearsCompletedOrdersCountFor(long customerId).
Отлично — мы увеличили площадь поверхности репозитория, затрудняя его поддержку. Кроме того, теперь нам надо полагаться на средства рефакторинга для принятия простейших решений. Вот, например — требования к скидке изменились, теперь заказы берутся не за три года, а за два. Что делать — менять метод, или его кто-то уже использует?
Введение отдельного метода оправдано там, где есть хотя бы одно из
а) он используется более 1го раза
б) его "смысл" не совпадает с его текстом.
Здесь нет ни одного из этих вариантов.
Правильный ответ:

var completedOrdersInLast3Years = 
  from o in _merchantDb.Orders 
  where 
    o.customerId == ord.Customer ID &&
    o.StateID == OrderState.Completed &&
    o.OrderDate >= DateTime.UtcNow.AddYears(-3)
  select o;
var count = completedOrdersInLast3Years.Count();

Если прямая запись предикатов царапает глаз, то к IEnumerable<Order> делается набор екстеншн-методов, и код переписывается так:

var completedOrdersCount = 
  merchantDb.Orders
    .ByCustomerId(ord.CustomerId)
    .ByState(OrderState.Completed)
    .WithinLast(TimeSpan.FromYears(3));

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

2. У DiscountCalculator публичный метод называется CalculateDiscountBy(long). Это — заботливо разложенные грабли.
Вот в таком коде компилятор не заметит никакой ошибки. Заметит ли её ревьювер кода?

public void CheckoutV2(long orderId)
{
  var order = _ordersRepository.GetOrder(orderId);
  var discount = _discountCalculator.CalculateDiscountBy(orderId);
  order.ApplyDiscount(discount);
  order.State = OrderState.AwaitingPayment;
  _ordersRepository.Save(order);
}

Если уж хочется упороться, то метод должен называться CalculateCustomerDiscount, а типом параметра должен быть или специальный отдельный СustomerId, или интерфейс ICustomerReference, который реализуется всеми объектами, ссылающимися на customer — и, в частности, order.

3. Примерно те же проблемы у метода DiscountBy() — по его вызову непонятно, что туда должно передаваться.
Плюс жульничество: то, что однострочное выражение для расчёта уровня скидки переписано в цепочку if, читаемость улучшило незначительно.
Я вот вовсе не уверен, что такой код:

private decimal DiscountBy(long completedOrdersCount)
{
  if (completedOrdersCount >= 5)
    return 30;
  if (completedOrdersCount >= 3)
    return 20;
  if (completedOrdersCount >= 1)
    return 10;

  return 0;
}

читается сильно лучше, чем

private decimal DiscountBy(long completedOrdersCount) => 
  completedOrdersCount >= 5 
    ? 30
    : completedOrdersCount >= 3
      ? 20
      : completedOrdersCount >= 1
        ? 10
        : 0;

И уж точно это читается хуже, чем

private decimal DiscountBy2(long completedOrdersCount) => 
  completedOrdersCount switch 
  {
    >= 5 => 30,
    >= 3 => 30,
    >= 1 => 10,      
    _    =>  0
  };

После такой записи становится понятно, почему мы не хотим выносить этот код в отдельный метод.
В итоге, метод вычисления скидки превращается в 1 выражение:

public decimal CalculateCustomerDiscount(ICustomerReference cr) =>
  _merchantDb.Orders
    .ByCustomerId(cr.CustomerId)
    .ByState(OrderState.Completed)
    .WithinLast(TimeSpan.FromYears(3))
    .Count() switch 
    {
      >= 5 => 30,
      >= 3 => 30,
      >= 1 => 10,      
      _    =>  0
    };

Всё. Никакого бойлерплейта, никаких посторонних методов в слое доступа к данным, никакого размазывания логики. Тут даже тестировать, в общем-то, нечего — код делает ровно то, что написано в ТЗ.

3. Отдельная песня про сопровождаемость этого кода. В реальном приложении у нас явно будет не одна скидка, а целая маркетинговая система со сложным набором правил. Поэтому никакого DiscountCalculator, да ещё с прямыми зависимостями от сервиса, предоставляющего предысторию заказов, у нас не будет. А будет движок правил, которые применяются к отдельным позициям и к заказу в целом. Это позволит не звать программистов каждый раз, как отделу продаж приходит в голову идея "на второй товар в заказе — скидка 20%, на третий — скидка 50%". И весь DDD с его Ubiquitos language пойдёт в быстром темпе вдоль и по диагонали. Потому что упускает существенный момент: моделировать надо не задачу, а решение. Даже в таком микроскопическом примере мы уже должны увидеть, как будут устроены сигнатуры типов, моделирующих правила применения скидок.
Скорее всего, нам потребуется информация как о кастомере, так и о заказе.
Поэтому метод CalculateCustomerDiscount заменится на CalculateOrderDiscount(IOrderReference o) с прицелом на будущее заменить эту заглушку на полноценную систему скидочных правил с приоритетами и ограничениями.

4. Перформанс. В реальном нагруженном приложении нам крайне не хочется видеть подъёма в память целого заказа ради того лишь, чтобы взять из него customerID, сбегать куда-то ещё в базу, и потом обновить скидку, и выполнить обратный апдейт всей записи. Зачем?
У нас есть функция, которая по order строит его скидку.
Всё, что нужно сделать — это передать эту функцию на сторону СУБД:

public void CheckoutV3(long orderId)
{
  _merchantDb.Orders.ById(orderId)
    .Set(o=>o.Discount, o=> _discountCalculator.CalculateDiscountBy(o)
    .Set(o=>o.State, o=> OrderState.AwaitingPayment)
    .Update();
}

PROFIT!

Re[4]: Option vs ? - критика Rust

var completedOrdersInLast3Years = 
  from o in _merchantDb.Orders 
  where 
    o.customerId == ord.Customer ID &&
    o.StateID == OrderState.Completed &&
    o.OrderDate >= DateTime.UtcNow.AddYears(-3)
  select o;
var count = completedOrdersInLast3Years.Count();

var completedOrdersCount = 
  merchantDb.Orders
    .ByCustomerId(ord.CustomerId)
    .ByState(OrderState.Completed)
    .WithinLast(TimeSpan.FromYears(3));

public void CheckoutV2(long orderId)
{
  var order = _ordersRepository.GetOrder(orderId);
  var discount = _discountCalculator.CalculateDiscountBy(orderId);
  order.ApplyDiscount(discount);
  order.State = OrderState.AwaitingPayment;
  _ordersRepository.Save(order);
}

private decimal DiscountBy(long completedOrdersCount)
{
  if (completedOrdersCount >= 5)
    return 30;
  if (completedOrdersCount >= 3)
    return 20;
  if (completedOrdersCount >= 1)
    return 10;

  return 0;
}

читается сильно лучше, чем

private decimal DiscountBy(long completedOrdersCount) => 
  completedOrdersCount >= 5 
    ? 30
    : completedOrdersCount >= 3
      ? 20
      : completedOrdersCount >= 1
        ? 10
        : 0;

И уж точно это читается хуже, чем

private decimal DiscountBy2(long completedOrdersCount) => 
  completedOrdersCount switch 
  {
    >= 5 => 30,
    >= 3 => 30,
    >= 1 => 10,      
    _    =>  0
  };

public decimal CalculateCustomerDiscount(ICustomerReference cr) =>
  _merchantDb.Orders
    .ByCustomerId(cr.CustomerId)
    .ByState(OrderState.Completed)
    .WithinLast(TimeSpan.FromYears(3))
    .Count() switch 
    {
      >= 5 => 30,
      >= 3 => 30,
      >= 1 => 10,      
      _    =>  0
    };

public void CheckoutV3(long orderId)
{
  _merchantDb.Orders.ById(orderId)
    .Set(o=>o.Discount, _discountCalculator.CalculateCustomerDiscount)
    .Set(o=>o.State, o=> OrderState.AwaitingPayment)
    .Update();
}

PROFIT!