zen of coding

Fatten up your Models

CakePHP models, unlike super models should be fat. It’s a good practice to keep redundant code out of your controllers. Here’s a quick example on how you can keep your controllers skinny and models fat.

Let’s say you need to get a list of all public and active products…

[sourcecode language=’php’]
$productList = $this->Product->find(‘list’, array(
‘conditions’ => array(
‘Product.name ASC’);

So the above code will select all products where is_public = 1 (assuming that means the products are public) and status = 2 (let’s say 2 means active).

Now if you have a bunch of different actions that require you to get such a list you’d have to repeat this same code over and over. Sounds like a bad idea…

Instead you should move this code into your Product model and create a method there called getList (for example). Remember that in the model you won’t need $this->Product. Adjust the code to read $this->find, the rest remains the same.

Now in the controller all you have to do is:

[sourcecode language=’php’]

You could even make getList more interesting by allowing the user to pass parameters, such as values for is_public and status.

Not only does this help to keep your code cleaner, but if your rules change to say that you need to get all products with status 3 instead of 2 you only need to modify the code in your Product model, rather than in a lot of different actions.

  • Hi,
    Sometimes is good idea overwrite the find method in own models to get something like did you talk. There’re an example here: http://debuggable.com/posts/implementing-different-types-for-cake%27s-model::find()-method:485030de-4778-456e-8400-44d84834cda3

    Cheers and thanks for your posts

  • teknoid


    I’m not sure if extending find() in necessary for something so simple, but anyway the main focus of this post was just to illustrate how a logic should be moved from controller to model.

  • Once you have passed your dataset from getList(..) to the controller is it then possible to then setup pagination using this dataset – so that it can be passed to the view?

  • @Chris

    No, that’s now how pagination works. You paginate model records, not the returned data.

  • RSK

    @teknoid : can u spare little time for me to correct this coding?Since i am a beginner in programming i think i need to learn through the best coding practices

    $yearlyHolidaysDetail = $this->YearlyHoliday -> getHolidays($this->data[‘ActivityCalendar’][‘month’] ,$this->data[‘ActivityCalendar’][‘year’] );

    Part 2
    $yearlyHolidays = $holidayDetails = array();
    if(!empty ($yearlyHolidaysDetail)){
    foreach ($yearlyHolidaysDetail as $key => $yearlyHoliday) {
    $yearlyHolidays[] = $yearlyHoliday[‘YearlyHoliday’][‘holidayDate’];
    $holidayDetails[(int)date(‘d’, strtotime($yearlyHoliday[‘YearlyHoliday’][‘holidayDate’]))] = $yearlyHoliday[‘YearlyHoliday’][‘description’];

    Now i written the whole code in controller.
    Is this a good practice. Whether i need to move the second part also into model? The second part is only used in this current controller only.where as part one is used from others also.that why i kept this in the controller. Is this ok? any other corrections should i do?

  • teknoid


    Looks fine to me. Even if the second part is in the controller, it might be a good idea to move that into a separate private controller method, this way you keep your action cleaner and just call $holidayDetails = $this->getHolidayDetails();

    At later time if you find yourself needing this elsewhere, it is also easier just to copy/paste the code to the model or component. (Maybe not in this case, but the overall strategy of breaking things up remains the same in general).

  • RSK

    @teknoid : Thankz for the reply.And will it to private controller method :)

%d bloggers like this: