Code Style: If and return

When contributing to opensource code at GitHub, I found some mistakes, or improvements when writing code, that are repetead over and over again for newbies and not so newbies developers. I’ll try to write some posts about this, but for not, let’s start with some “if” and “return” statements cases.

Return #1: If + big code + small else

When you have and if-else, I’d prefer to have the shortest block first so I can see more information at once. Specially, the first flow related with the conditional cases.

Bad:

public function foo($user)
{
    if ($user->hasAvatar()) {
        // tons of amazing code that make you scroll to the infinity
        // ...
    } else {
        // typical check
        return false;
    }
}

Better:

public function foo($user)
{
    if (!$user->hasAvatar()) {
        return false;
    } else {
        // tons of code that you should refactor :)
        // btw, check the next one for this case
    }    
}

Return #2: If + return + useless else

If you return something in the first case, “else” is useless. Removing it reduces lines of code, cyclomatic complexity and so on.

Bad:

public function foo($user)
{
    if (!$user->hasAvatar()) {
        return false;
    } else {
        // do something...
    }
}

Better:

public function foo($user)
{
    if (!$user->hasAvatar()) {
        return false;
    }

    // do something...
}

Return #3: If + return true + else return false

I don’t know why, but this is typical. If you are returning booleans, please, return the conditional evaluation.

Bad:

public function foo($user)
{
    if ($user->hasAvatar()) {
        return true;
    } else {
        return false;
    }
}

Better:

public function foo($user)
{
    return $user->hasAvatar();
}

Return #4: If + return A + else return B

Exactly the same with other types.

Bad:

public function foo($user)
{
    if ($user->hasAvatar()) {
        return 'yeah!';
    } else {
        return 'woo!';
    }
}

Better:

public function foo($user)
{
    return $user->hasAvatar() ? 'yeah!' : 'woo!';
}

Return #5: If + else that holds a default value

One variation for case #2.

Bad:

public function foo($user)
{
    if ($user->hasAvatar()) {
        $result = 'yeah!';
    } else {
        $result = null;
    }

    // Do whatever with $result...
    return $result;
}

Better:

public function foo($user)
{
    $result = null;
    if ($user->hasAvatar()) {
        $result = 'yeah!';
    }

    // Do whatever with $result...
    return $result;
}

Feel free to comment for adding other examples that may help others. Please, make your mates live easier. Happy Clean Coding!

  • No estoy de acuerdo con el primer punto pues lo que tiene mas posibilidades de cumplir el if debería ir primero… No? Con los otros coincido totalmente sobretodo con el #5

    • Carlos Buenosvinos

      En el caso de código corto, prefiero ponerlo primero. Así veo, la firma de la función, la lógica del if y que hay else en una única pantalla sin scrollear. Pero tiene diferentes interpretaciones.

      • Eloi Poch

        Puestos a peditr y a ser puristas, yo no creo que debas tener ni el if ni el else “grandes”, realmente deberías tener una llamada a una función específica en cada bloque, de esta manera el else y el if tendrían la misma longitud y los podrías ordenar de manera “natural”.

        • Carlos Buenosvinos

          Totalmente de acuerdo. Estos son ejemplos sin tener en cuenta clean code, extract til you drop o similar.

  • Oh yeah, exacto, that’s the way.
    Me ha gustado mucho la entrada, y es algo que me toca mucho las pelotas que se haga mal demasiado a menudo ( aún me jode mas que a la gente le importe un pimiento )…

  • coincido en todo. Lo único que añadaría es que el #4 es que si la linea ´return … ‘ ocupa más que una linea probablemente sea más fácil de leer de otra manera.