Я с большим энтузиазмом отношусь к любым попыткам улучшить общую культуру разработки в таком неоднозначном сообществе, как сообщество PHP-разработчиков. Но видя некоторые инициативы хочется убиться об стену их немного поправить, чтобы не дай бог не пришлось работать с кодом, сильно отличающимся от моих представлений о совершенном коде.
Сегодня почему-то не сдержался, увидев топик Из гадкого утёнка в лебедя, или как исправить криворукий код и решил его исправить по-своему. Тем более автор попросил дать ссылки на другие варианты решений.
Напомню, что стоит задача причесать код
<h1>Пользователи</h1>
<?
$DB = new DBConnector;
$DB->query(‘SELECT * FROM users LIMIT 10’);
if($DB->get_num_rows()){
while($user = $DB->fetch_row()){
echo ‘<p>’.$user[‘name’].’</p>’;
}
}
?>
С чего, по-моему, должен начинаться рефакторинг — с покрытием тестами кода, который собираемся рефакторить. Без этого мы не можем быть уверены, что всё делаем правильно и поведение кода не изменяется. Поскольку пример учебный, то не будет обрабатывать особые случаи (нет базы данных и т. п.) и будем считать, что код выводит имена четырех пользователей из БД: Username1, Username2,…
Пишем простенький тест прямо на PHP без использования фреймворков типа PHPUnit.
<?php
$expected = <<<'EOT'
<h1>Пользователи</h1>
<p>Username1</p><p>Username2</p><p>Username3</p><p>Username4</p>
EOT;
ob_start();
include 'index.php';
$actual = ob_get_clean();
echo $actual === $expected ? 'OK' : 'Failed', PHP_EOL;
Запускаем… и получаем ошибку «компиляции» из-за отсутствия класса DBConnector. Не мудрствуя лукаво пишем заглушку для него (на «боевом» рефакторинге пришлось бы его подключать, создавать тестовые таблицы и т. п., но пост не о методиках тестирования). Приблизительно так я представляю работу оригинального класса DBConnector :)
<?php
class DBConnector
{
private $users;
public function query($query)
{
for ($i=1; $i<=4; $i++) {
$this->users[] = array('name' => 'Username' . $i);
}
}
public function get_num_rows()
{
return count($this->users);
}
public function fetch_row()
{
$each = each($this->users);
return $each['value'];
}
}
Опять запускаем тест — получаем ОК. Поведение скрипта мы зафиксировали, теперь если нарефакторим что-то не то, то сразу получим Failed. Напомню, что на настоящем рефакторинге нам надо будет покрыть тестами все возможные потоки исполнения, скажем исключение или ошибки при работе с БД. Тут же ограничились одним.
Смотрим на наш код. Первое что лично мне бросается в глаза — всё перемешано заголовок, запрос к БД, вывод. Решено, разделяем на получение записей и вывод html.
<?php
include 'DBConnector.php';
$DB = new DBConnector;
$DB->query('SELECT * FROM users LIMIT 10');
if($DB->get_num_rows()){
while($user = $DB->fetch_row()){
$users[] = $user;
}
} else {
$users = array();
}
?>
<h1>Пользователи</h1>
<?
foreach ($users as $user) {
echo '<p>'.$user['name'].'</p>';
}
?>
Запускаем тест — OK, ничего не сломали. Разделяем для удобства на два файла, иными словами выносим всё что связано с html в отдельный файл index.php.html:
<?php
// index.php
include 'DBConnector.php';
$DB = new DBConnector;
$DB->query('SELECT * FROM users LIMIT 10');
if($DB->get_num_rows()){
while($user = $DB->fetch_row()){
$users[] = $user;
}
} else {
$users = array();
}
include 'index.php.html';
<h1>Пользователи</h1>
<?
// index.php.html
foreach ($users as $user) {
echo '<p>'.$user['name'].'</p>';
}
?>
Проверяем — ОК. Некоторые умные люди то, что у нас сейчас в новом файле называют представлением или видом. Но про него пока забываем. Хотя нет, сделаем его чуть посимпатичнее (на вкус и цвет...).
<h1>Пользователи</h1>
<?php foreach ($users as $user): ?>
<p><?=$user['name']?></p>
<?php endforeach; ?>
Запускаем тест — Failed! Мы изменили представление и тест не проходит. Но мы знаем, что в HTML пробелы не значимы и добавляем их в тест (никогда так не делайте! :) ). Теперь всё ОК. Возвращаемся к нашему скрипту. Бросается в глаза, что он сильно зависит от базы данных. И не исключено, что в других скриптах нашего приложения такие выборки повторяются часто. Да и вообще как-то все эти $DB мельтешат в глазах и надо разбираться что они делают. Ну что же, следуем как бы DRY и выносим работу с таблицей users в отдельный метод отдельного класса, соединение с БД передадим через конструктор, не глобальную же переменную делать. Назовём класс, ну, скажем, UserRepository.
<?php
class UserRepository
{
private $db_connector;
public function __construct(DBConnector $db_connector)
{
$this->db_connector = $db_connector;
}
public function getAll($limit=10)
{
$this->db_connector->query("SELECT * FROM users LIMIT {$limit}");
if($this->db_connector->get_num_rows()){
while($user = $this->db_connector->fetch_row()){
$users[] = $user;
}
} else {
$users = array();
}
return $users;
}
}
Код перенесён почти без изменений, добавлен только параметр limit. Машинально добавил :(, по-хорошему нельзя было. Но изменение простейшее и надеемся, что ничего не сломается. Но сначала нам надо изменить наш скрипт
<?php
require_once 'DBConnector.php';
require_once 'UserRepository.php';
$user_repository = new UserRepository(new DBConnector);
$users = $user_repository->getAll();
include 'index.php.html';
Тест? OK! (На каждом шагу рефакторинга нужно запускать тесты, чтобы потом не было обидно за бесцельно прожитые годы в поисках «где же напортачили»). Если отбросить «служебные» require_once, то файл нашего скрипта сократился до трех строчек: создаём репозиторий, получаем из него всех пользователей, выводим их в представлении. Ежу, наверное понятно будет с первого взгляда. Умные люди такой скрипт называют контроллером, причём тонким. Ну это так, для справки.
Взглянем ещё раз на наш репозиторий. Ни у кого не возникает диссонанс — создали класс «Репозиторий пользователей», объекты которого возвращают массив ассоциативных массивов (aka хэшей)? У меня возникает. Пускай он возвращает хотя бы массив объектов с оригинальным именем User.
while($user = $this->db_connector->fetch_row()){
$users[] = new User($user);
}
Ну и сам класс User, который умные люди называют классом модели предметной области или просто классом модели.
<?php
class User
{
private $name;
public function __construct(array $properties)
{
$this->name = $properties['name'];
}
public function getName()
{
return $this->name;
}
}
Изменяем представление для работы с массивом объектов
<h1>Пользователи</h1>
<?php foreach ($users as $user): ?>
<p><?= $user->getName() ?></p>
<?php endforeach; ?>
и добавляем декларацию класса модели в контроллер.
<?php
require_once 'DBConnector.php';
require_once 'User.php';
require_once 'UserRepository.php';
$user_repository = new UserRepository(new DBConnector);
$users = $user_repository->getAll();
include 'index.php.html';
Запускаем тест — всё ОК! Пожалуй на этом можно пока остановиться. Подведём итоги:
— поведение скрипта практически гарантированно не изменилось. Не считая нескольких проблеов появление которых мы заметили и задокументировали. Где? В тесте! Тест — это и документация к основному коду.
— представление у нас отделено от всего остального, всё что ему нужно, чтобы был в его области видимости массив users из объектов с методом getName(). Можем тестировать его отдельно.
— сложная (хе-хе) работа с базой данных у нас инкапсулирована в репозитории что не отвлекает от изучения логики приложения, но само соединение создаётся вне его, что даёт возможность а) подставлять соединения с разными БД и б) подставлять по настоящему фэйковые (стабы и моки) экземпляры соединений для тестирования и даже в) почти ничего не изменяя использовать любое другое хранилище — файлы, NoSQL СУБД и даже файлопомойки облачные файлохостинги.
— объекты модели по сути не зависят от БД вообще, да и вообще ни от чего, это так называемые POPO — Plain Old PHP Objects (плоские старые PHP объекты). Пока по сути никакой логики в них нет, но когда появится её также можно будет тестировать отдельно от всего остального.
— работа основного скрипта почти очевидна, только последний include почти ни о чём не говорит, но выделять его в функцию/метод мне уже было лень
Что ещё можно сделать с кодом для улучшения усложнения архитектуры:
— ещё больше абстрагироваться от БД, а то и вообще типа хранилища.
— абстрагироваться от типа представления (нашего include) — можно сделать, чтобы без проблем оно рендерилось каким-нибудь шаблонизатором — Smarty, Twig и т. п.
— сделать контроллер тоже объектом класса
— использовать паттерн FrontController
— без особого труда прикрутить другие сторонние компоненты (ORM, роутеры, конфиги и т. п.)
— перевести код на фреймворк, например Symfony2 :)
— покрыть нормальными тестами, от модульных (юнит) до приёмочных.
Если есть интерес, то это может быть первой статьёй небольшого цикла. Только сначала бы не помешало усложнить исходный пример говнокода до чего-то более-менее работающего и хоть немного неочевидного и говнистого :) Если есть желающие то реп на гитхабе github.com/VolCh/refactor-sample
Автор: VolCh
