Исходный код
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;
}
Что не так в исходном коде
- Основной цикл загромождён подробностями реализаций формирования критериев для каждого оператора
plus
,minus
,less
,less_eq
— такие вещи всегда нужно убирать в отдельные методы. В идеале, каждое условие должно представлять собой экземпляр соответствующего классаConditionPlus
,ConditionMinus
,ConditionLess
,ConditionLessEq
, у которых есть методgetCriterion
. - Непонятным мне образом критерии, формируемые в цикле, могут затираться на последующих итерациях.
- Во второй части метода четыре сформированных критерия обрабатываются однородным образом, но без применения цикла, что не хорошо.
- Объект
$criteria
создаётся в самом начале метода, заполняется в самом конце, а между этими двумя операциями идёт много не относящихся к ним вычислений — так делать не хорошо, это затрудняет чтение кода. - Формировать объект
$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
(по одному классу на критерий).
Теория
- Принцип единственного знания
- Рефакторинг «извлечение метода»
- Проклятие стандартных типов данных