Исходный код
function ViewStar($prop){
switch ($prop) {
case 3:
return "<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>";
break;
case 4:
return "<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>";
break;
case 5:
return "<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>
<img alt=\"Star\" src=\"".$z."images/star.gif\"/>";
break;
}
}
Что не так в исходном коде
Легко видеть, что имеет место дублирование. Не следует воспринимать его как абстактное дублирование участков кода, очень важно вдумчиво ответить на вопрос: что именно дублируется?
Здесь дублируется реализация алгоритма рендеринга одиночной звёздочки. Даже если бы тег звёздочки состоял из простой строки символов — это уже самостоятельное знание, знание о том, как отрендерить звёздочку. Тем более, что здесь путь к картинке звёздочки может меняться. Реализация этого знания в виде кода и есть алгоритм.
Следующий бросающийся в глаза момент — неоптимальная реализация повторов звёздочек. Когда число является счётчиком повторов — это всегда цикл. Если речь просто о повторах строки, то почти во всех языках есть функции для клонирования строки нужное количество раз.
Ещё один недочёт заключается в названиях переменных: они не рассказывают о смысле величин, в них хранящихся.
Я был бы не я, если бы не рассказал о многочисленности самостоятельных знаний в этом фрагменте кода. Нет смысла скрывать, здесь их три:
- знание о том, где лежат картинки на сайте;
- знание о том, как отрендерить одиночную звёздочку;
- знание о том, как отрендерить указанное количество звёздочек.
В идеале каждое из них должно быть заключено в отдельную функцию, однако это не всегда практично.
Во многих библиотеках есть хелперы для получения пути указанных ресурсов на сайте, в частности, картинок. Хелпер для картинок логично назвать image_path
. Такой хелпер пригодится ещё в сотне мест кода. Кстати, понимание факта самостоятельности знания о получении пути картники приводит к пониманию того, что и эта реализация тоже дублируется в исходном коде.
А вот знание о рендеринга тега звёздочки уже не такое ценное, поэтому его вполне допустимо объединить с знанием о рендеринге группы звёздочек.
Как можно отрефакторить исходный код
function ImagePath($image) {
return $site_root . 'images/' . $image;
}
function ViewStars($star_count) {
$star = '<img alt="Star" src="' . ImagePath('star.gif') . '"/>';
return str_repeat($star, $star_count);
}
Теория
- Принцип единственной реализации
- Принцип единственного знания
- Рефакторинг «извлечение метода»