Bad CakePHP Habits & How to Rectify Them!
SUNDAY OCTOBER 27, 08:23
I've been thinking a lot recently about how I can best help the CakePHP community - to try to give back some of the love which I have received over the years.
I have been fortunate to use CakePHP for over 5 years now, and in that time have gotten to know some key members of the CakePHP community.
So, what I wanted to go over was a few bad practices which I have seen over the years and how the correct procedure should be implemented. This isn't to say my code is perfect, but as a programmer we are always learning, so it is important to follow the best practices and adjust to them as you learn them!
CAKEPHP CONVENTIONS
There are actually CakePHP coding conventions which should be followed. These can be viewed here. I will highlight a few things which I often notice when viewing other programmers code.
1) Control Structures. So often you see programmers get this wrong, and in even some cases bring practices for other coding languages. CakePHP expects the following syntax:
if ((expr_1) || (expr_2)) {
// action_1;
} elseif (!(expr_3) && (expr_4)) {
// action_2;
} else {
// default_action;
}
In the control structures there should be 1 (one) space before the first parenthesis and 1 (one) space between the last parenthesis and the opening bracket. So this means that the following is incorrect:
if($this->request->data){
}
Note the spacing between the if and ( and the ) and {
Always use curly brackets in control structures, even if they are not needed. They increase the readability of the code, and they give you fewer logical errors.
So for example the following is incorrect:
if ($foo)
$bar = true
This should be formatted like this
if ($foo) {
$bar = true
}
Finally, watch where the brackets go. Brackets shouldn't go on new lines, and make sure all your brackets line up. Make sure the code is readable and indented correctly, so that each new bracket is inline with the closing bracket.
So, here are some incorrect examples which I see often:
if ($foo)
{
$bar = true;
}
This is incorrect, the opening bracket should be on the first line:
if ($foo) {
$bar = true;
if ($action) {
$to = false;
}
}
The indentation needs to line up correctly.
I often hear programmers say "but I am too busy to make the code neat...." My response is - "trust me, neat code will stand the test of time". Writing code which isn't readable will be a nightmare to come back to if you need to make a change in a few months.
CONTAINABLE & RECURSIVE
I was fortunate recently to have an informal discussion with a database developer from Facebook. We started talking about CakePHP and he said to me, "oh, that uses ORM doesn't it, that can be scary." I asked him what he meant, and he commented that with Object-relational mapping (ORM) it is easy for SQL queries to become unnecessarily large.
And he is right in a way. Part of CakePHP's magic is in it's use of ORM and the way that it groups different database table relations together. By default, CakePHP automatically selects any related 'Belongs To', 'Has One' and 'Has Many' data, and this can lead to very large SQL queries. These queries may not be a concern when you are initially developing an application, but after 6 months of collecting live data, and you may find the application becomes very slow, and in some cases crashes if the queries aren't optimised.
I look out for two things when auditing an existing website. Firstly, has the default recursive level been changed. By default, CakePHP sets the recursive level to 1, which in my opinion is too high. I always set it to -1 and then use the containable behaviour to get any related models.
And that leads nicely to the second thing I look for - has the Containable Behaviour been used. I often have new clients come to me who say that CakePHP is slow, and the reason is almost always because Containable hasn't been used! A good programmer will optimise their SQL queries regardless of how much "auto-magic" is done behind the scenes.
The containable behavior wasn't added until CakePHP 1.2, but boy has it made a difference. Be sure to use containable as much as possible, as it is such an effective way to optimise your SQL. For more information on how to implement and use the Containable behavior, click here.
FAT MODELS, SKINNY CONTROLLERS
Good CakePHP code will have the logic in the model files. This takes a bit to get used to, but once mastered there is no looking back! A controller file should be used for what it is intended for in the MVC pattern - controlling! So use your controller file to handle user actions, while let the code logic go in the model file.
A good example of this would be a simple CRUD - an everyday action! Lets take the add posts function from the blog tutorial as an example. The default add function is as follows:
public function add() {
if ($this->request->is('post')) {
$this->Post->create();
if ($this->Post->save($this->request->data)) {
$this->Session->setFlash(__('Your post has been saved.'));
return $this->redirect(array('action' => 'index'));
}
$this->Session->setFlash(__('Unable to add your post.'));
}
}
This controller action is fine for a simple add, but what would happen if you wanted to do things such as send an email to the admin when a post was added, or update another model association when a post was added. This is additional logic, but this logic shouldn't go into our controller file.
Instead we would write a function for this in our Post.php model, perhaps something like this:
public function addPost($data = array(), $emailAdmin = true) {
$this->create();
$this->save($data);
// update any other tables
// send the email to the admin user
if ($emailAdmin) {
}
// if all is successful
return true;
}
This would then result in a small change to the controller action as follows:
public function add() {
if ($this->request->is('post')) {
if ($this->Post->addPost($this->request->data)) {
$this->Session->setFlash(__('Your post has been saved.'));
return $this->redirect(array('action' => 'index'));
}
$this->Session->setFlash(__('Unable to add your post.'));
}
}
As you can see, the new action is actually one less line, because the $this->Post->create() has been moved to the model file.
This is a perfect, everyday example of where moving logic to the model file is a good idea - and it certainly makes for a much cleaner code base!
RETURN OFTEN, RETURN EARLY
This is always a bit of an ongoing debate, but returning often, and returning early certainly does make for much cleaner looking code. This applies to the model methods more than anything else.
But what exactly do I mean? Well, lets take a look at the method we added above:
public function addPost($data = array(), $emailAdmin = true) {
$this->create();
$this->save($data);
// update any other tables
// send the email to the admin user
if ($emailAdmin) {
}
// if all is successful
return true;
}
To return often, and return early means that as we run through our function, we check to make sure the everything is OK on a regular basis. If it isn't, then we return false, or return an error.
It might be easiest to show this with an example! There are two ways the above function could be written:
public function addPost($data = array(), $emailAdmin = true) {
if ($data) {
$this->create();
$result = $this->save($data);
if ($result) {
// update any other tables
// send the email to the admin user
if ($emailAdmin) {
// send the admin email
}
} else {
// problem saving the data
return false;
}
// if all is successful
return true;
} else {
// no data submitted
return false;
}
}
See how the code quickly becomes unreadable? There are if and else's all over the place, and the function quickly becomes one big indentation. Don't get me wrong, I love clean indentation, but watch how the function looks if it written with the return often, return early principle.
public function addPost($data = array(), $emailAdmin = true) {
if (!$data) {
// no data submitted
return false;
}
$this->create();
$result = $this->save($data);
if (!$result) {
// problem saving the data
return false;
}
// update any other tables
// send the email to the admin user
if ($emailAdmin) {
// send the admin email
}
// if all is successful
return true;
}
Straight away, in this small example, you will now see the code only has a single indentation and is much more readable. The logic actually makes more sense - let the logic run through line by line, and if there is any issue along the way, return the error and don't proceed to the next line.
This allows a programmer to write the same way that we read - reading code from left to right, top to bottom, rather than in different blocks, which can quickly get confusing!
DRY
DRY stands for Don't Repeat Yourself, and it is a philosophy which should be followed when coding in CakePHP. With object-oriented code, there is no excuse for repeating the same block of code twice!
Here is a few tips to ensure you don't repeat yourself:
1) As mentioned above, aim to put logic in model files so you can share the logic.
2) In your view files, if you are repeating views, either create the view code as an Element, or even a custom helper.
3) Set up some configuration settings - the app/Config/bootstrap.php file is a great place for this. So that you aren't hard coding things like the application name and the main email address. The last thing you want to do is go through hundreds of files just because the client has asked to update an email address in an application.
4) Always ask yourself - if I am repeating code, is there a better way to write this code, and am I putting this code in the right place! Chances are, if you need to repeat code, it could be written better.
DOC BLOCKING & INLINE COMMENTS
The last point I will make is in regards to comments. Firstly, doc blocking. A Doc Block is when you document a method or an action. It takes only a minute to record a little about what a function is doing, but it makes such a difference in terms of readability of code.
With CakePHP Doc Blocks, they need to go against the left margin of the page. So a simple example if to use the code from above.
/**
* Adds & saves a post as well as emails the admin to let them know the post has been added.
* Also performs some saving to another table
*
* @param array $data The post data
* @param bool $emailAdmin If set to true, will email the website admin
* @return bool Returns true if successful
*/
public function addPost($data = array(), $emailAdmin = true) {
As you will see, it doesn't take long to write a doc block, but it makes a huge difference in terms of longevity of the code. Ultimately, it means the code can live on past you as the developer.
Likewise with in-line comments. Don't be scared to explain what your code is doing and why! It makes it a lot easier in the long run to understand your code, especially if another developer is looking at it!
Comments
Post a Comment