Покритикуйте реализацию решения

Духовность™

Продвинутый новичок
Покритикуйте реализацию решения

В попытках написать что-то в стиле "MVC ради MVC", что с грохотом провалилось, придумал некую архитектурную композицию, которая, как мне кажется, должа быть представленна типовым решением (Фаулера читаю, но пока что только до II главы дошел, ещё ничего не переварил как следует).

Разберем на примере сценария листинга статей, "из которого" могут идти запросы на
- поднятие позиции статьи
- опускание позиции статьи
- установка темы дня
- установка темы рубрики

ВСЯ логика, обеспечивающая эти 4 действия, сосредоточена в конкретных методах класса articlesListController, в которых, в случае необходимости, могут вызываться методы самой модели "статья" для тех или иных изменений в статье.

На самой странице сервера (определение по Фаулеру) логики нет. Абстрактно это выглядит так:

PHP:
class articlesListController
{
    // Поднять статью "выше"
    public function processMotionUp()
    {
        // логика

        // используем модель статья
        $this->article->motionUp(...);
    }

    // Опустить статью "ниже"
    public function processMotionDown()
    {
        // логика

        // используем модель статья
        $this->article->motionDown(...);
    }

    // Установить тему дня
    public function processSetTheme()
    {
        // логика
    }

    // Установить тему рубрики
    public function processSetRubric()
    {
        // логика
    }
}
полная версия: http://phpclub.ru/paste/index.php?show=2182

В самом скрипте articles.list.php это вызывается примерно так:

PHP:
//...

$articleController = new articlesListController();

if ($articleController->processMotionUp())
{
    redirect('Статья поднята на позицию выше.');
}
else if ($articleController->processMotionDown())
{
    redirect('Статья опущена на позицию выше.');
}
else if ($articleController->processSetTheme())
{
    redirect('Тема дня установлена.');
}
else if ($articleController->processSetRubric())
{
    redirect('Тема рубрики установлена.');
}
Это всё. Меня интересует, насколько данный подход оправдан и что я вообще такое сделал?
 

флоппик

promotor fidei
Команда форума
Партнер клуба
и что я вообще такое сделал?
поздравляю, ты придумал MVC! :)

Совместил контроллер с моделью. Минус этого в том, что ты не можешь изменить модель не изменяя контроллер, а в MVC — можно. Например, переименовать поле в таблице. Тебе надо будет отредактировать _все_ контроллеры, работающие с этой таблицей, а не всего лишь одну модель.

-~{}~ 16.10.08 15:09:

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

asterisk

Новичок
triumvirat
я конечно не фaнaт ооп но лично мне не понравилось в коде:
1. в функциях по несколько точек выхода (return)
2. однотипные запросы к БД можно объединить в один конструктор
3. какие то ограниченные возможности sql конструктора, имя таблицы явно вставляется через разрыв строки а условия через знак "?", ну и приведение типов данных к интам на уровне php вместо использования конструкций типо "?i"
 

Духовность™

Продвинутый новичок
флоппик
Хорошо, спасибо.

Тогда становится непонятен один момент - вот этот класс предназначен для конкретного сценария - articles.list.php, который выводит список статей.

Но у меня есть ещё один сценарий - articles.edit.php, который редактирует статью. И для него был написан собственный класс-контроллер... внимание - фактически в один публичный метод - setData().

Мне не ясно, правильно ли я сделал, разделив классы-контроллеры.

-~{}~ 16.10.08 12:18:

ты не можешь изменить модель не изменяя контроллер
это ты о SQL коде в методах класса?
 

флоппик

promotor fidei
Команда форума
Партнер клуба
Мне не ясно, правильно ли я сделал, разделив классы-контроллеры.
Я честно говоря, не уверен, есть ли какие-либо конкретные рекомендации на этот счет, возможно, кто-нибудь еще это прокомментирует и поправит или дополнит меня. Я это делаю обычно следующим образом: я беру одиночный функционал, и изучаю его, допустим, реализацию статей. Я вижу, что у него есть функционал для редактирования, отображения, добавления и удаления статей. Я вижу _потенциал_ расширения этого функционала. И я вижу, что некоторые из этих действий над статьями — тоже комплексные. Я могу их отделить в отдельный контроллер каждый. Я вижу, что еще есть пара методов, применимые к _набору_ статей. (отображение списка, отображение списка с анонсом статьи)... но я не вижу, что можно расширить там, и я скорее включу эту возможность просто в контроллер, отвечающий за отображение. Примерно, так. Т.е. для меня разделение на несколько контроллеров — вещь несколько субъективная, чем подчиненная академическим правилам.

-~{}~ 16.10.08 15:31:

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

Fred

Новичок
Re: Покритикуйте реализацию решения

Автор оригинала: triumvirat
PHP:
//...

$articleController = new articlesListController();

if ($articleController->processMotionUp())
{
    redirect('Статья поднята на позицию выше.');
}
else if ($articleController->processMotionDown())
{
    redirect('Статья опущена на позицию выше.');
}
else if ($articleController->processSetTheme())
{
    redirect('Тема дня установлена.');
}
else if ($articleController->processSetRubric())
{
    redirect('Тема рубрики установлена.');
}
Что тут должно происходить? Судя по коду, сначала мы пытаемся поднять статью в рейтинге. Если не удалось - пытаемся опустить. Если не удалось, сставим тему .... и т.д. Может я что-то не понял?

-~{}~ 16.10.08 13:06:

Автор оригинала: asterisk
1. в функциях по несколько точек выхода (return)
Так это же удобно :)

Автор оригинала: asterisk
3. какие то ограниченные возможности sql конструктора, имя таблицы явно вставляется через разрыв строки а условия через знак "?", ну и приведение типов данных к интам на уровне php вместо использования конструкций типо "?i"
Параметры в условии потом не обязательно приводятся к инту. А использование ? - стандартная практика для неименованных placeholder'ов.
 

Духовность™

Продвинутый новичок
Что тут должно происходить?
это тоже самое, что и делать проверку на isset($_REQUEST['var']) с последующим сравнением и определением действия. http://phpclub.ru/paste/index.php?show=2182

Вообще, давайте код не разбирать, тема о архитектуре.


флоппик
спасибо. переварю все тобой сказанное
 

Fred

Новичок
Автор оригинала: triumvirat
это тоже самое, что и делать проверку на isset($_REQUEST['var']) с последующим сравнением и определением действия. http://phpclub.ru/paste/index.php?show=2182

Вообще, давайте код не разбирать, тема о архитектуре.
Не самое разумное решение - перебирать все действия контроллера в поисках нужного. Если уж на то пошло, то

PHP:
switch($_REQUEST['var'] {
  case "motionup":
    $controller->processMotionUp() ;
  case "motiondown":
    $controller->processMotionDown() ;
  // ...
}
в articles.list.php выглядит намного нагляднее и понятнее. Или сделай в самом ArticleController метод dispatch и уже в нем

PHP:
public function dispatch() {
  switch($_REQUEST['var'] {
    case "motionup":
      $this->processMotionUp() ;
    case "motiondown":
      $this->processMotionDown() ;
    // ...
  }
}
 
Сверху