Pull to refresh

Comments 54

А как быть с тем, что код с примером из пункта 1 генерирует сам webapp?
Или с тем, что похожий код генерирует gii?
А сотни примеров кода из гайдов?
Сказать Qiang Xue, что он злобный буратино, предавший каноны MVC?
Почему бы и нет? Первый Yii уже давольно стар (или зрел, кому как нравится) и всякое «исторически сложилось» там вполне может существовать.
В yii2 они сообразили сделать отдельный класс View и это, имхо, хорошо.
Обязательно сказать, только аргументировано и с примерами. А ещё лучше в виде pull-request.
И ответит он что то типа: «Я разработчик, мне виднее. Идите в лес»?
Скорее нет, чем да. Особенно при хорошей аргументации. К тому же, Qiang не единственный разработчик в команде Yii.
Ааарр!!! Я в свое время писал на Yii довольно крупный проект. В целом все удобно и просто, но что меня больше всего бесит в Yii — это прямое присваивание публичных полей в коде примеров. Подобный подход очень очень очень очень небезопасен! Ваши данные могут быть изменены (т.к. mutable) каким-нибудь багом внутри Yii и поломать всю бизнес логику и даже данные в базе.
Возможно, я не достиг уровня «будды» в программировании, но, если честно, я не понимаю повсеместного использования геттеров-сеттеров. Как бы да, в некоторых местах это необходимо. Например, критерия может быть в двух видах: CDbCriteria и array. В таком случае логично переписать сеттер который приводил бы тип к нужному. Но абсолютно везде — имхо, это лишний код и тупо усложняет.

Что касается приведенного кода, то, во-первых, код чисто для примера, и многое там опущено дабы не усложнять. Во-вторых, там мы не присваиваем никаких безопасных свойств: все присвоения атрибутов модели идут через setAttributes, который в свою очередь смотрит на валидаторы, и при сохранении проверяет валидность. Другие присвоения (например, присвоение критерии EMongoDocumentDataProvider) реализованный внутри соответствующего расширения и нам не надо заботиться об этом. Присвоения свойств экшена — это уже чисто на нашей совести. Но Yii нам помогает: если мы присвоим несуществующую вьюху или скоуп — он нам однозначно даст нам об этом знать.

Насчет «багов Yii» и «поломает бизнес логику и данные»: Что касается багов Yii, то я доверяю разработчика Yii, и баг Yii — это в какой-то степени форс-мажор. Но, естественно на «безбаговость» не приходиться рассчитывать — сломаться может не только Yii, а любой экстеншн, или даже собственный код. Для этого у нас есть пост-валидация бизнес-операций. И в случае ошибки — откатывать всю бизнес-операцию, с помощью транзакций или как-то по другому. В этом посте я не описал эту тему, ибо он и так слишком большой, плюс тема бизнес-операций — это тема «М». Это все я опишу в следующем посте.
Повсеместные геттеры и сеттеры — это страховка от изменения бизнес-логики. Если логика поменялась, нам не надо искать по всему проекту, где используется то или иное поле — доработка геттера устраняет все возможные проблемы. По поводу побочных эффектов — «лишний» код легко генерируется, а усложнение — да, в некоторой степени, но в первую очередь для человека, который сам так не делает. Если человек сам привык передавать всё через геттеры и сеттеры, код в таком стиле не доставляет никаких неудобств.
В качестве приятного бонуса — геттеры и сеттеры делают код более инкапсулированным и тем самым положительно влияют на архитектуру и карму.
В Yii есть поддержка обращения к виртуальным полям через геттер или сеттер. То есть:

class X extends CComponent
{  
  public function setField($value)
  {
    // ...
  }

  public function getField($name)
  {
    // ...
  }
}

$x = new X();
$x->field = 'test';
echo $x->field;


Это позволяет везде использовать поля и вводить геттеры и сеттеры прозрачно как только они реально понадобятся.
Вообще, конечно, хотелось бы, чтобы все поля были приватными по умолчанию, и только если нужно — меняется на public или пишутся геттеры-сеттеры. Причем, логика прав доступа должна распространяться не только на прямой доступ к полю, но и на методы вида updateAttributes(array newAttrs) (или как оно там называется в Yii). Но понимаю, что такое решение будет непопулярным.
Это здорово, но потерю типизации никто не отменял.

