Re: [PR 614] ArrayObject and isset() / empty() behaviour

On Fri, Mar 7, 2014 at 3:15 PM, Julien Pauli <jpauli@php.net> wrote: > On Fri, Mar 7, 2014 at 11:14 AM, Tjerk Meesters > <tjerk.meesters@gmail.com> wrote: > > Hi, > > > > I come across the following bug: > > https://bugs.php.net/bug.php?id=66834 > > > > With an extended ArrayObject instance, the behaviour of > > isset() and > > empty() is as follows: > > > > 1. Call static::offsetExists() > > 2. If return value is truthy, then > > a. isset() returns true > > b. empty() returns false > > c. spl_array_has_property() returns true > > > > However, if the ArrayObject instance is not overridden: > > > > 1. Fetch value (either via string or long key) > > 2. If key is found, then: > > a. isset() returns false if value is null, true > > otherwise. > > b. empty() returns true if value is truty, false otherwise. > > c. spl_array_has_property() returns true > > > > That's counterintuitive, so my patch addresses this by also calling > > offsetGet() if applicable. I wanted to bring this to the > > attention of > the > > group first, because it may affect performance to achieve this goal: > I have always been annoyed by this dichotomy between offsetExists() which is functionning like array_key_exists() and isset() which is returning false on defined, null values. Especially since isset() calls offsetExists() only. I would argue that the correct way to fix it is to have isset()/empty() call offsetGet() as well if it exists. So in that sense I feel like your patch is not addressing the right issue. In a sense ArrayObject does a hack to circumvent that in the first place, and you are trying to fix the hack to make it look more like what is supposed to happen. -- Etienne Kneuss