[development] comment module: minor issue unreachable code

[Rob Knight]

> I.m.h.o it would then need a rewrite to make sure the switch would 
> return it after it
> is finished.
>
> switch($foo) {
>
>     case ('bar'):
>         $result = 1;
>         break;
>     case ('otherbar');
>         $result = 2;
>         break;
>     default:
>         $result = 3;
> }
>
> return $result; // or whatever needs to be done with it.
>
> But then again, it's only my opinion.
>

This looks like a common coding practice that one would expect to see in 
a C application, but it is not necessarily suitable in PHP.  The reason 
is that in C, we declare the return type of a function and if a function 
has a return type then it *must* return a value.  Likewise, functions 
with a 'void' return type cannot return a value.  In PHP, we do not 
declare return types, therefore it is entirely possible that a function 
may return a value some times (depending on inputs) but not at other 
times.  Many Drupal functions work this way, particularly those 
functions with an $op parameter as in your original example.  We do a 
switch..case on the $op and might return a value for certain operations 
(e.g. 'view' or 'load') but not return a value for others (e.g. 
'delete').  This is why the return statement cannot be placed after the 
switch block, because we don't always have a value to return!

You are correct to say that the break statement after the return is, 
technically, redundant as it will never be executed.  However, it does 
help to maintain visual consistency between different case blocks, and 
it also ensures that the code cannot be broken by someone removing the 
return statement and forgetting to add a break.  switch statements are 
notoriously easy to mess up in comparison with if..else statements, due 
to the lack of braces and the occasional confusion over indentation 
levels.  Ensuring that every case has a break statement is a good way of 
avoiding silly errors (of course, there are *some* occasions where you 
don't want a break statement at all, but these are relatively unusual 
and I can't think of any examples in Drupal's code).

Regards,

Rob Knight



> Regards,
>
> Hans
>