И если для сеттера можно сделать проверку на уровне языка:
public function setUser(User $user) { ... }
то для обращения через виртуальные поля требуют ручной валидации кода.
Вроде ровно наоборот получается. Пока поле обычное паблик свойство, то типизации нет, но как только мы переводим его в виртуальное (пишем сеттер и делаем его приватным или вообще меняем логику хранения), то получаем и типизацию, поскольку обращение пойдёт через сеттер.
Если логика поменялась, нам не надо искать по всему проекту, где используется то или иное поле — доработка геттера устраняет все возможные проблемы.
Поискать по всему проекту не сложно — достаточно 2 кнопки нажать.
На меня конечно же в некоторой степени повлияло несколько лет программирования на Java и Scala.
Если предметная область правильно спроектирована, у нас всегда будет простой способ получить связанные данные. И это абсолютно решает проблему с тем, что контроллер нам не передал список заказов.

и далее
foreach($client->orders as $order)
и Yii вам нагенерит по 1 запросу к БД на каждый заказ.
Это не так, ибо полностью зависит от реализации $client->orders. Например, если он реализован с помощью релейшена AR — выборка будет единоразовой. В трейте для релейшенов yiimongodbsuite, который я написал, данные так же кешируется.

Если ArrayAccess написан так, что при каждой итерации будет производиться запрос — вы правы. В любом случае, это полностью лежит совести разработчика.
Если в AR-модели есть релейшн orders, то загрузка не будет ленивой только в случае выборки с with('orders').
Автор прав, вы путаете с чем-то таким:
foreach(Client::model->findAll() as $client) {
    $order = $client->order;
    ...
}
Зависит от реализации. И даже в случае если загрузка ленивая, то запрос может быть только один дополнительный типа SELECT * FROM order WHERE id IN (<список айдишников полученных ранее>)
И иногда это является единственным способом разгрузить БД ничего в ней не меняя.
Круто, спасибо! Взял у вас пару рецептиков.
Первый пример очень спорный. Вы просто перекидываете дублирование кода с одного места на другое. Вдобавок, в рамках MVC вьюшка должна заниматься только тем, что следует из её названия — показывать пришедшие данные. А выборки, фильтрацию и прочее — задача модели и контроллера. Вы, по сути, делаете ненужным контроллер как таковой. Это уже ближе к идеалогии MV-V-M, гда ваш контроллер является просто связующим.
На мой взгляд, самым грамотным решением будет такая архитектура, при которой View принимает обобщенный список (допустим, User::model()), в котором контроллер уже выставил свои ограничения. А View уж как-нибудь сам разребется, как извлекать данные.
Например, у нас есть список товаров: Goods. Нам нужно показать только те, у которых цена больше какой-то. В контроллере: Goods->AddFilter('price', '>', $price); и вызывать необходимое представление. Оно может быть общим для любого количества товаров. Предсталение вообще ничего не должно знать о том, с какими параметрами его запросили. И с таким подходом контроллеру не важно, как работает представление внутри — главное, чтобы обрабатывало именно этот класс
Насчет первого примера ошибаетесь. Автор как раз все правильно описал. Задача получения постов просто уходит в модель один раз и вызывается когда нужно, а не решается каждый раз в контроллере. Представление же просто обращается к постам пользователя $user->posts, причем даже более логично чем просто $posts.
Насчет правильной архитектуры вопрос, на мой взгляд, субъективный для каждой задачи. Здесь мне кажется не нужно закапываться в идеологию, когда можно сделать проще для маленькой задачи или проекта.
$user->posts

