Don't use "else"

September 11th, 2023

A popular approach to writing clean code is to avoid the "else" keyword and, if possible, avoid nesting if statements within other if statements. If I look at some code, I want to see as few indentation levels as possible, making the code easier to read and understand.

What should I do instead?

Instead, you check for a condition; if that isn't met, you return early. For example, here is some code I saw recently during a meetup talk:

$callingClass = $scope->getClassReflection()->getName();

if ($callingClass === TextMessageQueueProcessor::class) {
    return [];
}

$type = $scope->getType($node->var);

foreach ($type->getReferencedClasses() as $targetClass) {
    if ($targetClass === TextMessageSender::class) {
        return [
            RuleErrorBuilder::message(
                sprintf(
                    "Can not call %s from %s",
                    $targetClass,
                    $callingClass
                )
            )->build()
        ];
    }
}

return [];

There are no else statements.

If the calling class isn't of the required type, it returns immediately with no violations, and we continue, knowing the calling class must be what we need.

If the target class is one where the code shouldn't be called from, it returns immediately with the violation.

Finally, if no violations were found within the referenced classes, it returns an empty array.

Here's the thing

The code always returns an array of rule violations but does so as soon as possible at each point.

The code is clean and readable, and I can understand it, knowing once each condition is passed, I don't need to continue thinking about it.

Whilst there are some situations to use else, most of the time I've found that I can use an early return instead.

- Oliver