Squiz Labs Blog - The latest news from the R&D arm of Squiz®

Lost in SWITCH-lation

09 Feb

Corny title, but a serious issue.

Recently, a simple change made to one of our functions broke a unit test, but it was not obvious how it could have broken when the only thing that was being tested was a PHP SWITCH statement. The code being tested looked something like this:

function test($var)
{
  switch ($var) {
    case 'abc':
      echo 'abc';
    break;
    case 'def':
      echo 'def';
    break;
    default:
      echo 'default';
    break;
  }
  
 }//end test()

A quick investigation found that when the value int(0) is passed into the SWITCH statement, "abc" is printed instead of "default". We had incorrectly assumed that the SWITCH was doing an identical check rather than just equality.

What actually happens is that PHP casts the CASE conditions to the same type as the input to the SWITCH statement. So in the case above, string(abc) is cast to an integer, becoming int(0), which then matches our input.

A more obvious way of doing the comparison would be to cast the input to the same type as the CASE comparison value. If this was done, int(0) would become string(0) and it would not match string(abc).

To be fair, the SWITCH documentation does say that it does a loose comparison, but it doesn't mention that the CASE values that the developer explicitly coded would be cast based on the type of the input. Internally, PHP probably just takes the two values and passes them to a comparison function that uses the first argument to determine the type. But still, the logic is a bit backwards. But PHP can't be changed now as we are too far down the road.

So how do you get around this?

In our case, we now cast the input to a string before passing it to the SWITCH to be evaluated:

function test($var)
{
   $var = (string) $var;
   switch ($var) {
    ...
   }

 }//end test()

How many developers out there cast SWITCH input before evaluating it?

It's not the sort of thing you would normally think about, but it is not enough to use the DEFAULT case as a catch-all for both value and type comparisons. You need to be positive you know the type of a variable before sending it to a SWITCH or you may get unexpected results.

So now we have a decision to make. Do we enforce casting for all SWITCH input in our coding standard (or at least flag it for code review) or do we put this down to a once-off considering this is the first time our team has encountered the issue in all our years of product development?