А теперь добавьте пагинацию, сортировку по рейтингу, фильтр по какому-нибудь критерию. Через реляцию уже не получается(
Так или иначе эти условия должен задать контроллер. Контроллер может вернуть настроенную модель, массив постов или dataProvider. А что именно он вернет — суть дела не сильно меняет. Хотя вызов find() во вьюхе мне определенно не нравится.

userWithPosts.php:
<?php
$this->renderPartial('user', ['user' => $user]);//Эта вьюха понятия не имеет про посты пользователя
$this->renderPartial('posts', ['posts' => $posts]);//А эта - не знает ничего, кроме постов. И скорее всего предпочитает CDataProvider

Таким не хитрым образом мы проводим декомпозицию задачи, получаем гибкое и повторно-используемое решение без лишней копипасты.
Мне кажется вы не там ищете гибкость. На конкретную задачу используется своя вьюха, у нее смысл такой: отобразить данные по-своему. А в вашем примере, достаточно добавить одно условие к конкретной странице и она уже не такая гибкая, допустим на странице пользователя отображать посты без количества комментариев, а на странице постов с ними.
В любом случае, как я уже выше писал, любой подход не панацея, нужно во-первых использовать мозг и только потом шаблоны.
Серебрянную пулю я и не обещал. Здесь просто буква S из SOLID.

А в вашем примере

Равно как и в вашем и у ТС ;) Каждый случай индивидуален.
Вы открыли CDataProvider, он работает именно так.
В вашем случае:
$dp = new CDataProvider('Goods', $criteria);
$this->render('view', ['dataProvider' => $dp]);

В остальном же, полностью согласен
>>выборки, фильтрацию и прочее — задача модели и контроллера.
Популярное заблуждение, связанное с повсеместным внедрением ОРМ с автоматизированными квери-билдерами. Против SQL в контроллере любой немедленно возмутится, однако, когда тот же SQL в том же контроллере написан неявно — все почему-то довольны.

На самом деле, выборки и фильтрации делают тоже модели. Контроллер может только сказать модели, что он хочет — причем (это важно!) в терминах бизнес-логики, а не в терминах полей в таблице.
На самом деле, выборки и фильтрации делают тоже модели.

Популярное заблуждение :), связанное с повсеместным введением паттерна ActiveRecord, в котором сочетаются две ответственности: моделирование бизнес-логики и обеспечение персистентности. Да, на модель можно повесить функции фильтрации без знаний контроллера о её внутренней реализации, то тогда задачей контроллера становится загрузка всех данных в модель, а уж она будет там фильтровать в памяти в каком нибудь методе типа getPublicPost(), но обычно рационально выбрать из БД хранилища только посты с флагом is_public (если анализируя вызов контроллера, зная о реализации модели и вьюхи, мы понимаем что другие не понадобятся). Задача (одна из) контроллера подготовить модель к совершению операций бизнес-логики и/или отображению. В случае максимально изолированной модели (POPO) это означает либо подготовка всех необходимых данных для модели в контроллере, либо в передаче ей сущности (репозиторий, сервис, дата-провайдер и т. п.), которая ей эти данные предоставит (в терминах бизнес-логики). И даже если совмещать в одном классе хранение и логику (условно контроллер как бы передает $this как сервис хранения), то вызывать из метода бизнес-логики функции фильтрации этого сервиса означает опять смешивать ответветственности не только в рамках класса, но и в рамках метода, а также писать завуалированные sql запросы в них. :)
Против SQL в контроллере любой немедленно возмутится, однако, когда тот же SQL в том же контроллере написан неявно — все почему-то довольны.

Спорно. Не любой возмутится даже против явного, не говоря о неявном. Задача контроллера проинициализировать модель перед вызовом метода бизнес-логики и/или отображением, а потом сохранить состояние, если оно изменилось. Методы бизнес-логики (или геттеры для представлений) не должны дергать БД или иное хранилище напрямую, а значит контроллер или должен обеспечить модель всеми нужными данными до их вызова (явный или неявный SQL), либо предоставить ей ссылку на сущность, которая это позволяет (пускай даже на саму себя). Но в методе бизнес-логики дергать явный или неявный sql не есть гуд, имхо — данные или уже должны быть, или sql должен дергаться как минимум за одним уровнем абстракции в виде метода, который из терминов бизнес-логики (это важно! :) соберет запрос к базе, пускай даже этот метод в том же классе находится.
Нет.

Если говорить об Active Record, то он и состоит в объединении бизнес-логики и логики работы с хранилищем в рамках класса модели (паттерн или антипаттерн это — отдельный разговор, не относящийся к MVC).

