Пример рефакторинга «извлечение метода» при формировании объекта критериев


Исходный код

public function buildCriteria($table, $conditions)
{
    $source_table = $table->getTableName();
    $criteria = newcriteria($source_table, 'keyword');

    foreach ($conditionsas$field => $conds) {
        if (is_array($conds)) {

            if (isset($conds['plus'])) {
                $word = array_shift($conds['plus']);
                $like_criterion = newcriterion($field, '%' . $word . '%', criteria::LIKE);
                foreach ($conds['plus'] as$word) {
                    $like_criterion->addOr(newcriterion($field, '%' . $word . '%', criteria::LIKE));
                }
            }

            if (isset($conds['minus'])) {
                $word = array_shift($conds['minus']);
                $not_like_criterion = newcriterion($field, '%' . $word . '%', criteria::NOT_LIKE);
                foreach ($conds['minus'] as$word) {
                    $not_like_criterion->addAnd(newcriterion($field, '%' . $word . '%', criteria::NOT_LIKE));
                }
            }

            if (isset($conds['less'])) {
                if (!is_array($conds['less'])) {
                    $conds['less'] = array($conds['less']);
                }

                $word = array_shift($conds['less']);
                $less_criterion = newcriterion($field, $word, criteria::LESS);

                foreach ($conds['less'] as$word) {
                    $less_criterion->addAnd(newcriterion($field, $word, criteria::LESS));
                }
            }

            if (isset($conds['less_eq'])) {
                if (!is_array($conds['less_eq'])) {
                    $conds['less_eq'] = array($conds['less_eq']);
                }

                $word = array_shift($conds['less_eq']);
                $less_eq_criterion = newcriterion($field, $word, criteria::LESS_EQUAL );

                foreach ($conds['less_eq'] as$word) {
                    $less_eq_criterion->addAnd(newcriterion($field, $word, criteria::LESS_EQUAL));
                }
            }
        }
    }

    $main_criterion = new criterion();

    if (!empty($like_criterion)) {
        $main_criterion->add($like_criterion);
        $add_method = 'addAnd';
    }

    if (!empty($not_like_criterion)) {
        if (empty($add_method)) {
            $add_method = 'add';
        }
        $main_criterion->$add_method($not_like_criterion);
    }

    if (!empty($less_criterion)) {
        if (empty($add_method)) {
            $add_method = 'add';
        }
        $main_criterion->$add_method($less_criterion);
    }

    if (!empty($less_eq_criterion)) {
        if (empty($add_method)) {
            $add_method = 'add';
        }
        $main_criterion->$add_method($less_eq_criterion);
    }

    $criteria->add($main_criterion);

    return $criteria;
}

Что не так в исходном коде

  1. Основной цикл загромождён подробностями реализаций формирования критериев для каждого оператора plusminuslessless_eq — такие вещи всегда нужно убирать в отдельные методы. В идеале, каждое условие должно представлять собой экземпляр соответствующего класса ConditionPlus, ConditionMinus, ConditionLess, ConditionLessEq, у которых есть метод getCriterion.
  2. Непонятным мне образом критерии, формируемые в цикле, могут затираться на последующих итерациях.
  3. Во второй части метода четыре сформированных критерия обрабатываются однородным образом, но без применения цикла, что не хорошо.
  4. Объект $criteria создаётся в самом начале метода, заполняется в самом конце, а между этими двумя операциями идёт много не относящихся к ним вычислений — так делать не хорошо, это затрудняет чтение кода.
  5. Формировать объект $criteria внутри метода buildCriteria вообще нет серьёзной необходимости, так как эта ответственность никак не связана с остальными операциями в методе.

Как можно отрефакторить исходный код

$criteria = new criteria($table->getTableName(), 'keyword');
$criteria->add($this->buildCriterionForConditions($conditions));
public function buildCriterionForConditions($conditions)
{
    foreach ($conditions as $field => $conds) {
        if (is_array($conds)) {
            if (isset($conds['plus'])) {
                $like_criterion = $this->buildLikeCriterion($field, $conds['plus']);
            }
            if (isset($conds['minus'])) {
                $not_like_criterion = $this->buildNotLikeCriterion($field, $conds['minus']);
            }
            if (isset($conds['less'])) {
                $less_criterion = $this->buildLessCriterion($field, $conds['less']);
            }
            if (isset($conds['less_eq'])) {
                $less_eq_criterion = $this->buildLessEqCriterion($field, $conds['less_eq']);
            }
        }
    }
    $all_criterions = array_filter(array($like_criterion, $not_like_criterion, $less_criterion, $less_eq_criterion));
    return $this->joinCriterionAnd($all_criterions);
}

protected function buildLikeCriterion($field, $words)
{
    $word = array_shift($words);
    $like_criterion = new criterion($field, '%' . $word . '%', criteria::LIKE);

    foreach ($words as $word) {
        $like_criterion->addOr(new criterion($field, '%' . $word . '%', criteria::LIKE));
    }

    return $like_criterion;
}

protected function buildNotLikeCriterion($field, $words)
{
    $word = array_shift($words);
    $not_like_criterion = new criterion($field, '%' . $word . '%', criteria::NOT_LIKE);

    foreach ($words as $word) {
        $not_like_criterion->addAnd(new criterion($field, '%' . $word . '%', criteria::NOT_LIKE));
    }

    return$not_like_criterion;
}

protected function buildLessCriterion($field, $words)
{
    if (!is_array($words)) {
        $words = array($words);
    }

    $word = array_shift($words);
    $less_criterion = new criterion($field, $word, criteria::LESS);

    foreach ($words as $word) {
        $less_criterion->addAnd(new criterion($field, $word, criteria::LESS));
    }

    return $less_criterion;
}

protected function buildLessEqCriterion($field, $words)
{
    if (!is_array($words)) {
        $words = array($words);
    }

    $word = array_shift($words);
    $less_eq_criterion = new criterion($field, $word, criteria::LESS_EQUAL );

    foreach ($words as $word) {
        $less_eq_criterion->addAnd(new criterion($field, $word, criteria::LESS_EQUAL));
    }

    return $less_eq_criterion;
}

protected function joinCriterionAnd($criterions)
{
    $join_criterion = new criterion();
    if (!empty($criterions)) {
        $join_criterion->add(array_shift($criterions));
        foreach ($criterions as $criterion) {
            $join_criterion->addAnd($criterion);
        }
    }
    return $join_criterions;
}

Подобная раскладка кода на методы не является идеальной, но всё равно заметно лучше в рамках выбранной архитектуры, чем было до рефакторинга. Повторюсь, что архитектурно более чистым подходом будет инкапсулировать в отдельные классы всё поведение для критериев plus, minus, less, less_eq (по одному классу на критерий).

Теория