10

I'm using the MVC pattern in my web application built with PHP.

I'm always struggling to determine whether I need a new dedicated controller for a set of actions or if I should place them inside an already existing controller.

Are there any good rules of thumb to follow when creating controllers?

For example I can have:

AuthenticationController with actions:

  • index() to display login form.
  • submit() to handle form submission.
  • logout(), self-explanatory.

OR

LoginController with actions:

  • index() to display login form.
  • submit() to handle form submission.

LogoutController with action:

  • index() to handle logging out.

OR

AccountController with actions:

  • loginGet() to display login form.
  • loginPost() to handle login form submission.
  • logoutGet() to handle logging out.
  • registerGet() to display registration form.
  • registerPost() to handle form submission.

    And any other actions that are are involved with an account.

  • Maybe have a look at RESTful design. It doesn't solve every single problem of this kind, but gives you a very good direction how to think about it. – thorsten müller Sep 7 '14 at 19:31
3
+25

In order to find the right grouping for controllers, think of the testing.

(Even if you don't actually do any testing, thinking of how you would go about testing your controllers will give you some very good insights on how to structure them.)

An AuthenticationController is not testable by itself, because it only contains functionality for logging in and logging out, but your testing code will need to somehow create fake accounts for testing purposes before it can test a successful login. You could bypass the subsystem-under-test and go directly to your model for the creation of the testing accounts, but then you will have a fragile test in your hands: if the model changes, you will have to modify not only code which tests the model, but also also code which tests the controller, even though the interface and behavior of the controller has remained unchanged. That's unreasonable.

A LoginController is unsuitable for the same reasons: you cannot test it without creating accounts first, and there are even more things that you cannot test, like for example preventing duplicate logins but then allowing a user to login after having logged out. (Since this controller has no logout functionality.)

An AccountController will give you everything you need in order to do your testing: you can create a test account and then try to login, you can delete the account and then make sure you cannot login anymore, you can change the password and make sure that the right password has to be used in order to login, etc.

To conclude: in order to write even the smallest test suite, you will need to make all of the functionality of the AccountController available to it. Subdividing it down to smaller controllers appears to be yielding handicapped controllers with insufficient functionality for a proper test. This is a very good indication that the functionality of AccountController is the smallest subdivision that makes sense.

And generally speaking, the "think of the testing" approach will work not only in this particular scenario, but in any similar scenario you come across in the future.

1

Answer is not that obvious

Please let me clarify a few things before i will make any answering statements. First of all:

What is the controller?

Controller is a part of system that controls request - after dispatching. Thus, we can define it as some set of actions related to... what?

What is the scope of controller?

And thats more or less part when we will have any answer. What do you think? Is it a controller of things (for example an Account) or controller of actions? Of course its a controller of some model or some more abstract thing that provides actions on it.

The answer is...

AuthenticationController with actions:

  • index() to display login form.
  • submit() to handle form submission.
  • logout(), self-explanatory.

Nah, authentication is a process. Don't go that way.

LoginController with actions:

  • index() to display login form.
  • submit() to handle form submission.

Same here. Login - action. Better don't create action controller (you don't have correlated model to it).

AccountController with actions:

  • loginGet() to display login form.
  • loginPost() to handle login form submission.
  • logoutGet() to handle logging out.
  • registerGet() to display registration form.
  • registerPost() to handle form submission.

Quite good, but i am not convinced that building that low-level controller (controller is abstraction itself) is worth bringing. Anyway, creating methods with *Get or *Post are unclear.

Any suggestion?

Yes, consider it:

AccountController:

  • login(AccountModel)
  • logout(AccountModel)
  • register(AccountModel)
  • index()

