Quantcast

Zend\Db: Why no __set() in AbstractRowGateway?

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Zend\Db: Why no __set() in AbstractRowGateway?

demiankatz

The documentation and examples led me to believe that I could use object or array access for interacting with Zend\Db\RowGateway objects.  However, object access doesn’t work for creating new objects.  For example:

 

// works

$r = new RowGateway('id', 'user', $adapter);

$r['username'] = 'fishy';

$r->save();

 

// doesn’t work

$r = new RowGateway('id', 'user', $adapter);

$r->username = 'fishy';

$r->save();

 

It appears that this could be easily remedied by simply adding this to the AbstractRowGateway class:

 

public function __set($offset, $value)

{

    return $this->offsetSet($offset, $value);

}

 

Is there a compelling reason why this feature is not present?  If it’s an oversight, I’ll go ahead and patch my copy of the framework until the next release.  If it’s an intentional design decision, a note about it in the documentation would probably be a good idea – the fact that object access works in some contexts and not others is potentially confusing.

 

thanks,

Demian

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: Zend\Db: Why no __set() in AbstractRowGateway?

Marco Pivetta
What would the advantage in having `__set` and `__get` here be?

Marco Pivetta

http://twitter.com/Ocramius     

http://marco-pivetta.com    



On 12 July 2012 17:18, Demian Katz <[hidden email]> wrote:

The documentation and examples led me to believe that I could use object or array access for interacting with Zend\Db\RowGateway objects.  However, object access doesn’t work for creating new objects.  For example:

 

// works

$r = new RowGateway('id', 'user', $adapter);

$r['username'] = 'fishy';

$r->save();

 

// doesn’t work

$r = new RowGateway('id', 'user', $adapter);

$r->username = 'fishy';

$r->save();

 

It appears that this could be easily remedied by simply adding this to the AbstractRowGateway class:

 

public function __set($offset, $value)

{

    return $this->offsetSet($offset, $value);

}

 

Is there a compelling reason why this feature is not present?  If it’s an oversight, I’ll go ahead and patch my copy of the framework until the next release.  If it’s an intentional design decision, a note about it in the documentation would probably be a good idea – the fact that object access works in some contexts and not others is potentially confusing.

 

thanks,

Demian


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

RE: Zend\Db: Why no __set() in AbstractRowGateway?

demiankatz

The AbstractRowGateway already has __get defined – so if you access an undefined property on the object, it looks into the internal data array and retrieves a value from the matching database column.  If you attempt to use the same technique to assign a value to a row (if, for example, you are trying to insert a new row into the database), you need __set to be defined in order to update the internal data array to match.

 

The current behavior, without __set, is that doing something like

 

$r->username = 'fishy';

 

creates a new public property on the object called username but does not touch the internal data array.  When you call the save() method, this property is ignored, and because the internal data array is empty, the insert operation fails.

 

- Demian

 

From: Marco Pivetta [mailto:[hidden email]]
Sent: Thursday, July 12, 2012 11:21 AM
To: Demian Katz
Cc: [hidden email]
Subject: Re: [zf-contributors] Zend\Db: Why no __set() in AbstractRowGateway?

 

What would the advantage in having `__set` and `__get` here be?

Marco Pivetta

http://twitter.com/Ocramius     

http://marco-pivetta.com    


On 12 July 2012 17:18, Demian Katz <[hidden email]> wrote:

The documentation and examples led me to believe that I could use object or array access for interacting with Zend\Db\RowGateway objects.  However, object access doesn’t work for creating new objects.  For example:

 

// works

$r = new RowGateway('id', 'user', $adapter);

$r['username'] = 'fishy';

$r->save();

 

// doesn’t work

$r = new RowGateway('id', 'user', $adapter);

$r->username = 'fishy';

$r->save();

 

It appears that this could be easily remedied by simply adding this to the AbstractRowGateway class:

 

public function __set($offset, $value)

{

    return $this->offsetSet($offset, $value);

}

 

Is there a compelling reason why this feature is not present?  If it’s an oversight, I’ll go ahead and patch my copy of the framework until the next release.  If it’s an intentional design decision, a note about it in the documentation would probably be a good idea – the fact that object access works in some contexts and not others is potentially confusing.

 

thanks,

Demian

 

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: Zend\Db: Why no __set() in AbstractRowGateway?

David Muir-2
In reply to this post by Marco Pivetta
If I'm understanding the problem, then this would illustrate it better:

$r['username'] = 'something';
echo $r['username']; //prints something
echo $r->username; //prints something

$r->username = 'fishy';
echo $r['username']; //prints something
echo $r->username; //prints fishy

Cheers,
David

On 13/07/12 01:20, Marco Pivetta wrote:
What would the advantage in having `__set` and `__get` here be?

Marco Pivetta

http://twitter.com/Ocramius     

http://marco-pivetta.com    



On 12 July 2012 17:18, Demian Katz <[hidden email]> wrote:

The documentation and examples led me to believe that I could use object or array access for interacting with Zend\Db\RowGateway objects.  However, object access doesn’t work for creating new objects.  For example:

 

// works

$r = new RowGateway('id', 'user', $adapter);

$r['username'] = 'fishy';

$r->save();

 

// doesn’t work

$r = new RowGateway('id', 'user', $adapter);

$r->username = 'fishy';

$r->save();

 

It appears that this could be easily remedied by simply adding this to the AbstractRowGateway class:

 

public function __set($offset, $value)

{

    return $this->offsetSet($offset, $value);

}

 

Is there a compelling reason why this feature is not present?  If it’s an oversight, I’ll go ahead and patch my copy of the framework until the next release.  If it’s an intentional design decision, a note about it in the documentation would probably be a good idea – the fact that object access works in some contexts and not others is potentially confusing.

 

thanks,

Demian




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

RE: Zend\Db: Why no __set() in AbstractRowGateway?

demiankatz
Yes, thank you, that is definitely a more direct illustration of the problem.

I have run these tests with ZF2 beta5, and they behave exactly as your comments predict.  This certainly feels like a bug to me -- the object should either work consistently or completely disallow one mode of access!  Should I open a JIRA ticket?

- Demian

From: David Muir [[hidden email]]
Sent: Thursday, July 12, 2012 8:25 PM
To: Marco Pivetta
Cc: Demian Katz; [hidden email]
Subject: Re: [zf-contributors] Zend\Db: Why no __set() in AbstractRowGateway?

If I'm understanding the problem, then this would illustrate it better:

$r['username'] = 'something';
echo $r['username']; //prints something
echo $r->username; //prints something

$r->username = 'fishy';
echo $r['username']; //prints something
echo $r->username; //prints fishy

Cheers,
David

On 13/07/12 01:20, Marco Pivetta wrote:
What would the advantage in having `__set` and `__get` here be?

Marco Pivetta

http://twitter.com/Ocramius     

http://marco-pivetta.com    



On 12 July 2012 17:18, Demian Katz <[hidden email]> wrote:

The documentation and examples led me to believe that I could use object or array access for interacting with Zend\Db\RowGateway objects.  However, object access doesn’t work for creating new objects.  For example:

 

// works

$r = new RowGateway('id', 'user', $adapter);

$r['username'] = 'fishy';

$r->save();

 

// doesn’t work

$r = new RowGateway('id', 'user', $adapter);

$r->username = 'fishy';

$r->save();

 

It appears that this could be easily remedied by simply adding this to the AbstractRowGateway class:

 

public function __set($offset, $value)

{

    return $this->offsetSet($offset, $value);

}

 

Is there a compelling reason why this feature is not present?  If it’s an oversight, I’ll go ahead and patch my copy of the framework until the next release.  If it’s an intentional design decision, a note about it in the documentation would probably be a good idea – the fact that object access works in some contexts and not others is potentially confusing.

 

thanks,

Demian




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: Zend\Db: Why no __set() in AbstractRowGateway?

weierophinney
Administrator
-- Demian Katz <[hidden email]> wrote
(on Friday, 13 July 2012, 11:12 AM +0000):
> Yes, thank you, that is definitely a more direct illustration of the problem.
>
> I have run these tests with ZF2 beta5, and they behave exactly as your comments
> predict.  This certainly feels like a bug to me -- the object should either
> work consistently or completely disallow one mode of access!  Should I open a
> JIRA ticket?

