Pull to refresh

Comments 82

Вот только надо помнить, что для такой маленькой задачи — это отчетливый оверхэд ;)
Но как пример теперь гораздо больше похоже на правду.
Естественно :) Хотя в случае простого CRUD приложения в одном файле а-ля гостевая книга я бы уже не был так уверен.
Если сущность только одна, то тут уже зависит от сложности этой самой сущности и сложности ее валидации.
Если сущность из двух-трех полей — ну нафига городить?
А собственно чего городить? Выделить сущность и её репозиторий. Два класса. В репозитории условно 5 методов — create($user), update($user), delete($user), get($id), getAll(), в которых просто инкапсулирована работа с БД. Имхо, это лучше чем 5 функций типа create_user($db, $user) (мы же не будем как автор оригинала делать $db глобальной :) ). Какие-то дополнительные накладные расходы конечно есть, но в большинстве случаев пренебрежимы, имхо. А ООП код субъективно проще читается, чем процедурный. Кардинальный минус — полностью создается массив сущностей для передачи в представление, не всегда это приемлемо, и чтоб остаться в рамках интерфейсов над будет городить коллекцию с итератором по результатам запроса.
5 методов в одном классе без связи с другими классами и 5 методов отдельных — одно и то же ;)
Бритва Оккама — не стоит усложнять без причины.

> Кардинальный минус — полностью создается массив сущностей для передачи в представление

А это к репозитарию прямого отношения не имеет. Как реализуете — так и будет.
Там связь будет с соединением с БД.

Реализовывать-то в репозитории надо :)
Оверхэд — оверхедом, но взять большую задачу типа Wordpress, где все вперемешку, а между тем все пользуются. Парадокс. Так что с малого начинать — уже прогресс.
ну, это не парадокс. Это заказчики, которые уже привыкли к админке вордпресса и его бесплатности, и ни за что не хотят переучиваться…
Админка хороша — спору нет.
а большее заказчика и не волнует особо. Пока блог не станет сильно популярным и сервер не начнет офигивать от тьмы запросов… Хотя, говорят, эту фичу каким-то плагином можно полечить)
Это же пример.
Или вы хотели, что бы в статье отрефакторили весь Битрикс целиком описав каждый шаг в каждом файлике?
Данный пример не дает ни малейшего понимания зачем это все нужно ;)

