views:

529

answers:

2

When I have a controller, e.g. article I often have a action_view() that handles most of the code.

Sometimes, it can become 80-100 lines long.

My controller usually handles all of these:

  • binding template variables
  • setting sessions (where appropriate)
  • sending emails
  • validating forms

I could see bit and pieces that I could make another private method in the controller, not necessarily for reuse, but for separation of concerns.

However, then it looks weird (to me) having methods which can be called via a route, and methods that are internal only.

Also some things say to me "I should be in the model, not the controller". However, I'm not sure if that's correct either.

In the end, I have a somewhat fat controller method that looks quite procedural.

Should I actually have a list up top of my action_* methods, and then separate the rest of my code into smaller modules?

I've got an example below... is this typical controller stuff, or should the sessions etc be in the model?

public function action_pdf($type, $id) {

        // Get PDF file from db and send headers to it
        $id = (int) $id;

        $pdfFile = $this->model->getPdf($id, $type);

        if ($pdfFile) {
           $this->request->redirect($pdfFile);
        } else {
           $this->session->set('pdfMissing', true);
           $this->request->redirect(Route::get('properties')->uri());
        }

    }

So, my question is, am I doing it wrong?

+3  A: 

Your Models are there to encapsulate business logic and usually to abstract the data storage layer away from the rest of the architecture (controllers, views).

What this means is that any database accesses (e.g. SQL queries) and such should ideally be contained within your models. Your controller will get its data from your models (this includes ORM, which exposes itself through models) without itself having to access the database directly.

As far as email sending is I guess it depends on the situation. For example, when a user signs up I call a Model method to insert their details into the database. This method then triggers an email to be sent to them. This way I'm making the email sending an explicit part of the sign-up business logic (and that is what I want in this case) so no matter who calls the sign-up method in the model, the system won't forget to send an email out.

When it comes to form validation, I try and specify most of my validation rules in the ORM model classes. This is so that no matter who manipulates the model, the model always has some inherent understanding of how to validate its own data. And you'll notice that the ORM model object already provides a method for validating its data. Any extra validation/callbacks the model does not inherently need to know about keep track off can be done outside the model code.

hiddentao
+1  A: 

IANAPHPD (I Am Not A PHP Dev) but the perspective of a general programming language (Java/C#) developer might be helpful. To me, one of the benefits of MVC is to promote code reuse when you have core business logic and infrastructure that needs to be presented with multiple UIs. For example, an order management system that has a web interface as well as a desktop interface. In this case, that core logic is the Model. Each of the three interfaces have there own Views and Controllers. In C#, it is trivial to structure your code such that the core logic is in a set of packages/assemblies that are referenced from a desktop UI project as well as a web application project.

I think in these terms even when I'm pretty sure a particular app will not need another UI. When I'm trying to decide if something should go in the model or the controller, I ask myself if that functionality should be reusable from another UI.

This is a somewhat unnatural way of thinking of PHP code, since PHP is tied so tightly to the web platform (as far as I know). Regardless, this way of thinking still holds (Separation of Concerns and Don't Repeat Yourself) can still be applied: a secondary "UI" PHP could support would be a web service API. If a feature should be available to both a website and a web service API, then it should be exposed in the Model.

You may or may not be interested, but overall, this way of thinking integrates perfectly with Domain-Driven Design.

Note: Though all the UIs supported by PHP are web-based, I'd still be wary of putting web-specific concerns into the model (anything related to browser sessions, cookies, hit tracking, etc), primarily since those concerns are presentation-centric and not business-centric, and secondarily because that makes it harder to port the system piecemeal to another language/platform for whatever reason later.

gWiz