8000 [PropertyAccess] PropertyPath read/write property with __call · Issue #4683 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[PropertyAccess] PropertyPath read/write property with __call #4683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
jaugustin opened this issue Jun 29, 2012 · 10 comments
Closed

[PropertyAccess] PropertyPath read/write property with __call #4683

jaugustin opened this issue Jun 29, 2012 · 10 comments
Labels
Feature PropertyAccess

Comments

@jaugustin
Copy link
Contributor

Hi,

PropertyPath doesn't read or write object's properties that are handle with a __call method.

Propel has a delegation behavior, that delegate setter/getter with __call,

@bschussek , @fabpot what do you think about that ?

I can provide a PR.

cc @willdurand
@jaugustin
Copy link
Contributor Author

@fabpot @bschussek do you want a PR for this ?

@fabpot
Copy link
Member
fabpot commented Jul 6, 2012

I don't know how it would work in a sane way. You can work on a PR though but I'm not convinced it is a good idea.

@webmozart
Copy link
Contributor

I'm not convinced this is a good idea either. Do you mean that the name of the searched setter method should be passed to __call?

This definitely shouldn't go into 2.1 anymore.

@jaugustin
Copy link
Contributor Author

the PropertyPath try to find a valid getter setter, in case it can't find it and there is a __call method, we could give a try by calling the default getter/setter $object->getMyProperty() or $object->setMyProperty($value)

maybe this could be configurable (if we want the __call fallback or not)

@jaugustin jaugustin mentioned this issue Jul 25, 2012
Closed
@ethanresnick
Copy link

I'm very much in favor of this, so I'll try to make a case for it.

Using __call() for getters and setters makes sense for people who:

  1. Don't like creating getters and setters directly, because maintaining them can be a pain as properties change and because they add a lot of noise that distracts from the salient features of the code; and
  2. Don't like the inconsistency that results from __get() and __set(), which leaves some properties accessed directly and others through methods.

Under these conditions, __call() becomes a great way to reduce boilerplate. But if the PropertyPath doesn't support it, then using it in many places (notably most model classes) becomes impossible.

Given that PropertyPath would be at a point where it has to throw an Exception anyway, I also see little downside to this.

@westinpigott
Copy link

I agree with ethanresnick. This would be very useful.
When implementing an EAV type of model (which is excellent in doctrine ODM) it would be convientent to be able to reference attributes with getNickname() and setNickname('something'). Currently to support this and the symfony2 form system, I must utilize __get, __set, and __call. The __call option is preferred, because extending the class allows somebody to override the __call with getNickname() and do some additional processing at this point. (perhaps the nickname is the first letter of the firstname plus the last name).

@westinpigott
Copy link

I'll work on this if it is not tackled by mid January. I am uber busy until then.

@jaugustin
Copy link
Contributor Author

@bschussek now that the PropertyAccess has been created can we imagine to handle __call.
I look at the code to overwrite but all methods are private is there a good reason for that ?
I was thinking to provide an extended PropertyAccessor but it seems that I should duplicate all code :(

@webmozart
Copy link
Contributor

@jaugustin You can add the functionality to the base PropertyAccessor, but it should be disabled by default. Enabling (or disabling) should work with the following API:

$accessor = PropertyAccess::getPropertyAccessorBuilder()
    ->enableMagicCall()
    //->disableMagicCall()
    ->getPropertyAccessor();

@jaugustin
Copy link
Contributor Author

@bschussek ok I will work on a PR

@jaugustin jaugustin mentioned this issue Mar 4, 2013
Merged
1 task
jaugustin added a commit to jaugustin/symfony that referenced this issue Apr 21, 2013
@jaugustin
[PropertyAccess] add support for magic call, related to symfony#4683
a785baa 
add a new propertyAccessorBuilder to enable / disable the use of __call by
the PropertyAccessor
fabpot added a commit that referenced this issue Apr 25, 2013
@fabpot
merged branch jaugustin/property-access-add-magic-call (PR #7263)
d8ac478 
This PR was merged into the master branch.

Discussion
----------

[PropertyAccess] add support for magic call

Hi,

I add support for magic call with the `PropertyAccess`

the is basic implementation, if no `getter`, `isser`, or `hasser` or `_get` is found and there is `__call` then the PropertyAccess call the getter

the same for setter.

This functionality is disable by default

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | seems OK (failure/errors are the same on master)
| Fixed tickets | #4683, #6413, #5309
| License       | MIT
| Doc PR        | symfony/symfony-docs#2472

- [x] submit changes to the documentation

@bschussek is this ok ?

Commits
-------

a785baa [PropertyAccess] add support for magic call, related to #4683
@fabpot fabpot closed this as completed Apr 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature PropertyAccess
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @webmozart @ethanresnick @jaugustin @westinpigott
0