Skip to content
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.

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

can access the field of a object through same name method #149

Closed
wants to merge 3 commits into from
Closed

can access the field of a object through same name method #149

wants to merge 3 commits into from

Conversation

oxkhar
Copy link

@oxkhar oxkhar commented Apr 15, 2018

A object in collection with private attribute, and accesor is a method
which name is the same. Closure expression visitor can access value
throught that metod if exists.

A object in collection with private attribute, and accesor is a method
which name is the same. Closure expression visitor can access value
throught that metod if exists.
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a BC break, since this changes existing behaviour and allows accessing fields via accessor even though the author of the code may explicitly have deviated from the get$property naming specifically to avoid this kind of implicit fetches.

I'm conflicted on whether this should actually be considered for inclusion...

@alcaeus
Copy link
Member

alcaeus commented Apr 16, 2018

This is a BC break

I don't think so. The BC break would be preferring someField() to isSomeField(), which is easily fixed. Besides that, I don't see this as a BC break. Just like getSomeField and isSomeField are valid accessors for someField, so is someField(?<type> $someField). Some people prefer that access pattern for boolean flags as opposed to having setFlag and isFlag (or enableFlag, disableFlag and isFlag).

@Ocramius
Copy link
Member

@alcaeus personally speaking, I do use this kind of accessor naming SPECIFICALLY to prevent tooling from auto-discovering it

@alcaeus
Copy link
Member

alcaeus commented Apr 17, 2018

personally speaking, I do use this kind of accessor naming SPECIFICALLY to prevent tooling from auto-discovering it

That sounds like there's a different problem going on, namely these tools being used when they shouldn't. IMO this is a valid access style for properties and shouldn't be used to "hide something from tooling", especially since the Symfony PropertyAccessor component already reads it, so this is a very weak protection against unwanted access. Fact is, if you expose a relatively standard accessor (which this is), tooling should be able to use it.

A object in collection with private attribute, and accesor is a method
which name is the same. Closure expression visitor can access value
throught that metod if exists.
@oxkhar
Copy link
Author

oxkhar commented Apr 18, 2018

Personally I made this request because is not strange that your entities define getters accessors at its private properties with a same name method. It's a convention that would be considered in order to get attributes values, and if you prefer I set in last order to be in mind...

@Ocramius
Copy link
Member

It's a convention that would be considered in order to get attributes values, and if you prefer I set in last order to be in mind...

I write my accessors specifically as class Foo { private $bar; function bar() { return $this->bar; }} to avoid tooling that calls getBar() magically.

I'd say that we should deprecate the entire property access via getter instead, and rely only on state for lookups, which would remove the discussion in first place. Still, that is not for me to decide here, but I'm gonna close this PR as won't fix for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants