Most software developers have probably heard of the design principle “Fat Models, Skinny Controllers” but I constantly come across code that doesn’t follow this principle. How skinny is skinny? Properly designed controllers should not have more than a few lines of code. For most actions, if you have more than four lines in the method, that should be an indication that there is a better way to do something.
Let’s take a look at some code working with a delete() action.
Many URLs follow a RESTful setup. In this example, I’m assuming a URL structure like /model/$id/action.
For example, using Yii Framework, this would be my delete action for a VideoController:
public function actionDelete($id)
{
$success = $this->model->delete();
$this->sendAjaxResult($success, 'Deleted video', 'The video could not be deleted');
}
This is just two lines of code. Most standard actions need to follow the format of:
- load the appropriate object
- perform the action on the object
- send data back to the view with the result
In this case, the “view” is an AJAX response.
In this example, there is no code indicating that the object is being loaded. So how is that being accomplished?
Many PHP frameworks have an init() method available for controllers where you can specify code that should be run before the action. From the name, init you can tell that this should mainly be used for initializing variables.
In my Controller class I have the following:
class Controller extends CController
{
/**
* @var ActiveRecord
*/
protected $model;
public function init()
{
$this->model = $this->loadModel();
parent::init();
}
There is a variable $model which stores the current object to be acted upon. This is loaded through the loadModel() method which takes the $id, figures out the name of the model with a little bit of magic (ie. for ‘VideoController’ the model would be assumed to be ‘Video’) and stores it in the $model attribute. Since $model is protected, it can be accessed by child classes.
What if there is no ID? For example, the index page of /video may just contain a list of videos. Ah you got me. It’s good to see you’re thinking ahead.
My actual init() method looks like this:
public function init()
{
if (Yii::app()->getRequest()->getParam('id')) {
$this->model = $this->loadModel();
}
parent::init();
}
So loadModel() will only be called if the ID parameter is available in the URL. In the index action scenario, $model would be null and could not be acted upon.
Going back to the controller action, the delete() method on the object performs all the work. I won’t include the code here but you should push back all code into the object’s class (ActiveRecord in Yii, Zend_Db_Table_row in Zend Framework, etc.) so that you just need to call one method from the controller action.
The second line, $this->sendAjaxResult() performs some processing which sends back the response via AJAX. For standard rendering, many PHP frameworks provide a render() method for the controller to render a view and set data. This is a common scenario that the last line would be calling render() if it is not done automatically after the action.
What about authorization?
Good question. You’re still following along. Authorization needs to be performed before you even get to the action. If a user does not have access to the action, the processing should never even reach the action code. If you are putting authorization logic in the action code, that is another indicator that things need to be cleaned up.
In Yii Framework, for example, you can override the filters() and accessRules() methods in your controller. In my VideoController class, I have the following:
public function filters()
{
return array(
'ajaxOnly + delete',
'deleteOnly + delete',
'accessControl',
);
}
public function accessRules()
{
return array(
array('allow', 'actions' => array('delete'), 'expression' => array($this->model, 'isModerator')),
array('deny')
);
}
NOTE: This is just an example for Yii Framework. Other frameworks would handle authorization differently.
In this example, the filters() method is run first. Each value of the array will be checked to make sure that this action matches the filters. So the delete action must be called via AJAX and must be a DELETE request, as opposed to a GET or POST request. If those filters pass, then accessControl is called which makes use of the accessRules() method.
This method also returns an array which is pretty easy to follow along. Each value of the array is checked for the first match that returns true. In this case, the first value indicates that the user is allow ed to perform the delete action if the expression $this->model->isModerator() returns true. I won’t provide the code for that method, but as you can guess it checks to make sure that the current user is a moderator for the video and thereby has the ability to delete it.
If that access rule is false, then Yii will check the next one which is simply deny. For authorization, it is a good idea to deny access to anything you haven’t explicitly given access to. This is a good rule of thumb for security - deny everything and then slowly open up access one by one.
The other benefit of keeping authorization out of the action code, other than keeping skinny controllers, is that authorization and actions require two different trains of thought. When dealing with authorization, you should be thinking “Who needs access to this action and what special criteria do they need?” When writing the action code you should only be thinking about what action to perform and not be worrying about security. Separating authorization from the action is a good way to keep your mind focusing while programming and it should help reduce bugs.
So, in a nutshell, this is one example of how you can go about creating skinny controllers.
Let’s review the rules of thumb:
- actions should contain no more than a few lines of code
- push back logic into the model/ActiveRecord class
- initialize variables and attributes in an
init()method before the action - authorization logic should be performed before the action
Follow these principles and you can help your controllers lose weight!