Если говорить о Data Mapper, то DM тоже является частью «M» в MVC, но никак не является частью контроллера.
Data Mapper не является никакой частью MVC. Он вообще вне ее, т.к. это инфраструктурный слой, а MVC это слой приложения.
Ну это смотря как посмотреть. Вы же не станете использовать DM во view?
Если вдруг на странице пользователя сказали вывести нечто к пользователю не относящееся, типа список последних десяти постов, то почему нет? В контроллер UserController::Show это запихивать?
А обращаться к нему где?
В описанном в статье подходе это можно легко провернуть, внедрив в шаблон виджет, которому передали data provider.
Но от этого дата провайдер не становится частью модели, он часть системы хранения.
Верно. Модель, если исходить из ее общепринятого определения, как упрощенного образа реального объекта, точно не может себя выбирать. Так что обязанность выборки и остальных операций снимается с нее.
Этого часто не понимают, особенно при использовании ActiveRecord — поместить метод выборки с фильтрацией или сохранения в класс модели ещё куда ни шло, но вот писать код фильтрации или сохранения прямо в методах бизнес-логики — как говорится, руки бы поотрывать. Вот из недавнего — есть задача, к ней могут быть комментарии. Хранится в виде таблиц task и task_comment со связью один-ко-многим, маппится на классы Task и TaskComment соответственно. Есть метод Task::addComment($commentString), который вызывается из контроллера, а в нём код типа $commentRow = (new TaskComment)->setContent($commentString)->save(); то есть бизнес-операция (создание сущности и тсановка её свойств) и операция сохранения дергаются в одном методе и контроллер должен об этом знать.
Пока они объединены лишь в рамках класса, то это ещё более-менее нормально (разделение бизнес-логики и логики хранения осуществляется по методам). Как только только это смешивается в одних методах, то поддержка, рефакторинг, повторное использование и т. п. превращаются в ад в проектах посложнее «бложика».
Я согласен, что AR — проблемный паттерн. Но это не причина переносить в контроллер хоть какую-либо логику, кроме инициализации и связывания M и V. Для загрузки моделей по критериям есть Data Mapper, для сложной «обвязочной» работы — сервисы (и вот их уже можно считать частью C, но сервисы тоже ничего не знают о полях в базе ;).
Так выборка только тех данных, которые потребуются, и является инициализацией модели. Пускай система показывает машины. Есть модель Car с свойством color, есть метод контроллера show($id) и list($color_filter = null). Когда в первом мы дергаем что-то вроде $car = Car::findByPK($id) — это к бизнес-логике не относится, простая инициализация, чистая логика хранения, так? Почему же когда во втором дергаем что-то типа $cars = Car::findByColor($color), то это становится бизнес-логикой — ведь та же инициализация?
Car::findByColor — это нормально. В базе, может, вообще три поля r, g, b — мы от этого абстрагированы.

Я про код в контроллере условно вида

Car::findAll()->where('color', '=', $color)->and('is_hidden', '=', false)->orderBy('updated')

— вот такое как раз должно скрываться внутри того же DM.
Так это абстрагирование ответственности контроллера по инициализация модели от логики хранения, но к самой модели оно никакого отношения не имеет, разве что чисто случайно находится в тех же классах из-за выбранного паттерна ORM, и то не факт, если есть отдельный класс типа CarTable.
Именно по этому я написал что иногда разграничивать контроллер и вью — нецелесообразно. Но приведенный подход как минимум может сделать код реюзабельным:

   public function actions()
    {
        return [

            'view1' => [
                'class' => ARViewAction::class,
                'modelClass' => User::class,
                'criteria' => [
                    'with' => [
                        'orders' => [
                            'condition' => 'sum < 1000'
                        ]
                    ]
                ]
            ]
        ];
    }


