Re: Merge PR 621

From: Date: Thu, 20 Mar 2014 20:05:47 +0000
Subject: Re: Merge PR 621
References: 1 2 3 4 5  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message 
Hi!

> The current behavior is:
> 
> isset()   ->   has_dimension(check_exmpty=false)   ->   offsetExists()
> empty()   ->   !has_dimension(check_exmpty=true)   ->   !offsetExists()
> || offsetGet()

I think isset part is wrong - it should use offsetGet (probably via
check_empty=true) and then verify that the result is not null.

> This patch is about decoupling has_dimension from isset()/empty() and
> doing, instead:
> 
> isset()   ->   has_dimension() && get_dimension() !== NULL   ->  
> offsetExists() && offsetGet() !== NULL
> empty()   ->   !has_dimension() || !get_dimension()   ->  
> !offsetExists() && !offsetGet() 

I'm not sure this is the right way to go. It changes the existing API
without any need - while it can be handled within the existing API as well.

> Now it is clear it is a BC breaking change for people that defined
> offsetExists with array_key_exists: isset() which returned true for NULL
> will now invoke offsetGet and thus return false, but it is a change
> towards more consistency.

This is actually OK, isset should return for ArrayObject the same it
would return for array with the same data.

-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Thread (23 messages)

« previous php.internals (#73344) next »