ramblings on PHP, SQL, the web, politics, ultimate frisbee and what else is on in my life
back 1  2  »  

Returning early

One CS (coding style) item I miss in most such guides is the point to always return as early as possible in your methods/functions. This translates to more readable code and less indenting. Let me give an example (btw this example is also a very bad example of meaningful variable names):


<?php
function checkSomething($a, $b)
{
  if ($a->doIexist()) {
    $a->doSomeStuff();
    $b->wowIamSoComplex();
    if ($b->stillGoing() || $b->maybeThisWorks()) {
      return true;
   }
  }
  return false;
}
?>

If I reorganize things to immediately "return false" if "$a->doIexist()" returns false, I can move the rest of the code down one indenting level. I also immediately see that "$a->doIexist()" better return true.


<?php
function checkSomething($a, $b)
{
  if (!$a->doIexist()) {
    return false;
  }

  $a->doSomeStuff();
  $b->wowIamSoComplex();
  if ($b->stillGoing() || $b->maybeThisWorks()) {
      return true;
  }

  return false;
}
?>

As you can see I still need a "return false" at the bottom. So the line count has actually gone up! I still feel that this is a better way to represent this piece of code.

I guess in this particular case I could take things one step further and also do:


<?php
function checkSomething($a, $b)
{
  if (!$a->doIexist()) {
    return false;
  }

  $a->doSomeStuff();
  $b->wowIamSoComplex();
  if (!$b->stillGoing() && !$b->maybeThisWorks()) {
    return false;
  }

  return true;
}
?>

This way I am consistently checking for conditions that should return false and if none of these match I return true.

Comments



Re: Returning early

I'm a bit ambivalent on this one since it changes the as-read semantics of the code. Its also easy for the guards to become lost if the code is added to so you might get stuff happneing in between the check and the critical section whcih doesnt depend on the existence of $a at all.

Of course equally sometimes its the right thing to do. For the reasons you have outlined. Its jsut Im not sure its a clear cut enough thing to make a point of style.

Re: Returning early

I'm writing using this style too, but don't simply use it for functions. Break or continue loops e.g. early too.

foreach ($list as $v) {
if ($v=='something') continue;
....
}

Another CS I'm missing quite often is "assign early":

$a = 'abc';
if (condition) {
$a = 'def';
}

instead of

if (condition) {
$a = 'def';
} else {
$a = 'abc';
}

Balu

Re: Returning early

What happened to the 'prefer a single point of return' principle (see e.g., http://www.leepoint.net/JavaBasics/methods/method-commentary/methcom-30-multiple-return.html
I think that in many cases it makes a function more robust.
However, for your function there seems to be no easy way to transform it into a single point of return.

Re: Returning early

I think it depends on the situation.

Sometimes you have to have a single return, if have allocated resources or changed some state within the engine, like error_reporting(0) whilst using filesystem calls.

Have been cases where have used...

$r = $foo->doA();
$r = $r && $foo->doB();
$r = $r && $foo->doC();
return $r && $foo->doD();

particulary with XmlWriter where each startElement()/writeAttribute() returns true / false.

Re: Returning early

Single point of return is usually an exercise in unnecessary complexity as the code becomes more context (state) sensistive. This makes it harder to refactor and harder to trace because you have to trace more code given an input that would otherwise return early thus introducing an opportunity for misunderstanding or miscomprehension. Minimizing nesting is always a good thing.

As mentioned above though there are exceptions to when you should return early such as when there are resources to manage or an action to perform regardless of what the result of the function is.

Re: Returning early

I'm also a fan of returning early (and early initialization too). I think that it can still usually be achieved even in the face of locking, file handling and other pre/post type work - although with an increase in code size.

Take for example:

$f = fopen("foo", "w");
// do work with multiple complex branches
fclose($f);

This could be redone using a small class:

class FileAutoCloser
{

public function __construct($f) { $this->f = $f; }
public function __destruct() { fclose($this->f); }
}

The initial code then becomes:

$f = fopen("foo", "w");
$fAutoCloser = new FileAutoCloser($f);
// do work with less complex branches by returning early

Re: Returning early

Hey, while I certainly agree on 'early return or early assignment', perhaps salted with a bunch of comments, the behaviour of ...($b->stillGoing() || $b->maybeThisWorks())... example will differ after the rewrite due to "short-circuited" code, or is that intended?

Re: Returning early

Hi! I also agree with your statement about returning early, though in this particular case following alternative perhaps might have a chance to live?

function checkSomething($a, $b)
{

····if ($a->doIexist()) {
········$a->doSomeStuff();
········$b->wowIamSoComplex();
········return $b->stillGoing() || $b->maybeThisWorks();

····} else {
········return false;
····}
}

Re: Returning early

This is http://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

Before you can post a comment please solve the following captcha.
your name


1  2  »