Skip to content

CSHARP-4432: POC of standard implementations of Equals. #1273

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/MongoDB.Bson/EqualsPoc/EquatableBaseClass.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System;

namespace MongoDB.Bson.EqualsPoc
{
internal class EquatableBaseClass : IEquatable<EquatableBaseClass>
{
public object _ref1;
public int _value1;

public EquatableBaseClass(object ref1, int value1)
{
_ref1 = ref1;
_value1 = value1;
}

public override bool Equals(object obj) =>
Equals(obj as EquatableBaseClass);

public bool Equals(EquatableBaseClass other) =>
object.ReferenceEquals(this, other) ||
!object.ReferenceEquals(other, null) &&
this.GetType() == other.GetType() &&
object.Equals(_ref1, other._ref1) &&
_value1.Equals(other._value1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes:

Some very old articles claim the following is faster than using object.ReferenceEquals:

(object)this == other

but most newer articles say that after JITing they are exactly the same thing. Using object.ReferenceEquals is more intention-revealing than the more obscure cast.

We must check that the types are equal. Objects of different types are never equal.

My recommendation is that we NEVER use == in the implementation fo Equals. Instead, we should always call Equals when comparing fields.

The reason is that in some cases == and Equals do NOT return the same value. When writing and reading Equals implementations it is better to standardize on calling Equals to compare fields instead of having to analyze field by field whether == semantics are correct.

Always use object.Equals(_ref1, other._ref1) instead of _ref1.Equals(other._ref1) when comparing reference values. That way nulls are handled correctly.

Always use _value1.Equals(other._value1) when comparing struct values. That way boxing is usually avoided (most struct classes will implement IEquatable<T>).


public override int GetHashCode() => 0; // implement as appropriate

// optionally implement == and !=
public static bool operator ==(EquatableBaseClass lhs, EquatableBaseClass rhs) => object.Equals(lhs, rhs);
public static bool operator !=(EquatableBaseClass lhs, EquatableBaseClass rhs) => !(lhs == rhs);
}
}
28 changes: 28 additions & 0 deletions src/MongoDB.Bson/EqualsPoc/EquatableDerivedClass.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using System;

namespace MongoDB.Bson.EqualsPoc
{
internal class EquatableDerivedClass : EquatableBaseClass, IEquatable<EquatableDerivedClass>
{
private object _ref2;
private int _value2;

public EquatableDerivedClass(object ref1, int value1, object ref2, int value2)
: base(ref1, value1)
{
_ref2 = ref2;
_value2 = value2;
}

public override bool Equals(object obj) =>
Equals(obj as EquatableDerivedClass);

public bool Equals(EquatableDerivedClass other) =>
object.ReferenceEquals(this, other) ||
base.Equals(other) &&
object.Equals(_ref2, other._ref2) &&
_value2.Equals(other._value2);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes:

The object.ReferenceEquals(this, other) check must be coded at every level of a polymorphic class hierarchy for the optimization to apply at each level.

No need to check for null or compare types, that happens in base.Equals.


public override int GetHashCode() => 0; // implement as appropriate
}
}
31 changes: 31 additions & 0 deletions src/MongoDB.Bson/EqualsPoc/EquatableNonPolymorphicClass.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using System;

namespace MongoDB.Bson.EqualsPoc
{
internal sealed class EquatableNonPolymorphicClass : IEquatable<EquatableNonPolymorphicClass>
{
private object _ref;
private int _value;

public EquatableNonPolymorphicClass(object @ref, int value)
{
_ref = @ref;
_value = value;
}

public override bool Equals(object obj) =>
Equals(obj as EquatableNonPolymorphicClass);

public bool Equals(EquatableNonPolymorphicClass other) =>
object.ReferenceEquals(this, other) ||
!object.ReferenceEquals(other, null) &&
object.Equals(_ref, other._ref) &&
_value.Equals(other._value);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to compare types because the class is non polymorphic.

This assumption is only valid is the class is sealed.


public override int GetHashCode() => 0; // implement as appropriate

// optionally implement == and !=
public static bool operator ==(EquatableNonPolymorphicClass lhs, EquatableNonPolymorphicClass rhs) => object.Equals(lhs, rhs);
public static bool operator !=(EquatableNonPolymorphicClass lhs, EquatableNonPolymorphicClass rhs) => !(lhs == rhs);
}
}
29 changes: 29 additions & 0 deletions src/MongoDB.Bson/EqualsPoc/EquatableStruct.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using System;

namespace MongoDB.Bson.EqualsPoc
{
internal struct EquatableStruct : IEquatable<EquatableStruct>
{
private object _ref;
private int _value;

public EquatableStruct(object @ref, int value)
{
_ref = @ref;
_value = value;
}

public override bool Equals(object obj) =>
obj is EquatableStruct other && Equals(other);

public bool Equals(EquatableStruct other) =>
object.Equals(_ref, other._ref) &&
_value.Equals(other._value);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check for null because this is a struct.

No need to compare types because structs can't be polymorphic.


public override int GetHashCode() => 0; // implement as appropriate

// optionally implement == and !=
public static bool operator ==(EquatableStruct lhs, EquatableStruct rhs) => lhs.Equals(rhs);
public static bool operator !=(EquatableStruct lhs, EquatableStruct rhs) => !(lhs == rhs);
}
}