-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[WIP] Bootstrap PropertyAccess component #2136
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
Conversation
Cool, thanks for starting this! |
~~~~~~~~~~~~~~~~~~~~ | ||
|
||
And it don't even stop there. If there is no getter found, the accessor will | ||
look for a hasser or isser. This method is created the same as getters are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say a isser or a hasser
here, to give them in the same order than the one used by the code when checking the existance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stof thanks for the tip. If this is true it will make sense, but does the accessor have a specific order in which he looks for methods? I haven't had the time to take a look in the source code and after the blog article I thought it doesn't have a specific order ( _$data->getProp(), $data->isProp(), $data->hasProp(), $data->_get('prop') or $data->prop, whichever is found first )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whichever is found first, yes. But the order it which it looks for them comes from the implementation, and so is known: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/PropertyAccess/PropertyAccessor.php#L206
The order is the one in which the different ways are described in the sentence you copied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stof thank you, I understand it know. I will fix this soon. I think I need to rewrite some parts of the article, because I thought of a different order.
I've added 2 new sections and squashed the commits. I think it's ready for merging. ping @bschussek what do you think? |
ping @bschussek - hope you're feeling better! If you want to review this, then awesome, and if not - let me know and I'll merge it in, proofread it, etc. Thanks! |
|
||
// ... | ||
$person = array( | ||
'name' => 'Wouter', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should use 'first_name' everywhere instead of 'name' to better showcase what happens with underscores/camelizing.
Looks good for a start! (apart from my comments) If you feel motivated, you can also write a couple of sections on using property paths programmatically (by using the Thanks @wouterj! |
Thanks for the review @bschussek I will take a look at it soon. I think this is a good bootstrap to have before merging sf2.2 and I will take a look (and document) the other features somewhere in the development phrase of Sf2.3. |
I'm very fine with that :) Thank you so much! |
I have fixed the issues (except this one: #2136 (comment) ) I have also moved the file to its own directory, to make life easier when adding more articles about this component. It's ready for merging @weaverryan |
@weaverryan thank you, what are your thoughts about the open discussion? |
|
||
$person = new Person(); | ||
|
||
echo $accessor->getValue($person, 'first_name'); // 'Wouter' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be firstName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, it will work as the getter would be the same, but it is indeed confusing
Todo
This is the rough first version of the first part of the documentation about the new PropertyAccess component. I will work on this further the comming days. If you spot any problems or if you think something else is better, just leave a comment please.