Closed (fixed)
Project:
D8 Entity API improvements (Sandbox)
Component:
Entity Property API
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
25 Sep 2012 at 08:37 UTC
Updated:
9 Oct 2012 at 10:21 UTC
Coming from http://drupal.org/node/1777268#comment-6459964:
<fago> dixon_, so a concern that jose raised is that setProperties() right now takes property objects and values, what's indeed not nice
dixon_, then I've that in setValue() as well, such that you can do $node->title = $node2->title;
dixon_, what makes it even more confusing :-)
dixon_, so I think unwrapping property objects should not be done in setValue() but in the magic getter or setter. so setValue would work only with values
<dixon_> fago: yeah, I've thought about that as well
<fago> dixon_, then, there is not much use for setProperties() as usually exchanging property objects cannot be allowed by the implementation anyway, so I thought about removing this one
dixon_, and instead do getPropertyValues() and setPropertyValues() whereas the firmer is what toArray() is now
dixon_, what do you think about this all?
<dixon_> fago: I'd actually not be against removing some of the magic. That would definitely be a bit more verbose. But it would at the same time lift up the underlying API to the surface and make people understand what's going on better
<fago> dixon_, true. but we might be able to speed up things thanks to the magic, seen http://drupal.org/node/1762258?
<Druplicon> http://drupal.org/node/1762258 => Speed up consequent calls to bypass magic => D8 Entity API improvements, Entity Property API, normal, needs review, 0 comments, 2 IRC mentions
<dixon_> fago: replacing get/setProperties() with get/setPropertyValues() makes a lot of sense, yeah
<dixon_> fago: yes, I've seen that. Let's start with the get/setPropertyValue() thing
<dixon_> that might make things a bit more straight forward and consistent
<fago> dixon_, it woudl be moving toArray() to getPropertyValues(). getProperties() would stay I think, it's like iterating over property objects.
<fago> dixon_, and with getProperties() you can specify whether you want to include computed properties
<dixon_> fago: true
fago: makes sense
<fago> dixon_, yep, let's do so then :-)
<dixon_> fago: agreed :)
Comments
Comment #1
fagoImplemented and pushed to entity-property-fago.