[ObjectMapper] embed collection transformer #60442
Conversation
src/Symfony/Component/ObjectMapper/Tests/Fixtures/TransformCollection/A.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Jérôme Tamarelle <[email protected]>
| 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.
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.
Indeed. Plus, if the iterator is not rewindable, the actual implementation can cause troubles.
| { | ||
| } | ||
|
|
||
| public function __invoke(mixed $value, object $source, ?object $target): mixed |
There was a problem hiding this comment.
Name "$value" does not say anything about what this parameter means from a semantical POV.
"$sourceProperty" might be a better name. Calling a field/property of a class a "variable" is very much confusing to me.
There was a problem hiding this comment.
I agree but the convention is to name the variable the same as its defined inside the interface, here its the value being mapped.
There was a problem hiding this comment.
Uff. Then this should definitely be changed in the interface. A field/property is not a variable ☝️🤓
interface TransformCallableInterface
{
/**
* @param mixed $propertyValue The property being mapped
* @param T $source The object we're working on
* @param T2|null $target The target we're mapping to
*/
public function __invoke(mixed $propertyValue, object $source, ?object $target): mixed;
}
```There was a problem hiding this comment.
It's the value of an object's key, in PHP it's not necessary a property though its nitpicking.
Co-authored-by: Oskar Stark <[email protected]>
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.