Skip to content

[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

Merged
merged 1 commit into from
Feb 25, 2013

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Jan 14, 2013

Q A
Doc fix? no
New docs? yes (symfony/symfony#6595)
Applies to 2.2+
Fixed tickets #2130

Todo

  • Document write features
  • Render document and fix issues
  • Get reviewed by @bschussek

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.

@webmozart
Copy link
Contributor

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
Copy link
Member

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

Copy link
Member Author

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 )

Copy link
Member

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.

Copy link
Member Author

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.

@wouterj
Copy link
Member Author

wouterj commented Jan 18, 2013

I've added 2 new sections and squashed the commits. I think it's ready for merging.

ping @bschussek what do you think?

@weaverryan
Copy link
Member

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',
Copy link
Contributor

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.

@webmozart
Copy link
Contributor

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 PropertyPath class), constructing them programmatically (by using PropertyPathBuilder) and iterating them (using PropertyPathIterator).

Thanks @wouterj!

@wouterj
Copy link
Member Author

wouterj commented Feb 22, 2013

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.

@webmozart
Copy link
Contributor

I'm very fine with that :) Thank you so much!

@wouterj
Copy link
Member Author

wouterj commented Feb 22, 2013

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 weaverryan merged commit 5e5c93d into symfony:master Feb 25, 2013
@weaverryan
Copy link
Member

Merged! This is very awesome - great work @wouterj! I only had minor tweaks at sha: 24274f0. As always, if you notice anything that you don't like, please let me know.

And thanks to @bschussek for checking this over and for the nice feature!

Thanks!

@wouterj
Copy link
Member Author

wouterj commented Feb 25, 2013

@weaverryan thank you, what are your thoughts about the open discussion?


$person = new Person();

echo $accessor->getValue($person, 'first_name'); // 'Wouter'
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be firstName

Copy link
Member

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

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

Successfully merging this pull request may close these issues.

4 participants