And related model to it, ofc Account class. It will give you an opportunity to move your model-controller pair somewhere else (if it will be neccessary) and make a clear code (it's obvious what the login() method mean). Stincking to model is really famous especially with CRUD applications and maybe it is a way for you.

1

Controllers are usually created for a certain resource (an entity class, a table in the database), but can also be created to group together actions that are responsible with a certain part of the application. In your examples, that would be a a controller that handles the security for the application:

class SecurityController
{
    // can handle both the login page display and
    // the login page submission
    login(); 

    logout();

    register();

    // optional: confirm account after registration
    confirm();

    // displays the forgot password page
    forgotPassword();

    // displays the reset password page
    // and handle the form submission
    resetPassword();
}

Note: don't put the security related actions and the user profile actions in the same controller; it might make sense because they are related to the user, but one should handle authentication and the other should handle email, name etc. updates.

With controllers created for resources(let's say Task), you would have the usual CRUD actions:

class TasksController
{
    // usually displays a paginated list of tasks
    index();

    // displays a certain task, based on an identifier
    show(id);

    // displays page with form and
    // handles form submission for creating
    // new tasks
    create();

    // same as create(), but for changing records
    update(id);     

    // displays confirmation message
    // and handles submissions in case of confirmation
    delete()
}

Of course, you have the possibility to add related resources to the same controller. Say for example you have the entity Business, and each one has several BusinessService entities. A controller for it might look like this:

class BusinessController
{
    index();

    show(id);

    create();

    update(id);

    delete();

    // display the business services for a certain business
    listBusinessServices(businessId);

    // displays a certain business service
    showBusinessService(id);

    // create a new business service for a certain business
    createBusinessService(businessId);

    // updates a certain business service
    updateBusinessService(id);

    // deletes a certain business service
    deleteBusinessService(id);
}

This approach makes sense when the related children entities cannot exists without the parent entity.

These are my recommendations:

  • create controllers based on a group of related operations (handling certain responsibilities like security, or CRUD operations on resources etc.);
  • for resource-based controllers, don't add unnecessary actions (if you are not supposed to update the resource, don't add the update action);
  • you can add "custom" actions to simplify things (e.g. you have a Subscription entity that has an availability based on a limited number of entries, you can add a new action to the controller named use() that has the single purpose of subtracting one entry from the Subscription)
  • keep things simple - don't clutter your controller with a huge number of actions and complex logic, try to simplify things by decreasing the number of actions or making two controller;
  • if you are using an MVC focused framework, follow their best-practice guidelines (if they have it).

Some resources for further reading here.

0

I see two antagonistic design "forces" (which are not exclusive to controllers):

  • cohesiveness - controllers should group related actions
  • simplicity - controllers should be as small as possible to manage their complexity

From cohesiveness point of view, all three actions (login, logout, registration) are related, but login & logout much more than registration. They are related semantically (one is an inversion of the other) and quite possibly will also use same service objects (their implementations are also cohesive).

My first instict would be to group login and logout into one controller. But if login & logout controller implementations are not so simple (e.g. login has captcha, more authentication methods etc.), I would have no problem to divide them into LoginController and LogoutController to maintain simplicity. Where this threshold of complexity (when you should start splitting the controller) lies is a bit personal.

Also remember, that whatever you design your code initially, you can (and should) refactor it as it changes. In this case it is quite typical to start with simple design (have one AuthenticationController) and with time you'll receive more requirements which will complicate the code. Once it crosses the complexity threshold, you should refactor it to two controllers.

BTW, your code suggests that you're logging out user with GET request. That's a bad idea since HTTP GET should be nullipotent (it shouldn't modify the state of the application).

0

Here are a few rules of thumb:

  • Organise by subject or topic, with the controller name being the topic's name.

  • Remember that the controller's name will appear in the URL, visible to your users, so preferably it should make sense to them.

In the situation you mention (authentication) the MVC team has already written the controller for you. Open Visual Studio 2013 and then click

File / New / Project... 
Search installed templates for "ASP.NET MVC4 Web Application"
Choose "Internet Application" / OK.

AccountController.cs contains all methods for managing user accounts:

Login()
Logoff()
Register()
Disassociate()
Manage()
ExternalLogin()

So they have organised by topic "User accounts and authentication", with visible topic name "Account".

0

Terminology

I believe it is a big misconception to call a class containing some HTTP-related methods a "controller".

Controller is a method that handles request, but not a class containing such methods. So, index(), submit(), logout() are controllers.

Class containing that kind of methods is named "controller" just because it constitutes a group of controllers, and is playing a role of "bottom-level" namespace. In FP language (like Haskell) it would be just a module. It is good practice to keep those "controller" classes as stateless as possible in OOP languages, except references to services and other program-wide stuff.

The answer

With terminology sorted out, the question is "how should we separate controllers into namespaces/modules?" I think the answer is: controllers inside a single namespace/module should deal with the same kind of data. For example, UserController deals primarily with instances of User class, but occasionally touches other related things, if required.

Since login, logout and other such actions are mostly dealing with the session, it probably best to put them inside SessionController, and index controller, which just prints a form, should be placed into LoginPageController, since it obviously deals with login page. It makes a little sense to put HTML rendering and session management into a single class, and that would violate SRP and probably a bunch of other good practices.

General principle

When you're having trouble deciding where to put a piece of code, start with data (and types) you deal with.

  • 2
    Sorry, those are Actions, not Controllers :) – JK01 Apr 27 '15 at 22:33
  • @JK01 Those are what you call them. It's terminology, you know. And there are frameworks which call those functions "controllers" (or "handlers"), since there are many frameworks which doesn't organize those into classes, since namespaces/modules are enough already. You can use any terms you like, it's just words, but I think having less terms is better. – scriptin Apr 27 '15 at 23:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.