Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72998
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.182 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CACQesk6jiYdzzibjzr2ba5XC3U1hSRCnj1gyaC1KNDJMaP1+sQ@mail.gmail.com>
References: <CAHMUw2GjeDRveLK99Q8U08y-3zqftAN8mLKPt=yikjT2BrNdBQ@mail.gmail.com>
	<CAMUwpuTxoiR-QhniYsBAebec0rusiu+FHoXx2hY=VY=z9um_mQ@mail.gmail.com>
	<CACQesk5+Wvy_kY9qjR6Ma9ChFjyvRjJrXa9DQYjOh43_uNu3hA@mail.gmail.com>
	<CACQesk6jiYdzzibjzr2ba5XC3U1hSRCnj1gyaC1KNDJMaP1+sQ@mail.gmail.com>
Date: Sat, 8 Mar 2014 08:27:45 +0800
Message-ID: <CAHMUw2HAOpM8E93rEyo6YVQab36sYbi4ssLoQPX+Kk8MMEJ-nA@mail.gmail.com>
To: Etienne Kneuss <colder@php.net>
Cc: Julien Pauli <jpauli@php.net>, PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11c3a92071b61904f40d70a8
Subject: Re: [PHP-DEV] [PR 614] ArrayObject and isset() / empty() behaviour
From: tjerk.meesters@gmail.com (Tjerk Meesters)

--001a11c3a92071b61904f40d70a8
Content-Type: text/plain; charset=ISO-8859-1

Hi Etienne,

Thanks for your input, much appreciated.

I have moved the decision up the chain as you suggested; the results can be
found here:

https://github.com/datibbaw/php-src/compare/php:PHP-5.4...bug66834a

It's suffering from a few memory leaks which can be fixed, but would this
be acceptable otherwise?



On Sat, Mar 8, 2014 at 12:12 AM, Etienne Kneuss <colder@php.net> wrote:

>
>
>
> On Fri, Mar 7, 2014 at 4:58 PM, Etienne Kneuss <colder@php.net> wrote:
>
>>
>>
>> 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.
>>
>>
> Apart from this underlying "design" issue, it is true an overridden
> version of ArrayObject is not functionally equivalent to the original
> class. So it seems worth fixing independently from this hack.
> --
> Etienne Kneuss
>



-- 
--
Tjerk

--001a11c3a92071b61904f40d70a8--