views:

65

answers:

3

Hello,

MVC is a really good pattern, but sometimes it is really boring to put everything into Controller's methods. Controller is constantly growing and it takes time to get rid of thousands of code lines. Some people highly recommends to put as much as possible into Model, but I prefer to keep Model clean (I don't put controller oriented methods into Model).

The idea is to put each Controller's action into own class...

class Post_Add {}
class Post_Remove {}
class Post_View {}

All code, which is common for all action classes we're putting into class Post_Parent and passing it's instance into action constructor.

So, calling action will look like...

$parent = new Post_Parent();
$action = new Post_Add($parent);
$action->run();

So, what we have?

  • Each action is in separated class, so we can add as much private methods, vars, constants as we want.
  • All common code is separated into parent class (Post_Parent) and can be accessed from action classes. It is very good for organizing ACL etc.-

Is this idea worth living? Is there any similar design patterns for this?

Thank you.

+1  A: 

Hello,

Personally I don't think the pattern you describe is going to serve you well in the long run. If your controllers already have "thousands of code lines" you've got a general problem of encapsulation, and creating a class per action is just going to shift your problem into a different layer.

Your controllers should be thin. You have already recognised this by writing your post. A controller should orchestrate the interaction between your views and your model. The model is where your business logic lives, so your controller should only have enough logic to ensure that the appropriate validation is carried out, that the correct business logic is invoked, and to return the right views when business logic processing has completed.

Øyvind
Please, could you tell me where can I read about "general problem of incapsulation"? Saying "general problem of incapsulation" you mean I should more refactor and put as much methods into separated classes to be shared over all controllers etc.?
Kirzilla
I would recommend reading the following two links as a primer: http://en.wikipedia.org/wiki/Encapsulation_(object-oriented_programming), and http://en.wikipedia.org/wiki/Solid_(object-oriented_design). Then I would encourage you to read more about SOLID principles: http://butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod
Øyvind
A: 

I'd suggest to back up a little and think why are the controllers growing so much. Maybe you could do a refactoring and extract some shared components into separate modules? Maybe you have some code in your logic that should be in your model or view instead? Do you use a good template system?

Splitting controllers into smaller pieces will not solve the underlying problem, only sweep it under the rug.

Adam Byrtek
I'm using Smarty as template engine. My controller's methods mostly contains of strings like $data = $model->getItems(); $smarty->assign("data", $data); And this code really depresses me... :(
Kirzilla
A: 

Have a look at the Transaction Script and PageController patterns. Transaction script is the most basic of the Domain Logic patterns and suited for small applications. A PageController's purpose is to handle input from your UI. If you want that to be a single command, that's okay. You could do

class PostAddController implements RequestHandler {
    public function handle($request) {
       $post = filter_input(INPUT_POST, 'post', FILTER_SANITIZE_SPECIAL_CHARS);
       $model = new PostAddTransactionScript;   
       $model->process($post);
       include 'postAddViewScript.php';
    }
}

PostAddTransactionScript would then write the $postData to the database or whatever it is supposed to do. The simplified example above would still be in accordance to MVC because it keeps the Model logic inside the transaction script and the input handling inside the presentation layer.

Whether you organize your input handling logic into a single Controller class or many smaller commands is up to you. Grouping responsibilities just makes more sense, especially if you need to share state or common functionality between the Commands.

As for your example, I'd rather use the Strategy Pattern and have the Post_Parent use a Command instead of the command using the Parent, e.g.

$commander = new PostCommander;
$commander->setStrategy(new PostAddCommand);
$commander->handle($_POST);

In any case, I agree with the others that your controllers should be thin and the model should do the main work.

Gordon