Govnokodu.Net

Пример рефакторинга «извлечение метода» при получении играющих команд

Исходный код

if ($ajax_teamID == $value['team2ID']){
    $visitors_team = $Bet->CBetType[$Bet->betTypeIndex]->CTeam->getOne($value['team1ID'])->title;
    $home_team     = $Bet->CBetType[$Bet->betTypeIndex]->CTeam->getOne($value['team2ID'])->title;
} else {      
    $home_team     = $Bet->CBetType[$Bet->betTypeIndex]->CTeam->getOne($value['team1ID'])->title;              
    $visitors_team = $Bet->CBetType[$Bet->betTypeIndex]->CTeam->getOne($value['team2ID'])->title;
}

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

Чувствуется явный запах завистливой функции. Справиться с ним можно снова с помощью рефакторингов «извлечение метода» и «перемещение метода». Запах относится к фрагменту $Bet->CBetType[$Bet->betTypeIndex]->CTeam: тут посторонний код «проникает» в секреты объекта $Bet, касающиеся получения доступа к репозиторию команд. Совершенно необходимо спрятать реализацию этого секретного знания в методе объекта $Bet.

Есть также претензии к названиям переменных:

  • $ajax_teamID, $value — неясен смысл хранящихся в них данных;
  • $visitors_team, $visitors_team — судя по названию, в них должны храниться команды, а на самом деле — названия команд.

Я бы сохранил команды 1 и 2 в отдельных переменных, домашнюю и гостевую команды — в отдельных, а отдельные переменные под названия могут и не понадобиться.

Вариант рефакторинга исходного кода

$team1 = $Bet->CTeam->getOne($value['team1ID']);
$team2 = $Bet->CTeam->getOne($value['team2ID']);
if ($home_teamID == $value['team2ID']){
    list($home_team, $visitor_team) = [$team2, $team1];
}
else {
    list($home_team, $visitor_team) = [$team1, $team2];
}
function __get($property) {
    switch ($property) {
        case'CTeam':
            return$this->CBetType[$this->betTypeIndex]->CTeam;
    }
}

Теория

  • Запах кода «завистливые функции»
  • Рефакторинг «извлечение метода»
  • Утечка знания

Сергей Галанин