Настройка переменных при изменении атрибута статуса моей модели Order Laravel при обновленном событии в наблюдателе

У меня есть модель заказа и модель клиента. У клиента много заказов, заказ принадлежит одному клиенту.

В клиентской модели я отслеживаю, сколько денег заплатил клиент (total_generated_revenue), сколько заказов он отменил (orders_cancelled), и сколько его заказов было выполнено (orders_got).

В модели заказа у меня есть атрибут статуса и общий атрибут (и client_id атрибут, конечно).

Когда статус заказа изменяется, я изменяю другие свойства объекта относительно точного изменения, которое произошло в статусе. Действия при каждом изменении разные. Статус может быть 0, 1 или 2, поэтому есть 6 возможных изменений статуса. Я справляюсь с этим с помощью 7 if заявления:

public function updated(Order $order)
{
    $unchanged_order = $order->getOriginal();
    if($unchanged_order->status == $order->status)
    {
        // skip if status wasn't changed. I use this skip so that we don't have to check all of the following. 
    }
    elseif($unchanged_order->status == 0 && $order->status == 1)
    {
        $order->client->total_generated_revenue += $order->total;
        $order->client->orders_got += 1;
    }
    elseif($unchanged_order->status == 0 && $order->status == 2)
    {
        $order->client->orders_cancelled += 1;
    }
    elseif($unchanged_order->status == 1 && $order->status == 2)
    {
        $order->client->total_generated_revenue -= $order->total;
        $order->client->orders_got -= 1;
        $order->client->orders_cancelled += 1;
    }
    elseif($unchanged_order->status == 1 && $order->status == 0)
    {
        $order->client->total_generated_revenue -= $order->total;
        $order->client->orders_got -= 1;
    }
    elseif($unchanged_order->status == 2 && $order->status == 1)
    {
        $order->client->total_generated_revenue += $order->total;
        $order->client->orders_got += 1;
        $order->client->orders_cancelled -= 1;
    }
    elseif($unchanged_order->status == 2 && $order->status == 0)
    {
        $order->client->orders_cancelled -= 1;
    }
    $order->client->save();
}

Считается ли это плохой практикой? Если да, то как мне провести рефакторинг этого кода?

Имя $unchanged_order достаточно хорошо отражают, что я имею в виду версию до изменения? Должен ли я переименовать его в $order_before_change?

1 ответ
1

Девять комбинаций статусов

Статус может быть 0, 1 или 2, поэтому существует 8 возможных изменений статуса.

Есть девять (три состояния, умноженные на три состояния) возможных «изменений» (комбинаций состояний). Хотя трое из них останутся прежними. Вот почему вам нужно семь if заявления (а на самом деле вы этого не делаете, последнее может быть простым else): шесть, чтобы охватить возможные изменения, и один, чтобы охватить три возможных способа остаться прежним. Если бы было только два статуса, это было бы четыре (два раза два) и три (один плюс два раза один). Если четыре, то шестнадцать (четыре раза по четыре) и тринадцать (один плюс четыре раза по три).

В общем, если есть $ n $ состояний, то будет $ п ^ 2 $ комбинации и $ 1 + n (n — 1) = n ^ 2 -n + 1 $ условия. Есть $ n $ комбинации, которые остаются прежними. Таким образом, общее количество комбинаций минус число, в котором статус остается прежним, плюс один для всех тех, где статус остается прежним.

Именование

    $unchanged_order = $order->getOriginal();

Я бы нашел $original_order яснее.

        // skip if status wasn't changed. I use this skip so that we don't have to check all of the following. 

Вместо того, чтобы комментировать, было бы проще просто return в этом случае. Поскольку нет причин обновлять клиент, если ничего не изменилось.

        // Nothing changed; nothing to do.  

Кроме того, этот комментарий короче и лучше объясняет, что происходит.

Альтернатива

    if ($original_order->state === $order->state) {
        return;
    }

    if ($original_order->state === 1) {
        $order->client->total_generated_revenue -= $order->total;
        $order->client->orders_got -= 1;
    } elseif ($original_order->state === 2) {
        $order->client->orders_cancelled -= 1;
    }

    if ($order->state === 1) {
        $order->client->total_generated_revenue += $order->total;
        $order->client->orders_got += 1;
    } elseif ($order->state === 2) {
        $order->client->orders_cancelled += 1;
    }

Если вы проработаете это, все будут те же результаты, но только с пятью сравнениями, а не с семью, которую вы использовали.

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

Статусные объекты

Более продвинутой формой было бы использование объектов или классов в качестве состояний, а не примитивных целых чисел. Тогда вы можете заменить обе конструкции (не первую if) с чем-то вроде

    $original_order->status->leave($order);
    $order->status->enter($order);

Тогда вы могли бы сделать более простые методы, например

namespace OrderStatus;

class Cancelled implements OrderStatus
{

    public function enter($order)
    {
        $order->client->orders_cancelled++;
    }

Нет if/else строительные леса.

Это упростило бы добавление статуса, так как вам просто нужно было бы создать один файл и реализовать необходимые методы. Это делает update код проще за счет усложнения статуса.

    Добавить комментарий

    Ваш адрес email не будет опубликован. Обязательные поля помечены *