Defensive programming PHP Best practices

This series of tips is dedicated to strengthening your code.

Always check for unexpected values

Beginner

Rule

Your switch statements should always have a default.

Explanation

Just imagine you have a function that does some processing on an article, based on its status. Its status can be "active", "pending" or "inactive"

What you should NOT do:
function doStuff($status) {
    switch ($status) {
        case "active":
            // Do some stuff
            break;
        case "pending":
            // Do some stuff
            break;
        case "inactive":
            // Do some stuff
            break;
    }
}

But in the future, what if a new status is added, like "archived"? How long will it take before you notice?

So your code should look like this:
function doStuff($status) {
    switch ($status) {
        case "active":
            // Do some stuff
            break;
        case "pending":
            // Do some stuff
            break;
        case "inactive":
            // Do some stuff
            break;
        default:
            // This is good! In case an unexpected status is sent, you will notice
            throw InvalidStatusException(sprintf("Unknown Status '%s'", $status));
    }
}

By adding a default statement that throws an exception, you are sure to notice if something goes wrong in the future.

Note: this rule also applies for long series of if, elseif statements without a final else.

Use constants

Beginner

Rule

Constants should be stored in classes using the const keyword.

Avoid using the define keyword for constants.

Explanation

What you should NOT do:
function doStuff($status) {
    switch ($status) {
        case "active":
            // Do some stuff
            break;
        case "pending":
            // Do some stuff
            break;
        case "inactive":
            // Do some stuff
            break;
        default:
            throw InvalidStatusException(sprintf("Unknown Status '%s'", $status));
    }
}
What you should do instead:
class StatusEnum
{
    const ACTIVE = "active";
    const PENDING = "pending";
    const INACTIVE = "inactive";
}
function doStuff($status) {
    switch ($status) {
        case StatusEnum::ACTIVE:
            // Do some stuff
            break;
        case StatusEnum::PENDING:
            // Do some stuff
            break;
        case StatusEnum::INACTIVE:
            // Do some stuff
            break;
        default:
            throw InvalidStatusException(sprintf("Unknown Status '%s'", $status));
    }
}

By storing the list of possible statuses in a class, the IDE will offer autocompletion on your enumeration class. Furthermore, you cannot misspell a status in your code. If a status is removed from the class, you will get an error if you try to access it (so your code will fail instead of silently ignoring the error, which is good).

You should use const instead of define because:

  • it allows grouping constants into classes: your code is more organized and self describing
  • classes can be autoloaded: you don't need to include a file containing all define statements for each request so your code is more efficient

Use an enumeration library

Pro

Rule

This is an improvement over the previous rules:

Consider using an enumeration library (like myclabs/php-enum) to manage your enumeration classes.

Explanation

Thanks to an enumeration library, you can type hint your enumerations. This is a useful type check that makes your code more robust and simpler:

use MyCLabs\Enum\Enum;

class StatusEnum extends Enum
{
    const ACTIVE = "active";
    const PENDING = "pending";
    const INACTIVE = "inactive";
}
// Please notice the type hinting on StatusEnum
function doStuff(StatusEnum $status) {
    if ($status == StatusEnum::ACTIVE()) {
        // Do stuff
    }
}

// Function call looks like this:
doStuff(StatusEnum::ACTIVE());

Using an enum library, rather than referring to the constant directly, you call a static function that has the same name as the constant. So instead of doing StatusEnum::ACTIVE, you write StatusEnum::ACTIVE(). The ACTIVE() static method will return an instance of the StatusEnum class that represents the constant. Therefore, you can type hint on it.

Your code is safer and simpler. Now, no one can call the doStuff method without passing a valid status, so you don't need to check that in your method!

The Yoda condition

Padawan

Rule

When doing logical comparisons, always put the variable on the right side, constants or literals on the left.

Explanation

What you should NOT do:
if ( $theForce == true ) {
    $victorious = youWill( $be );
}

Imagine that you omit an equal sign in the above code. The assignment would be perfectly valid, returning 1, causing the if statement to evaluate to true, and you could be chasing that bug for a while.

So your code should look like this:
if ( true == $theForce ) {
    $victorious = youWill( $be );
}

If you omit an equal sign you’ll get a parse error, because you can’t assign to a constant like true

Note: this rule also applies to ==, !=, ===, and !==. Yoda conditions for <, > , <= or >= are significantly more difficult to read and are best avoided.

Found a typo? Something is wrong in this documentation? Just fork and edit it!

image description