Your code sucks, lets fix it

 Aug 1, 2013

Original

First off, I want to make sure that the original talks gets its due. You can watch it with slides on Pro Talk or via YouTube right here (I’ve embedded it below)

Guidelines

  1. Only one indentation level per method

Refactor methods to reduce indentation to 1 level. Where indentation is deeper than the level for the current method, consider why and what needs to be refactored into other methods. This can highlight where a method is trying to do too much

  1. Do not use ‘else’ keyword

Using the ‘else’ keyword, especially in nested ‘if’ statements can increase the complexity of the code exponentially. Increasing the number of paths through the code also increases the mental workload of understanding the code. Instead, build the ‘happy path’ through the method, with exit points (returns) for the exceptions to the happy path.

  1. Wrap primitive types and strings

Improve readability by wrapping primitive types and strings that also deal with behaviours into classes. You can then inject these objects into methods to make more sense. An example from the video is

$component->repaint( new Animate(false) );

// instead of

$component->repaint(false);

In the second line of code above, how are you expected to know what is false

  1. Only use one -> per line

When chaining methods or objects together, look at what you are doing. It may be a code smell that the current piece of code needs to know too much about other classes (Law of Demeter). If you are chaining though, improve readability by placing each link in the chain on a new line, and align the -> operators

$component->getUI()
->repaint();
  1. Do not abbreviate

This one was pretty simple. Don’t use abbreviations for method/variable names. Sometimes we want to put a lot of information into our names, but abbreviating is never going to help.

Example from talk:

$tr->method();  //$tr relates to $translationService
  1. Keep your classes small (200 lines per class, 10-20 lines per method, 10 methods per class, 15 classes per namespace/folder)

Again, this one is more about maintaining our code. As applications grow, opening a directory with 55+ files, with 1000’s of lines of code each can send a shudder down anyones spine. Keeping our classes, methods and namespaces smaller can help both in understanding the code, fixing or changing it and in reusing smaller chunks in other projects

While I would love to implement this, I almost feel its one of the hardest to do in PHP. Some of the core classes in CakePHP reach into the 1000’s of lines of code and to be honest, many of mine do as well.

  1. Limit the number of instance methods in a class to max:5

This is a question of the dependencies in the class. How many properties are assigned and why? As with number 6 I think this could be quite difficult to stick to, but then Rafael’s description of instance variables is slightly different to my understanding.

  1. Use first class collections (if class is a collection that’s all it should be)

Sticking to the single responsibility prinicipal again, if classes are built to handle collections then that is all they should be doing. External behaviours need to be handled by external classes

  1. Use accessors * (in PHP, Other languages have better solutions so this becomes Don’t use accessors)

Use accessor methods in PHP classes to add behaviour to the setting and getting of instance variable. As mentioned in the video there is an RFC inplace for changing PHP’s handling of accessors to implement the behaviour as part of the interface definition (You can read more here)

  1. Document your code

This last rule is a no-brainer really. We should all be documenting our code (as in comments not external documentation), but the trick here is to make it count.

Don’t do this:

// if the user is set do something
if (isset($user)){
//snip
}

There is no need to comment every obvious step in the code. However, if something is complex and needs explanation it should get it.

The last point here is Don’t comment bad code. Fix it

Well that’s just my take aways from this excellent talk, mainly written down as an aid to my terrible memory. Please, please, please watch the video as my comments just don’t do it justice!