[dev] [cvs] commit: framework/Horde/Horde ErrorHandler.php

[Michael M Slusarz]

Quoting Chuck Hagenbuch <chuck at horde.org>:

> Quoting Michael M Slusarz <slusarz at horde.org>:
>
>>  Log:
>>  Clean up PHP 5 code.
>
> Why this part?
>
> -        if (!($errno & self::$_mask)) {
> -            return;
> +        if ($errno & self::$_mask) {
> +            self::$_errors[] = array(
> +                'no' => $errno,
> +                'str' => self::_cleanErrorString($errstr),
> +                'file' => $errfile,
> +                'line' => $errline,
> +                'trace' => self::_errorBacktrace(),
> +            );
>          }
> -
> -        self::$_errors[] = array(
> -            'no' => $errno,
> -            'str' => self::_cleanErrorString($errstr),
> -            'file' => $errfile,
> -            'line' => $errline,
> -            'trace' => self::_errorBacktrace(),
> -        );
>
> That just increases the nesting level, no?

IMHO given an if clause at the bottom of a function especially when  
there is no return value and you are only doing work on one of the  
conditions, that return call is superfluous.

i.e., given $foo as a boolean variable:

function() {
   ...
   if ($foo) {
     return;
   }
   $foo2 = a;
}

(not so good) vs.

function() {
   ...
   if (!$foo) {
     $foo2 = a;
   }
}

(better).

> Also here:
>
> -        $headers = headers_list();
> -        foreach ($headers as $header) {
> +        foreach (headers_list() as $header) {
>
> Is repeated function calls in foreach no longer an issue?

There was never an issue with foreach and repeated function calls, as  
far as I know.  it is true that you wouldn't be able to do this in PHP  
4, but that was due to a limitation in the Zend engine.  foreach()  
makes a copy of the given array and in PHP 4 this was required to be a  
variable rather than something like an array function return.  But  
this is possible now in PHP 5.  So the assignment to $headers is  
unnecessary.

We still want to avoid foreach() for large arrays, but headers_list()  
should not be outputting more than a few KB of data so we should be  
fine here.

michael

--
___________________________________
Michael Slusarz [slusarz at horde.org]