CSHARP-5461: Add targetSerializer parameter to Translate methods. #1596
Conversation
…pressionTranslator.
…tionExpressionTranslator.Translate(context,.
| } | ||
| var creatorMapArguments = creatorMap?.Arguments?.ToArray(); | ||
|
|
||
| for (var i = 0; i < constructorParameters.Length; i++) |
There was a problem hiding this comment.
Should we iterate over creatorMap arguments instead? In such case we do not need to do a member lookup and will use custom member if it was configured explicitly.
There was a problem hiding this comment.
No because the creatorMap could be null.
We only have a creatorMap IFF the serializer is an IBsonClassMapSerializer and not all serializers are.
| coll.UpdateOne(filter, updateError, new() { IsUpsert = true }); | ||
| } | ||
|
|
||
| // [Fact] |
There was a problem hiding this comment.
We probably can either remove this test at all or expect an error here, as we do not support the functionality for now.
There was a problem hiding this comment.
I will remove it before the final commit for this PR.
For now I didn't want to forget about it, but I also didn't want the patch builds to fail.
| itemSerializer ??= itemTranslation.Serializer; | ||
| if (itemSerializer == null) | ||
| { | ||
| itemSerializer = itemTranslation.Serializer; | ||
| } |
There was a problem hiding this comment.
I think this change is unnecessary?
There was a problem hiding this comment.
You are right I guess. It's just two ways of writing the same thing.
I don't think I intentionally meant to change it. I think I deleted the line at one point and later realized I needed this to happen and I just wrote it differently.
Which version do you like better? The shorter version or the clearer version?
There was a problem hiding this comment.
I'll prefer the shorter version
No description provided.