CSHARP-3315: Implement Equals for serializers. #1281
Conversation
|
Not ready for review yet. The new commits are just to back up work in progress in case something happens to my laptop. |
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Folder Include="Shared\" /> |
There was a problem hiding this comment.
I think this folder include is not needed.
There was a problem hiding this comment.
Not sure how it got here. Don't think I edited this file manually.
| object.Equals(_memberInfo, other._memberInfo) && | ||
| _order.Equals(other._order) && | ||
| object.Equals(_serializer, other._serializer) && | ||
| object.Equals(_shouldSerializeMethod, other._shouldSerializeMethod); |
There was a problem hiding this comment.
Do we need to compare _idGenerator?
There was a problem hiding this comment.
I think so. Looks like I missed it.
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <None Include="..\..\THIRD-PARTY-NOTICES" Pack="true" PackagePath="\" /> | ||
| <Folder Include="Shared\" /> |
There was a problem hiding this comment.
Same question as in Bson project.
|
|
||
| var result = x.Equals(y); | ||
|
|
||
| result.Should().Be(notEqualFieldName == null ? true : false); |
There was a problem hiding this comment.
notEqualFieldName can't be null?
There was a problem hiding this comment.
I will remove the null check. It's left over from some intermediate experiments.
|
|
||
| private BsonClassMap Clone(BsonClassMap classMap) | ||
| { | ||
| var clone = (BsonClassMap)FormatterServices.GetUninitializedObject(classMap.GetType()); |
There was a problem hiding this comment.
Do we have to use the Reflector here?
There was a problem hiding this comment.
I think we do because there is no easy way to change just ONE field at a time to test Equals, but we can do it easily using reflection.
There was a problem hiding this comment.
I have avoided using reflection wherever possible.
| [Fact] | ||
| public void Equals_with_equal_fields_should_return_true() | ||
| { | ||
| var x = (StandardDiscriminatorConvention)new ConcreteStandardDiscriminatorConvention("_t"); |
There was a problem hiding this comment.
minor: is this cast needed?
There was a problem hiding this comment.
Depends how you define "needed".
I wanted the compile time type of x to be the abstract base class, not the concrete type.
No description provided.