Пример рефакторинга «извлечение функции» для вычисления цвета ячейки таблицы


Исходный код

if ( 0 == 0 ) 
{ 
  $x += 1; 
  $yellow = ""; 
  if ( $myhighlight_unit[$i_high] == $row[$i]['ID'] ) 
  { 
    //echo $myhighlight_unit[$i_high]." == ".$i_high."<br />";
    $yellow = "background-color: #ffcc33;"; 
  } 
  if ( $row[$i]['approved'] > 0 && $_SESSION['userid'] > 3 ) 
  { 
    $yellow = "background-color: #ff0000;"; 
  } 
  if ( $i_tr == 0 ) 
  { 
    echo"<tr>"; 
  }
}

Как можно улучшить исходный код

При беглом взгляде на код легко видеть, что его основную часть занимает вычисление цвета фона. Это достаточно осмысленная операция, чтобы её перенести в отдельную функцию:

function cellStyles($cell, $unit_id) {
    if ($cell['approved'] > 0 && $_SESSION['userid'] > 3) {
        return"background-color: #ff0000;";
    }
    if ($cell['ID'] === $unit_id) {
        return"background-color: #ffcc33;";
    }
    return '';
}

Вызов функции будет выглядеть так:

$yellow = cellStyles($row[$i], $myhighlight_unit[$i_high);

В идеале подобные вопросы решаются через CSS-классы: функция должна возвращать имена классов для клетки таблицы, а в CSS должны быть прописаны стили для этих классов.

Имена переменных следует привести в соответствие с их смыслом:

  • $yellow → $cell_styles или $cell_classes
  • $x → вероятно, $cell_number
  • $i_tr — тоже очень похоже на $cell_number, надо разобраться, что из них для чего

Избавляться от условия if ( 0 == 0 ) скорее всего нет смысла, тут явное указание на ещё не устоявшуюся часть алгоритма, которая то и дело меняется.

Теория