Keeping your CakePHP Controllers Clean

As my office gains more and more experience with CakePHP, we are beginning to learn to build more organized web sites.  Looking back at our first few projects, I’m astonished to see how messy our controllers are!

I know the controllers are the “brain” of MVC because it pieces our data to our views, but that doesn’t mean everything should go in there.  Our original controllers would contain all of our logic, all of our data manipulation, all of our custom queries, and some additional data validation.

We are now working very hard to keep our controllers as clean and simple as possible.  My goal is to make every function in our controllers under 20 lines of code.  It might sound ambitious, but I think it is completely feasable.

Before I begin explaining my thoughts on the situation, I would love to hear what others have to say about this.

Here are my keys to keeping our CakePHP controllers nice and clean and easily readable:

1. Put all data validation in the model
2. Put all custom queries in the model
3. Put any complex queries (more than a simple find(‘all’)) in the model
4. Create a component for all logic
5. Create a component for all data manipulation

By following the above 5 rules, it will make tracking down bugs and making simple alterations 10 times easier.  Imagine having to look through 200 lines of code to find where that error message is.  If it were in the model, it would be extremely easy to pinpoint it down and change it.

Have a problem with some logic, go right to your component and do not be convoluted by other code that is unrelated to the problem.

Below is a sample function that is under 20 lines of code and has all of the above 5 items segregated appropriately:

[code]function add() {
 // check if we are posting data
 if (!empty($this->data)) {
  // manipulate our data
  $user = $this->UserSetup->manipulateSomeData($this->data);
  
  // validate and save our data
  if ($this->User->save($user)) {
   $this->Session->setFlash(‘User successfully saved’);
   $this->redirect(‘index’);
   exit();
  } else {
   $this->Session->setFlash(‘There was an error saving your data’);
  }
  
  // get country list
  $this->set(‘countries’, $this->Country->find(‘all’));
  // get provinces based on countries
  $this->set(‘provinces’, $this->Province->getByCountryId(1));
 }
}
[/code]

Again, as mentioned early, I would love to hear your thoughts if you agree or disagree.  If you disagree, please let me know your approach.