Improving conditions clauses- few small things with hudge impact

It’s fascinating how very small changes can improve code readability. Below, you can find some examples often found in the code of inexperienced programmers.

Conditional operators:

Lets say that we have a condition that looks like in this example:

 public function countWorkingHours($day, ... )
    {
        // ... some things in your code ...

        if ($day > 5 && $day 

What is wrong with this code? The intention behind the “if” condition is unclear. There’s no explanation as to why we’re comparing specific numbers, nor what those numbers represent.

Let’s see how we can improve its readability by applying the “Don’t Make Me Think” principle:

public function countWorkingHours($day, ... )
    {
        // ... some things in your code ...

        if ($this->isWeekend($day)) {
            // ... do something
        } else {
            // ... do something else
        }
    }

    private function isWeekend($day) 
    {
        $friday = 5;
        $sunday = 7;

        return $day > $friday && $day 

Yes, it takes more lines of code now, but it’s clear what the “$this->isWeekend($day)” condition is checking.

This is a simple example, but often the defined condition rules are more complex, and this type of solution makes their intention clear for the programmer.

Indentations:

“… if you need more than 3 levels of indentation, you’re screwed anyway, and should fix your program.” – Linus Torvalds, Linux kernel coding style.

I really like that rule. It’s one of the code smells that’s easy to spot, indicating that something needs to be improved in the code. So, what does it mean? Let’s look at an example:

function someFunction($some_array)
{
    foreach ($some_array as $row) {
        // ... do something

        if ($this->someCondition()) {

            if ($this->anotherCondition()) {
            // ... do something 

            continue;
            }

            // ... do something
        }
    }
}

What is wrong? You have too many nested code blocks here. These should be extracted to separate functions or even objects, depending on the logic.

Let’s see how it can be improved for better readability:

function someFunction($some_array)
{
    foreach ($some_array as $row) {
        // ... do something
        $this->proceedRow($row);
    }
}

private function proceedRow($row)
{
    if ($this->someCondition()) {

        if ($this->anotherCondition()) {
        // ... do something 

        return;
        }

        // ... do something
    }

}

Guard Clauses:

Many times I see code that look like:

function calculatePrice(User $user, Product $product, $discount = 0.0) : float
{
    $price = 0;

    if ($discount == 0) {
        $price = $item->getPrice();
    } else if ($user->isPremium()) {
        $price = calculateDiscountedPremium($item->getPrice(), $discount);
    } else {
        $price = calculateDiscountedDefault($item->getPrice(), $discount);
    }

    return $price;
}

It’s not so bad, and after a while, it became quite clear what it’s responsible for. But can it be improved? Yes, and there are several ways to do so. The first step is to use early returns and remove the “else” block. After that, the code will look like this:

function calculatePrice(User $user, Product $product, $discount = 0.0) : float
{
    if ($discount == 0) {
        return $item->getPrice();
    } else if ($user->isPremium()) {
        return calculateDiscountedPremium($item->getPrice(), $discount);
    } 

    return calculateDiscountedDefault($item->getPrice(), $discount);
}

So a little bit less lines of code. Now lets change the if-else if statements into guard:

function calculatePrice(User $user, Product $product, $discount = 0.0) : float
{
    if ($discount == 0) {
        return $item->getPrice();
    } 

    if ($user->isPremium()) {
        return calculateDiscountedPremium($item->getPrice(), $discount);
    } 

    return calculateDiscountedDefault($item->getPrice(), $discount);
}

Very small change, but great impact – don’t you think?