Skip to content

[Forms] Added data-mapper docs #10756

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

Closed
wants to merge 3 commits into from
Closed

Conversation

vudaltsov
Copy link
Contributor

#SymfonyConHackday2018

Improved version of #6900
Thank you @wouterj for a great job and the initial text! Thanx @HeahDude for helping me with this PR.

@vudaltsov
Copy link
Contributor Author

@stof, @xabbuh , need your review :)

Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

great work 👍 just some small minor comments


The red, green and blue form fields have to be mapped to the constructor
arguments and the ``Color`` instance has to be mapped to red, green and blue
form fields. Recognize a familiar pattern? It's time for a data mapper!
Copy link
Member

Choose a reason for hiding this comment

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

Maybe end this sentence with a colon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that ! brings some more joy and interaction here :)


.. tip::

You can also implement ``DataMapperInterface`` in the ``ColorType`` and add
Copy link
Member

Choose a reason for hiding this comment

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

[...] implement the DataMapperInterface [...]

Copy link
Member

Choose a reason for hiding this comment

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

Should we also talk about the possibility to implement the data mapper as an anonymous class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in this very case when we have two rather complicated methods to implement, an anonymous class will be harder to read.

@vudaltsov
Copy link
Contributor Author

@xabbuh , thank you for review, addressed your comments.

@xabbuh
Copy link
Member

xabbuh commented Dec 11, 2018

Thank you @vudaltsov.

xabbuh added a commit that referenced this pull request Dec 11, 2018
This PR was squashed before being merged into the 3.4 branch (closes #10756).

Discussion
----------

[Forms] Added data-mapper docs

#SymfonyConHackday2018

Improved version of #6900
Thank you @wouterj for a great job and the initial text! Thanx @HeahDude for helping me with this PR.

Commits
-------

b1cb1c0 [Forms] Added data-mapper docs
@xabbuh xabbuh closed this Dec 11, 2018
@vudaltsov vudaltsov deleted the data-mappers branch December 11, 2018 10:05
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.

4 participants