Стоит ли так рефакторить? Замена условий на простые математические формулы

#php #рефакторинг

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

Есть таблица Orders, состоящая из id, user_id, created_at, type, value, status, в
данном примере нас интересуют только поля value и type. value - любое целочисленное
положительное число, type - либо 0, либо 1 (0 - списание, 1 - начисление)

И собсно простой метод getPoints в классе User, который подсчитывает количество доступных
очков пользователя

public function getPoints() {
        $return = 0;
        foreach (Order::findAll(['user_id'=> $this->id, 'status' => 1]) as $k =>
$value) {
            if ($value['type'] == 0)
                $return -= $value['value'];
            if ($value['type'] == 1)
                $return += $value['value'];
        }
        return $return;
}


От нечего делать я решил его немного изменить

public function getPoints() {
        $return = 0;
        foreach (Order::findAll(['user_id'=> $this->id, 'status' => 1]) as $value)
                $return += (-1 + 2*(int)$value['type'])*$value['value']; //$value['type']
равно либо 0, либо 1. 0 - вычитание, 1 - сумма. Формула -1 + 2*$value['type'] нужна
для сокращения кода -1 + 2*0 = -1 -1 + 2*1 = 1

        return $return;
    }


То есть по сути просто избавился от условий и заменил это дело формулой (предполагается,
что значения 0 и 1 никогда не будут меняться). Так вообще нормально делать? Стоит ли? 
    

Ответы

Ответ 1

Если уже "рефакторить", то где то так

public function getPoints() {
        $return = 0;
        foreach (Order::findAll(['user_id'=> $this->id, 'status' => 1]) as $k =>
$value) {
            $type = $value['type'];
            $val = $value['value'];
            if ($type == 0) {
                $return -= $val;
            else if ($type == 1) {
                $return += $val;
            } else {
              # а тут добавить вывод в лог, может что то пошло не так
            }
        }
        return $return;
}


Ответ 2

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

P.S. По этому поводу на харбе недавно хорошая статья вышла: https://habrahabr.ru/post/347166/