Для нормальной иллюстрации нужен пример с несколькими связанными сущностями и crud`ом, тогда четко будет видно, в чем получена выгода.
Есть подходящий? Вордпресс не предлагать чур :)
Быстрый поиск по гитхабу — github.com/Peetra/shop
Вполне себе образчик говнокода ;)
Думаю автор не сильно обидиться, если его код порефакторят ;)
Вроде то, что доктор прописал. Но явно в формат хабротопика, даже нескольких, подробное описание полного рефакторинга не входит. Надо подумать.
Вот так постепенно можно перейти к написанию книг :-)
tac, мы ваш код уже обсуждали ;)
да там не мой :)
На четвертом листинге разве не будет ошибки с необъявленным массивом?
Что перед циклом с $users[] = $user; нет $users = array();? Нет, не будет. Из мана: «Если массив $arr еще не существует, он будет создан.» Хотя обычно объявляю сам не знаю зачем.
Ага, а если ($user = $DB->fetch_row()) == false? Массив не будет создан, несмотря на тесты выскочит ошибка.
По идее тело цикла выполнится хоть раз или сработает ветка else. Честно говоря не понимаю зачем там вообще условие с num_rows(). Может API у DBConnector такой хитрый. Было бы что знакомое (PDO, mysql) исключил бы и массив объявил в начале.
Так что нужно в любом случае заранее объявлять
Потому что следующая строка в мануале: «Однако такой способ применять не рекомендуется, так как если переменная $arr уже содержит некоторое значение (например, значение типа string из переменной запроса), то это значение останется на месте и [] может на самом деле означать доступ к символу в строке. Лучше инициализировать переменную путем явного присваивания значения. „
Кроме того, раньше это вызывало warning, как в 5.4 незнаю.
В 5.3 проверял недавно с E_ALL | E_STRICT не видел варнингов.
Признаюсь честно, всю статью не читал.
public function getAll($limit=10)
{
        $this->db_connector->query("SELECT * FROM users LIMIT {$limit}");
}

Разве тут нет потенциальной SQL-Injection?
Неизвестно :) Даже неизвестно SQL ли это :) Но вообще я написал «никогда так не делайте!»
а кто его знает, как и где там могут обрабатываться входяшие данные)
Благодарю за пост. Совет использования Symphony2 кстати очень оправдано.
Спасибо вам что «заставили» его написать :)

Собственно не совет, а хотел показать как от спагетти перейти к symfony2
Да ладно, разве ж это спагетти? Вот когда в таком стиле написан весь проект, включая архитектуру БД… И когда очень много вмешанных в код разных связей и зависимостей, вот тогда это настоящий спагетти и тогда этот подход может не сработать.

Ваш пример слишком уж похож на сферические спагетти в вакууме. То, что таким образом можно переписать этот пример ещё не означает, что этот подход сработает в другом случае.

Ну, т.е. в вашем примере уж очень атомарной является логика. Пусть вы и назвали это спагетти, но это на самом деле на него мало похоже. Слишком уж оно аккуратное. И в вакууме — не имеет никаких внешних связей и зависимостей.

К примеру, архитектурная проблема может быть в БД. В таком случае просто обернуть будет недостаточно. Нужно менять архитектуру БД, и при этом, если это делать постепенно, как-то поддерживать целостность данных.

Как пример алгоритма работы с кодом — может быть и пойдёт, но, кажется, не всегда получится действовать по такой схеме.
Мопед не мой… Код взял из другого топика. И цель поста дать кому-то толчок в правильном (ну, по-моему, правильном) направлении. А в целом подход показал свою жизнеспособность на реальных задачах, проектах с многолетней историей. В том числе, с изменениями схем БД, сложными и неявными (глобальные переменные, синглтоны) зависимостями.

Есть более подходящий пример? Не столь примитивный, но и не как тут выше написали Битрикс полностью отрефакторить и каждый шаг описать. Может open source проект какой?
Пример то сферический, но… в реальных проектах приходилось сталкиваться именно с таким…
Например в одном проекте имел дело с шаблонами и контроллерами в которых преобладающая часть кода была на SQL (ужас картины осознаётся при том, что сайт изначально был на базе ZF). Усугублялась ситуация стилем написания кода вроде восьми if'ов для отработки трёх независимых действий по состоянию трёх опций в форме и копипастой всего этого кошмара между методами контроллера с полным игнорированием существующих моделей.
Было немного не по себе, когда я сокращал сотни строчек до десятка. Оверхед? Спагетти — в разы больший оверхед, особенно когда он рандомно разбросан по проекту. Проект может себе позволить спагетти? Значит оверхед на поддержку качественного код он тем более сможет себе позволить.
Но в общем, я думаю, нам тут не о чем спорить, это скорее был крик души о наболевшем ))
Да-да-да, вот это уже ближе к реальности :) Всё видел, и повальная копипаста (с небольшими изменениями в некоторых местах — это вообще ад), и SQL в шаблонах… Вот в таком ужасе гораздо сложнее внедрять эти инструменты. И всё отнюдь не так однозначно — «нарисовали тестик, заменили прямой вызов на обёртку, инкапсулировали данные». Там в ужасе думаешь за что браться и с чего начинать, чтобы оно просто разом не умерло.

И да, страшновато когда код сокращается в разы. Боишься, что что-то обязательно должно сломаться :)
а в 5.3 вроде короткие теги выключены, нет?
Да и
<?=$example; ?>

вроде тоже не рекомендован к использованию…
Зато в 5.4 их уже нельзя отсключать и они всячески рекомендуемы к использованию (на DevConf авторы рассказывали).
Что-то я не понял. Говорили, что сделаем хорошо, и аж две версии. Я конечно php вообще не понимаю, но даже мне видно, что накой-то засунули цикл в представление. И внутри этого цикла представление обращается к бизнес-логике. В то время как представление должно быть свободно от бизнес логики… Странные у вас рефакториги — уж лучше как было оставить.
Цикл — логика представления. Где ему ещё помещаться? И где вы видите обращение к бизнес-логике? $user->getName();? А если заменить на $user->name;?
> А если заменить на $user->name;?

Ничего не изменится.

> Где ему ещё помещаться?

Вне .html, это по сути главный управляющий генерацией цикл. По сути это аналогично генерации строки

foreach (users as user):
str += "<?=" + user.getName() + "?>"

и это логика контроллера — как раз синхронизации визуализации и бизнес модели, вот где-оно якобы " лишнее копирование всех или большинства свойств из объекта User в объект UserPresentation"
т.е. UserPresentation — это и есть часть html, а хотели бы визуализировать не в браузере, а формой, контроллер этот список кинул бы в визуальный контроллер UserList

так

foreach (users as user)
UserList.Add(user.getName())

Так возможно лучше видно, что это за код? Разве подобный код это логика визуализации/представления? Нет это логика заполнения визуализации, что и есть логика синхронизации невизуальных свойств с визуальным представлением — дело контроллера.
Т.е. собственно для чего мы отделяем визуализацию от бизнес-логики, как раз для того, чтобы с легкостью заменить вывод в веб-браузер на экранную форму приложения (к примеру). Мы выкидываем представление, но вместе с ним теряем главный цикл заполнения — вот вам и проверка — правильно ли отделена бизнес-логика от визуализации.
То есть, допустим, нужно вывести не только имя, но идентификатор и код в контроллеку должен быть примерно таким
foreach ($users as $user) {
    $user_names[] = new NamePresentation(user.getName());
    $user_ids[] = new IdPresentation(user.getId());
}

$view = new ViewFabric::getHtmlView('index', PHP_RENDERER);
$view->render($user_names, $users_ids);
$view->show();


А в index.php.html таким:
<?php for($i=0; $i<len($user_names); $i++): ?>
<?= $user_ids[$i]->asHtml() ?> <?= $user_names[$i]->asHtml() ?>
<?php endfor; ?>
Ну, мне тяжело читать этот код — но вроде уже лучше
Хотя цикл по прежнему не ушел из index.php.html, вы доверили заполнение визуализации вызывая как я понимаю метод $view->render, но место для этого $view->render — как раз тут же (в контроллере), а не в визуализации
Впрочем $user_names[] — это у вас типа визуальные контроллы — тогда хоть натянуто, но годится. Просто визуальные контроллы в данном случае это и есть html, и можно не городить new NamePresentation. Тогда это скорее будет new TextBoxPresentation — тогда смысл сохраняется.
Нет, это не контролы, это значения для использования в контролах. В одних «контролах» будет использоваться $user_name->asHtml, в других $user_name->asText, в третьих $user_name->asJson, в четвёртых $user_name->asUtf32

Я вот что не пойму, где по-вашему должны динамически создаваться контролы и может ли этот процесс быть совмещён с их заполнением.
> может ли этот процесс быть совмещён с их заполнением

в общем случае нет. Это для Веба — не столь понятно, так как представление живет не долго, и не может меняться. Но по сути визуальные контроллы должны создаваться один раз при создании визуального представления/формы. А заполняться в зависимости от действий пользователя+бизнесс-логики.

Если это переварить на логику веба, то получается — статическую часть .html вы должны создавать один раз пока пользователь не покинет соответствующую страницу. А заполнять контроллы в зависимости от нажатия кнопок или галочек, заполнения полей на текущей странице.

Т.е. пусть на странице будет две кнопки — одна «Дай всех пользователей», вторая «Дай только пользователей с правом удаления»

Тогда контрол будет создаваться один раз, а заполняться в зависимости от нажатия кнопки.
При этом перерисовывать/пересоздавать кнопки не нужно, нужно просто перезаполнить список.
И даже более того, если на форме есть два списка или другие значения — они не должны пересоздаваться вытягиваться заново — они должны остаться без изменений.
а ваш код с циклом внутри index.php.html будет перевыбирать пользователей, даже если клиент просто щелкнул по какой нибудь галочке не связанной со списком пользователей.
«А заполнять контроллы» -> читай парсить участки статического .html, но уже заполненного разными данными.
Что если в зависимости от действий пользователя+бизнес-логики должны создаваться новые контролы. Собственно это основная задача для чего внедряются те же циклы в html. Пускай есть у нас какой-то контрол типа label, представляющий элемент какого-то динамичного списка list. Контроллер получает этот список из модели предметной области. Он что должен для каждого элемента создавать контрол а-ля
$model = new Model();
$view = new View();

$list = $model->getList();

foreach ($list as $item)
  $values[] = $item->getValue(); // отвязывание данных представления от бизнес-модели

for ($i=0; $i<len($values); $i++)
  $view->addLabelForValue(); //создание контролов

for ($i=0; $i<len($values); $i++)
  $view->fillLabelForValue($i, $values[$i]); //Заполнение контролов

$view->show();


Так? А если подобных списков не один, а два, три, пять, десять? Три цикла на каждый? А если эти десять списков по сути свойства одной сущности, но представляемые разными контролами? Всё равно 30 циклов? Или можно обойтись тремя? А если у нас есть два представления, скажем html, где каждый элемент из десяти списков представлен одним контролом, и есть дисплейная форма где есть контрол типа grid, то у этих представлений должен быть общий интерфейс по созданию контролов (то есть грид нужно создавать поячеечно) или допустимо что в html создаём и заполняем поштучно (по иному не получится), а в форме одним махом, двумерным массивом?

Тут вы перестарались :)

кусок

foreach ($list as $item)
$values[] = $item->getValue();

лишний

достаточно

for ($i=0; $i<len($values); $i++)
$view->fillLabelForValue($i, $item->getValue());

а если речь о инициализации, а не обновлении страницы, то

for ($i=0; $i<len($values); $i++)
$view->addLabelForValue($item->getValue());
просто в зависимости от ситуации будет или addLabelForValue (для новой страницы) или fillLabelForValue
На старой странице добавляется новый Label.
цикл там конечно будет не по len($values), а по len($list)
> допустимо что в html создаём и заполняем поштучно (по иному не получится), а в форме одним махом, двумерным массивом?

конечно допустимо, но двумерный массив надо еще получить — как правило он уже получен как результат запроса к базе данных.
Или еще для примера — где Вы располагаете такого рода код:

MyCheckBox.Value = MyBE.IsUser

где MyCheckBox — визуальный элемент CheckBox на форме, а MyBE — моя бизнес-сущность.
в index.php.html
<input type="checkbox" <?= MyBE->IsUser()? "checked" : "" ?> >
ну, не верно. Тогда никакого отделения визуализации от бизнес-логики нет. Это все равно, что я написал бы в XAML-форме обращение к бизнес-классу.
<input type="checkbox" <?= DataOfMyBE->IsUser()? "checked" : "" ?> >

лучше?
тоже самое, данные от бизнес-объекта не отделяются, иначе это не бизнес-объект, а структура — аля не объектное программирование, а процедурное
DataOfMyBE где-то в контроллере инициализируется необходимыми для view данными MyBE
тогда правильнее:

<input type=«checkbox» <?= this.MyCheckBox.Value? «checked»: "" ?> >

А где то в контроллере собственно, что и спрашивалось

MyCheckBox.Value = MyBE.IsUser
И как еще вы выведете список?
Отличайте бизнес-логику от логики представления.
— представление у нас отделено от всего остального, всё что ему нужно, чтобы был в его области видимости массив users из объектов с методом getName()

Да в том то и дело, что не отделено. Массив users — это массив объектов бизнес-логики, и к ним нельзя обращаться из представления… иначе ну никакое это не отделение — а фикция.
я не профессиональный пехепешник, но скажите, лучше создавать промежуточные словари/массивы и передавать в шаблон?
я вообще не занимаюсь веб-программированием, но на мой взгляд правильно генерить html, а не переплетать его кодом.
include 'index.php.html'; как раз и генерирует html с точки зрения контроллера. По-хорошему там должно быть что-то вроде render('index.php.html', $users); (или render('index.php.html', new IndexModel($users)); если использовать отдельную модель представления), но суть не изменится, генерация html уже изолирована от контроллера, он не знает даже есть ли там html или генерируется, скажем, json.
> include 'index.php.html'; как раз и генерирует html

Очень хорошо, но в index.php.html должен быть уже готовый .html, а не с обращением к бизнес-логике. Я конечно понимаю, что такой стиль скорее навязывается языком php — но это повод задуматься о выборе языка.
Вроде все популярные языки используемые в вебе приходят к «навязанному» PHP стилю — шаблонизации. Нет, можно, конечно вместо include 'index.php.html' написать что-то вроде:
ob_start(); // перехват вывода
include 'index.php.html';
$content = ob_get_clean(); // получение вывода как строки
file_put_contents('index.html', $content); // пишем перехваченный вывод в файл
readfile('index.html'); // выдаём файл

но зачем? Юзкейсы есть, конечно, например, кэширование условно статических страниц, но в общем случае зачем? Повышенный расход памяти и запись/чтение диска. Часто используется вариант, когда вместо последних двух строк ставят echo $content, но это если требуется, например, декорировать вывод.
Я же не призываю записывать в файл, все тоже самое можно делать в памяти, генерировать index.html на лету.
Так index.php.html и есть этот генератор html на лету. Принимает на вход модель представления (может совпадающую с моделью предметной области от лени) и выдаёт html. Можно настроить чтобы выдавал сразу в вывод, можно чтобы возвращал html как строку или, скажем, объект HttpResponse.
т.е. вот что должно быть в index.php.html перед выкидыванием его в браузер

Пользователи
Username1Username2Username3Username4
тьфу, съел разметку :)
Формально может вы и правы и нужно создавать ещё и отдельную модель представления, но на практике в подавляющем большинстве случаев это будет означать лишь лишнее копирование всех или большинства свойств из объекта User в объект UserPresentation или IndexModel.
> подавляющем большинстве случаев это будет означать лишь лишнее копирование всех или большинства свойств из объекта User в объект UserPresentation

Именно так. Это называется синхронизация свойств бизнес-модели с визуализацией. В противном случае, вы ничего не отделили.
В типовых применения PHP синхронизация односторонняя и одноразовая, представление это статический текст и не бывает необходимости получить значение «свойства» (фрагмент текста) представления в контроллере или передать его в модель предметной области.
эта вырожденность в данном конкретном случае не должна нас путать в плане отделения визуализации от бизнес-логики (см. выше habrahabr.ru/post/147438/#comment_4972862)
Sign up to leave a comment.

Articles

Change theme settings