-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Pass the source member value to Condition #4655
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -454,9 +454,13 @@ private Expression CreatePropertyMapFunc(MemberMap memberMap, Expression destina | |
| : Assign(destinationMemberAccess, mappedMemberVariable); | ||
| if (memberMap.Condition != null) | ||
| { | ||
| var conditionMemberType = memberMap.Condition.Parameters[2].Type; | ||
| var conditionSourceMember = conditionMemberType.IsAssignableFrom(resolvedValueVariable.Type) | ||
| ? (Expression)resolvedValueVariable | ||
|
Comment on lines
+457
to
+459
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With the documented non-generic API Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed and fixed in fec985c. Reproduced it exactly as described: Root cause is as you diagnosed: the non-generic Took the first option — the wrapper for |
||
| : mappedMemberVariable; | ||
| _expressions.Add(IfThen( | ||
| _configuration.ConvertReplaceParameters(memberMap.Condition, | ||
| [customSource, _destination, mappedMemberVariable, destinationMemberGetter, ContextParameter]), | ||
| [customSource, _destination, conditionSourceMember, destinationMemberGetter, ContextParameter]), | ||
| mapperExpr)); | ||
| } | ||
| else if (!destinationMemberReadOnly) | ||
|
|
||
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.
If you want to keep the destination value when mapping
null, you can setUseDestinationValue, no need for conditions.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.
You're right, and it's the better answer for this shape — I checked it against
int?->int:ForAllMembers(o => o.UseDestinationValue())keeps 7 for a null source and takes 3 when set, and it works on a singleForMembertoo, where a condition can't see the null at all. Documented it in fec985c ahead of thePreConditionadvice.One caveat I noted with it, so nobody reads it as a blanket PATCH switch: on a member that is itself a mapped object it maps into the existing instance rather than replacing it, so the nested map still overwrites that object's own members (a source with only
Aset leftBat 0 on the preserved instance). Scalars are where it's a clean substitute for a condition.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'm not sure what you mean :) If the missing value marker is not
null, you need a condition, but other than that, it seems to me thatUseDestinationValueworks in all cases, inner maps included. Maybe I don't know what PATCH needs. A failing test?