Tags

, ,

This had me pondering for a minute.

Quite often I write code along the lines of:

public function doStuff($stuff, $complex = FALSE) {
  if ($complex) {
      echo 'Doing complex stuff<br />';
  } else {
      echo 'Doing simple stuff<br />';
  }
}

Alrighty, this does not seem too bad.
If our default is $complex = FALSE, then we’ll always be doing “simple stuff”.

However, once the client (developer) accesses the method based on the given API … We could have a potential problem.

Let’s look at these scenarios:

$this->doStuff('some stuff'); //lazy developer, knows the API, doesn't bother with unnecessary argument
$this->doStuff('some stuff', FALSE); //proper developer, better be safe than sorry
$this->doStuff('some stuff', 'false'); //silly developer

As you can imagine in the first two cases we were “Doing simple stuff”… However because the third developer accidentally (or what have ya) enclosed the ‘false’ Boolean into a string PHP took care of type conversion and had to evaluate that string to meaning TRUE.

PHP’s fault? …. neah
Developer’s fault? …. neah (although some will argue that).
Lazy API developer? … most likely.

Let’s improve the method a little:

public function doStuff($stuff, $complex = FALSE) {
if (is_bool($complex) && $complex) {
     echo 'Doing complex stuff<br />';
  } else if (is_bool($complex) && !$complex) {
     echo 'Doing simple stuff<br />';
  } else {
     echo 'You have passed the wrong type, silly<br />';
  }
}

More, but safer code. Just wanted to point this out. Pick your own battles.