[dev] Re: [cvs] commit: horde/lib String.php

[Michael M Slusarz]

Quoting Chuck Hagenbuch <chuck at horde.org>:

| Quoting Jan Schneider <jan at horde.org>:
| 
| > Please revert this.
| > I recognized during a performance audit that this function was a real
| > performance hit. Chuck and me also tested the performance of
| > array_key_exist() vs. isset() and detected that isset() is around twice
| > as fast. Unless there is really a reason to use array_key_exists(), we
| > should in the future use isset() again.
| 
| I agree with Jan, fwiw - I hadn't bothered to test this before, but you
| can look at http://marina.horde.org/wip/isset.php and
| http://marina.horde.org/wip/isset.phps (please copy and confirm locally).

My numbers from this script work out to be that isset() is approximately 
25% faster that array_key_exists().  So you are right about that (although 
I am not getting anywhere near 100% performance increases like you and/or 
Chuck).

Here's the catch though.  isset() returns false if the value is set to 
null.  Thus, the following script:

-----
$a = array();
$a['b'] = null;

if (isset($a['b'])) {
  echo "HI";
}
-----

would output nothing.  A bunch of our code uses null as an actual value, so 
this would result in screwy behavior.  If you ask me, in this case 
index 'b ' _is_ set and has the value null - null is a perfectly fine value 
if you ask me.  Thus isset returning false here is non-obvious in my book.

Additionally, the following code:

-----
$a = 'Some text.';
if (isset($a['hi_i_am_a_fake_index'])) {
  echo 'HI';
}
-----

amazingly enough outputs HI.  I realize this is bad coding, but this might 
be a situation where a bug would be difficult to track down - especially if 
the variable was defined elsewhere.  Once again, this is a case where I 
personally would expect isset() to return the opposite of what it actually 
does.

In both cases, array_key_exists() correctly handles these cases (correctly 
in the sense that it will return what you think it should return... 
whatever that means).

Thus the argument to keep array_key_exists() is to ensure correct handling 
of this kind of situation Horde-wide.  It doesn't really matter to me - but 
these were the reasons I orginally wanted to switch to array_key_exists().

michael

______________________________________________
Michael Slusarz [slusarz at bigworm.colorado.edu]
The University of Colorado at Boulder