Re: Merge PR 621
On Tue, Mar 25, 2014 at 11:34 AM, Stas Malyshev <[email protected]>wrote:
> Hi!
>
> > Do you have any issues with the code as it stands now?
> >
> > https://github.com/php/php-src/pull/614
>
> Something I don't understand in this patch: so if we don't have "has"
> override, we'd get value == NULL and then we go and check internal
> hashtable. Shouldn't we first check the get override and ask it with
> BP_VAR_IS, and only if get override is not there use direct hash access?
>
It's my understanding that you should (have to) only call "get" if you know
that the offset exists.
>
> E.g. imagine this:
>
> class SecretArrayObject extends ArrayObject
> {
> public function offsetGet($offset) {
> var_dump('Called: '.__METHOD__);
> return parent::offsetGet(str_rot13($offset));
> }
> public function offsetSet($offset) {
> var_dump('Called: '.__METHOD__);
> return parent::offsetSet(str_rot13($offset));
> }
> }
>
> I think on this example your code may fail to go to correct items when
> doing isset/isempty, since if I ask for 'qux' it would actually check
> the value I've put there for 'dhk'.
>
This particular class should override offsetExists($offset) as well, i.e.:
public function offsetExists($offset)
{
return parent::offsetExists(str_rot13($offset));
}
> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
> (408)454-6900 ext. 227
>
--
--
Tjerk
Thread (23 messages)