-
Notifications
You must be signed in to change notification settings - Fork 17
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
feat(mapper): allow to merge existing values by extracting identifiers #260
base: main
Are you sure you want to change the base?
Conversation
f57c1f9
to
d33b20f
Compare
171675c
to
0979712
Compare
745cb5b
to
12a3e45
Compare
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 do think it adds a lot of complexity to Array transformers (from/to), would it be possible to add some kind of abstraction to separate the pure mapping part & the identifier/existing values handling in separate class / traits or whatever ?
private readonly string $property, | ||
private readonly bool $private = false, | ||
public readonly string $property, | ||
public readonly bool $private = false, | ||
public readonly ?\ReflectionParameter $parameter = null, | ||
private readonly ?string $removeMethodName = null, |
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.
Can't we pass this as public as well ? Since it's readonly there is no point letting it alone as private.
$itemStatements[] = new Stmt\Expression(new Expr\Assign($mappedValueVar, $output)); | ||
$itemStatements[] = new Stmt\If_(new Expr\BinaryOp\NotIdentical(new Expr\ConstFetch(new Name('null')), $mappedValueVar), [ | ||
'stmts' => [ | ||
new Stmt\Expression($propertyMapping->target->writeMutator->getExpression($target, $mappedValueVar, $assignByRef)), | ||
], | ||
]); | ||
|
||
// @TODO handle existingValue |
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 introduce the possibility to deep merge object with collections and fetching the correct value to update it instead of adding a new one into the collection