melp.nl

I have gathered some of the less obvious do's and don'ts in how to make your code more explanatory to your coworkers. I left out the more obvious like writing doc-comments, using coding standards, etc. Those go without saying. This might be considered "drm's php tiplist revisited" ;)

#5 Use one exit point in a function. Except for exceptions.

This eases debugging, and makes your code more readable. Since the flow of the function will always end in one point (and not break flow somewhere in the middle) it is easier to understand what it does. Use a standardized value name to store the return value for the function, like 'ret', which you will not use in any other circumstance. Always initialize this value at the top of the function with a default value, even if the default value will never be used, so it is visible that your function will be busy determining what that value should be.

#!php
function doSomething($param1, $param2) {
    $ret = null;
    if($param1 == doSomethingElse($param2)) {
        $ret = $param1;
    } else {
        $ret = $param2;
    }
    return $ret;
}

If you have a function or method that might enter an unknown state, or unexpected state throw exceptions, that's what they're for. Check your input and be aware that even if your function is called only internally, you never know what people will get to work with the code and you therefore never know what they might think is appropriate to feed it.

Don't:

#!php
function checkStuff($foo) {
    switch($foo) {
        case 'a': return 1;
        case 'b': return 5;
    }
}

Do:

#!php
function checkStuff($foo) {
    $ret = null;
    switch($foo) {
        case 'a':
            $ret = 1;
        break;
        case 'b':
            $ret =2;
        break;
        default:
            throw new InvalidArgumentException('Expected a or b for $foo');
    }
    return $ret;
}

The exceptions I use most are InvalidArgumentException (for arguments I didn't expect), UnexpectedValueException (for return values of other functions I didn't expect) and LogicException (for a combination of circumstances that don't make sense). Whenever possible and suitable, I extend these to provide more information about the context of the exception.

#4 Use 0, null and false consistently

• If you use 0, you mean a countable or arithmetic result 0, which is a numeric representation of "nothing".
• If you use false, you mean a boolean state of the variable or property, which means that the only other state the property or variable can be in, is true. Unfortunately, some of php's standard library functions don't follow this (e.g. strpos()), but don't let that be a guideline ...
• If you use null, you mean either an unknown state of the variable, or the variable has not been initialized (yet), or the state of the variable is to be considered invalid. To check if a variable is null, use is_null(), since an initialized state of the variable can be a value that evaluates to false in a boolean expression. This leads to the following paradigm: is_null() checks if a variable has a sensible value (yet).

For example:

#!php
function getCount() {
    if(is_null($this->_count)) { 
        $this->_count = count($this->_property);
    }
    return $this->_count;
}

#3 Design by contract, design for extension

Try to extract interfaces for all your classes. Even if you don't explicitly use these interfaces, e.g. in type hints, next to being a good exercise it still serves the purpose of showing what parts of a class are specific to the classes (own) implementation and what parts aren't. In other words: it shows the reader what functionality of a class is actually there because it follows a contract defined by an interface, and what code is merely aiding the implementation; hence showing how the same interface could be used for other implementations.

Designing for extension means that you show what parts are actually meant to be extended. This does not mean that you should declare everything protected or public. It means that you should think hard about what entry points a derived class should use, and where it is supposed to hook into base functionality. Think about what method should be final and what properties could possibly be private, so you're not ambiguous about where you'd want a derived class to hook in:

Don't:

#!php
// We have 3 extension points
class Foo {
    // #1 (could be set by a derived constructor)
    protected $_thingy = null; 

    // #2 (could be overridden by a derived class)
    function getThingy() {
        if(is_null($this->_thingy)) {
            $this->_thingy = $this->_loadThingy();
        }
        return $this->_thingy;
    }

    // #3 (could be overriden by a derived class)
    protected function _loadThingy() {
        return "some value that relies on some heavy resources";
    }
}

Do:

#!php
// now we have only one. It is obvious the we 
// want _loadThingy to be the extension point
class Foo {
    private $_thingy = null;

    final function getThingy () {
        if(is_null($this->_thingy)) {
            $this->_thingy = $this->_loadThingy();
        }
        return $this->_thingy;
    }

    protected function _loadThingy() {
        return "some value that relies on some heavy resources";
    }
}

It goes without saying that if you don't intend your class to be extended at all, you should declare it final too.

Last but not least: always define a base constructor, even if it does nothing. This way, a derived class has no reason not to call the base constructor (which imho a derived class always should), so you won't break compatibility if you add initialization code to the base constructor later.

#2 Make your symbols explain themselves

I have the following few rules of thumb I use for naming methods and variables which I (try to) use consistently throughout my code.

• Be consistent in your naming throughout your application. Don't use a "name" field in one table, and a "title" in another with exactly the same semantics. Avoid mixing your native tongue with English unless there is absolutely no way around it.
• "Long" names are in the eye of the beholder. Get a bigger screen, get rid of that hopelessly outdated 80-character wordwrap in your IDE and consider that every time you or your colleague needs to wonder what something means, it is a precious human CPU cycle lost.
• A method that starts with get never changes the object state, except for lazy-load, in which case the changed object property is accessed only by that method. It always returns the property that the method name is implying: getTitle() returns a title.
• A method that starts with load does some heavy loading, and it never caches the value. The caching is done by the get() method (or the service which it is calling, for that matter), which lazy-loads the property. In other words, a loader method is typically private, since the getter that stands right next to it is the only one to call it.
• A method that starts with init does local initialization, and does not call any external services or heavy-duty work. It is a piece of initialiation that is either there for lazy-load reasons, or because it is a Template Method for hooking into another method (typically a parent's constructor).
• Variables and methods that have boolean values or return values always start with is or has. This way the names explain what the value stand for, and that they represent a true boolean value: either yes or no and nothing in between.
• Use your own exception implementation, with which you provide additional context information on why and how the exception occurred. Since you use your own definitions, you can easily catch and handle the exceptions at a higher level.

#1 Only comment on code that needs clarification

Following #2, it does not make sense to comment on stuff that the code itself already explains. If you have code with clear symbol names, commenting is simply redundant most of the times. On the other hand, if you feel a piece of code needs more explanation, check to see if it could be solved with better naming of your symbols. In many cases, the code could easily transformed in a more readable equivalent, just by renaming some variables.

Don't

#!php
// Check if $c is the last one
if ($c == $l) {
    // print the table footer
    echo $tfoot;
}

Do

#!php
if($current == $last) {
    echo $htmlTableFooter;
}

And most of all, don't repeat your code in your comments, as I saw some time back: #!php // get page $page = getPage();

// assign page to template
$smarty->assign('page', $page);

Right. :P

If you still feel your code needs commenting, comment on a block of code, not on a per line basis. Each line should clarify itself by the way it is written, so you just need to explain the overall purpose of a piece of code. In many cases, though, the code should be in a separate function already, and you would comment on the inner workings of the function by using doccomments, not inline comments.

However, if an algorithm is complex enough to be commented on, try to comment in such a way that the comment is readable without the code. In other words, the comment is actually telling a story on "what is happening", so you needn't read the code to know what it does. This works best if all the code is commented on, not just some parts that aren't clear to the bare eye.

Read the original post & comments at drm.tweakblogs.net.