Вот пример когда вьюха подразумевает что ей передают дата-провайдер:

   public function actions()
    {
        return [

            // Если вьюха подразумевает что ей передают dataProvader'ы
            // в AR метода getRelationCriteria(relationName, additionalCriteria) нет, но его можно самому дописать
            'view2' => [
                'class' => ARViewAction::class,
                'modelClass' => User::class,
                'dataProviders' => function(User $user){
                        return [
                            'ordersDataProvider' => [
                                'modelClass' => Order::class,
                                'criteria' => $user->getRelationCriteria('orders', 'sum < 1000')
                            ]
                        ];
                    }
            ],
        ];
    }
Словно из сна вышел, прочитав вашу статью. Более того — многое из написанного кажется очевидным, что верно свидетельствует о правильности предложенных подходов.

Спасибо!
Хороший пост. Yii знаю довольно поверхностно, поэтому нюансы решений должным образом оценить не берусь, но подняты проблемы, которые существуют и в других MVC-фреймворках, причём не только на PHP, особенно с использованием паттерна Active Record. Потому было бы неплохо добавить пост в хаб PHP как минимум, и, наверное, «Проектирование и рефакторинг»
Спасибо за хороший пост про «настоящий MVC».
Совсем вскользь упомянут вопрос тестирования, который на самом деле является очень важным.
1) Тестирование бизнес-логики становится на порядок проще, поскольку сильно снижается трудоемкость написания модульных тестов. Не оставляя логики в контроллерах, достаточно тестировать только модели.
2) Модульное тестирование контроллеров — это очень скользкая тема, поскольку все чаще превращается в функциональное тестирование. Но опять же, оно становится намного проще, если в контроллере минимум связанной бизнес-логики.

Возвращаясь к MVC, контроллер должен обрабатывать (принимать) пользовательские данные и передавать их в модели. В php в целом прием пользовательских данных — тривиальная задача, поэтому контроллеры вырождаются в модель-контроллеры, что экономит время, силы, память (разработчика) при первичном прототипировании, но в дальнейшем усложняет поддержку.
Тут следует научиться видеть границу и соблюдать меру. Если в приложении единственных экшн, который делает вывод типового запроса к базе, можно обойтись шаблонной моделью и контроллером. Но как только начнет появляться новый код — необходимо прибегнуть к рефакторингу.

В статье есть пример про универсальный CRUD-контроллер. К сожалению, эта универсальность разбивается в дребезги уже при небольших различиях в требованиях к выводу данных. Начинается лапша с конфигурацией, множество реализаций или еще какие-нибудь костыли. А все ради того, чтобы не копипастить инициализацию моделей в нескольких экшнах. Отличный пример из мира Yii — CListView.

В заключении, хочу сказать, что Yii — это очень мощный и гибкий инструмент. Вариантов правильного применения множество, но только опытный разработчик сможет задействовать всю мощь без ущерба себе. Поэтому, мой совет — оглядеться по сторонам и посмотреть, как устроен мир других фреймворков, поинтересоваться причинами. Не хочу холиворить, но я бы посоветовал освоить symfony2 под крылом опытного коллеги, а потом с новыми знаниями возвращаться в Yii.
Но все же во вьюхах его непосредственное присутствие создает путаницу. Я много раз видел (да чего греха таить, и сам так делал), как логику представления выносили в контроллер (какие-то специальные методы для форматирования или что-то подобное) лишь по тому-что контроллер присутствовал во всех его вьюхах.

Можно пример?

Еще вопрос как быть с логикой вывода во вьюхах? К примеру если есть посты — вывести, если нет — сказать что постов нет
Еще вопрос как быть с логикой вывода во вьюхах? К примеру если есть посты — вывести, если нет — сказать что постов нет

Смотрим по модели.Получаем что-то вроде:
<?php /** @var Post[] $posts */ ?>
<?php $posts = $user->getPosts() ?>
<?php if (count($posts) > 0): ?>
  <ul>
    <?php /** @var Post $post */
    <?php foreach($posts as $post): ?>
      <li>
         <h1><?php echo $post->getTitle() ?></h1>
         <?php echo $post->getContent() ?>
      </li>
    <?php endforeach ?>
  </ul>
<?php else: ?>
  <?= _('no posts') ?>
<?php endif ?>

Спасибо, я думал что то другое будет.
Какая замечательная статья. Который раз её перечитываю. Большое спасибо автору.
Only those users with full accounts are able to leave comments. Log in, please.