Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[ObjectMapper] embed collection transformer#60442
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.4
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
| use Symfony\Component\ObjectMapper\Tests\Fixtures\TransformCollection\A as TransformCollectionA; | ||
| use Symfony\Component\ObjectMapper\Tests\Fixtures\TransformCollection\B as TransformCollectionB; | ||
| use Symfony\Component\ObjectMapper\Tests\Fixtures\TransformCollection\C as TransformCollectionC; | ||
| use Symfony\Component\ObjectMapper\Tests\Fixtures\TransformCollection\D as TransformCollectionD; |
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 make the classe names longer instead of using an alias. It's simpler to call the class by its name.
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.
its nitpicking as these are test fixtures I don't see the benefits
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/ObjectMapper/Tests/Fixtures/TransformCollection/A.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Jérôme Tamarelle <jerome@tamarelle.net>
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Oskar Stark <oskarstark@googlemail.com>
| throw new MappingException(sprintf('The MapCollection transform expects an iterable, "%s" given.', get_debug_type($value))); | ||
| } | ||
| foreach ($value as &$v) { |
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 it'd be best to create a new array this is probably why psalm is not happy :P
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.
Indeed. Plus, if the iterator is not rewindable, the actual implementation can cause troubles.
This is another approach to embeded collection mapping that avoids changing the ObjectMapper. Even if this doesn't land in the component it'd be a good thing to document. Let me know your thoughts.