Yes... or fix the issue and provide a pull request. :)


> From: David Muir [[hidden email]]
> Sent: Thursday, July 12, 2012 8:25 PM
> To: Marco Pivetta
> Cc: Demian Katz; [hidden email]
> Subject: Re: [zf-contributors] Zend\Db: Why no __set() in AbstractRowGateway?
>
> If I'm understanding the problem, then this would illustrate it better:
>
>
> $r['username'] = 'something';
> echo $r['username']; //prints something
> echo $r->username; //prints something
>
> $r->username = 'fishy';
> echo $r['username']; //prints something
> echo $r->username; //prints fishy
>
>
> Cheers,
> David
>
> On 13/07/12 01:20, Marco Pivetta wrote:
>
>     What would the advantage in having `__set` and `__get` here be?
>
>     Marco Pivetta
>
>     http://twitter.com/Ocramius     
>
>     http://marco-pivetta.com   
>
>
>
>     On 12 July 2012 17:18, Demian Katz <[hidden email]> wrote:
>
>
>         The documentation and examples led me to believe that I could use
>         object or array access for interacting with Zend\Db\RowGateway
>         objects.  However, object access doesn’t work for creating new
>         objects.  For example:
>
>          
>
>         // works
>
>         $r = new RowGateway('id', 'user', $adapter);
>
>         $r['username'] = 'fishy';
>
>         $r->save();
>
>          
>
>         // doesn’t work
>
>         $r = new RowGateway('id', 'user', $adapter);
>
>         $r->username = 'fishy';
>
>         $r->save();
>
>          
>
>         It appears that this could be easily remedied by simply adding this to
>         the AbstractRowGateway class:
>
>          
>
>         public function __set($offset, $value)
>
>         {
>
>             return $this->offsetSet($offset, $value);
>
>         }
>
>          
>
>         Is there a compelling reason why this feature is not present?  If it’s
>         an oversight, I’ll go ahead and patch my copy of the framework until
>         the next release.  If it’s an intentional design decision, a note about
>         it in the documentation would probably be a good idea – the fact that
>         object access works in some contexts and not others is potentially
>         confusing.
>
>          
>
>         thanks,
>
>         Demian
>
>
>
>
>

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

RE: Zend\Db: Why no __set() in AbstractRowGateway?

demiankatz
> Yes... or fix the issue and provide a pull request. :)

Good point -- I'll do it on Monday; it's a good excuse to learn some new Git skills.

RowGateway doesn't seem to have any unit tests yet, so I'm unlikely to notice if my changes have obscure side effects.  Is anyone working on this, or am I missing something?  If it's helpful, I can at least include the start of a test to verify my own fix.

thanks,
Demian
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: Zend\Db: Why no __set() in AbstractRowGateway?

weierophinney
Administrator
-- Demian Katz <[hidden email]> wrote
(on Friday, 13 July 2012, 04:29 PM +0000):
> > Yes... or fix the issue and provide a pull request. :)
>
> Good point -- I'll do it on Monday; it's a good excuse to learn some new Git skills.
>
> RowGateway doesn't seem to have any unit tests yet, so I'm unlikely to
> notice if my changes have obscure side effects.  Is anyone working on
> this, or am I missing something?  If it's helpful, I can at least
> include the start of a test to verify my own fix.

RowGateway simply extends ArrayObject. You may be able to simply
override the constructor in order to provide the
ArrayObject::ARRAY_AS_PROPS flag.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

RE: Zend\Db: Why no __set() in AbstractRowGateway?

demiankatz
> RowGateway simply extends ArrayObject. You may be able to simply
> override the constructor in order to provide the
> ArrayObject::ARRAY_AS_PROPS flag.

Actually, RowGateway does not currently extend anything.  It just implements the ArrayAccess interface.  Perhaps this changed at some point in the past, and that refactoring caused the issue I am seeing.  For now, proxying offsetSet through __set seems like the simplest DRY solution.

In any case, I have made my first pull request:

https://github.com/zendframework/zf2/pull/1895

Thanks for encouraging me to do this; please let me know if there are any problems with it!

thanks,
Demian
Loading...