Одинаковая реализация двух кейсов при рендеринге веб-страницы


Исходный код

if ($_name=='homepage' AND !is_numeric($_GET['id2']))
	include(SITEROOT."tpl/main.php");
else
  include(SITEROOT."tpl/main.php");

Что так в этом коде

Вероятнее всего, дублирование в этом фрагменте кода появилось в результате многократных изменений, то есть эволюционно.

Здесь нужно смотреть в первую очередь на само ветвление в целом, а не на то, что происходит внутри веток. Ветвление появилось по причине того, что пользователь может запросить два вида страниц: домашнюю либо другую (по коду не понятно, какую) — то есть ветвление является отражением предметной области. Двум видам страниц соответствуют две ветки алгоритма. Тот факт, что в данный момент два вида страниц рендерятся с помощью одного и того же шаблона, ещё не отменяет факта предметной области: существуют два вида страниц.

Поэтому перед тем, как убрать ветвление и сделать безусловным рендеринг одного шаблона, необходимо поинтересоваться, какая обстановка в предметной области.

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

Тем не менее, в данном фрагменте кода есть вещи, которые можно улучшить.

Первый момент — не считывается смысл выражения в операторе if: нет ясности, какие виды страниц там подразумеваются.

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

if (isOtherPage())
	// render other page
else
  // render home page

или дополнительная переменная:

$page = $_name === 'homepage' AND !is_numeric($_GET['id2']) ? 'home-page' : 'other-page';

if ($page === 'other-page') 
	//
else
  //

В обоих вариантах мы разделили код, который определяет вид страницы, запрошенной посетителем, и код, который определяет способ рендеринга каждого вида страницы.

Такое разделение весьма полезно как для понимания кода, так и для будущих изменений в этом коде.

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

$page = $_name === 'homepage' AND !is_numeric($_GET['id2']) ? 'home-page' : 'other-page';

$template = $page === 'other-page' ? 'main.php' : 'main.php';

include(TEMPLATEROOT . $template);

В таком варианте достигнуты максимальное разъединение ответственностей (определение вида страницы, определение шаблона страницы, рендеринг шаблона) и максимальная гибкость кода. В идеале каждую ответственность осталось убрать в отдельную функцию.