CODEWITHSUNDEEP
c#
Th3Fix3r
Th3Fix3r

Reputation:

Why does this object equality test fail?

Take the following class and unit test. 

public class Entity
    {
        public object Id { get; set; }

        public override bool Equals(object obj)
        {
            return this == (Entity)obj;
        }

        public static bool operator == (Entity base1, Entity base2)
        {
            if (base1.Id != base2.Id)
            {
                return false;
            }

            return true;
        }

        public static bool operator != (Entity base1, Entity base2)
        {
            return (!(base1.Id == base2.Id));
        }
    }

        [TestMethod]
        public void Test()
        {
            Entity e1 = new Entity { Id = 1 };
            Entity e2 = new Entity { Id = 1 };
            Assert.IsTrue(e1 == e2); //Always fails
        }

Can someone explain why its fails?

Upvotes: 3

Views: 712

Answers (8)

bruno conde
bruno conde

Reputation: 48265

Because you are relying on an object reference for comparator:

public object Id { get; set; }

Replace

    public static bool operator == (Entity base1, Entity base2)
    {
        if (base1.Id != base2.Id)
        {
            return false;
        }

        return true;
    }

With

    public static bool operator == (Entity base1, Entity base2)
    {
        return object.Equals(base1.Id, base2.Id);
    }

Upvotes: 3

Jon Skeet
Jon Skeet

Reputation: 1499810

Your Id property is of type object. When you construct two instances using 1 as the Id for each of them, you'll end up with two different boxed objects. You're then comparing those objects using reference equality.

Suggested changes to fix this:

  • Change the type of Id to be of type int if that's appropriate.
  • Use the static object.Equals method to compare Ids instead of ==

Either of these will work, but the first is preferable IMO.

There are various other ways in which the implementation could be made cleaner if you're interested, but I understand this may just be a dummy example. Just a quick list at a glance:

  • You should be overriding GetHashCode as well as Equals.
  • Your Equals override shouldn't perform a cast unconditionally, as that will throw an exception if the object is of the wrong type. If the type is wrong, return false.

  • Your current == implementation can be simplified to just

    return base1.Id == base2.Id;
  • Your == implementation should perform nullity checks
  • It's generally best to implement != by returning !(base1 == base2) unless you want specialist behaviour.
  • Overriding Equals in a non-sealed class can be problematic. Unless you're planning for inheritance, it would be worth sealing the class (IMO - this will probably be controversial).

Upvotes: 8

Macros
Macros

Reputation: 7119

Change the type of Id to an int, or other value type. The problem is that you are comparing 2 objects, which I assume is the problem that you overloaded the == operator to solve

Upvotes: 0

JerSchneid
JerSchneid

Reputation: 5917

If you made your Id member an int, it would work. But as CookieOfFortune said, when you're comparing two objects, it is looking to see if they are the exact same object, not whether they have the same value.

Upvotes: 0

tanascius
tanascius

Reputation: 53924

Your Id is an object, not an int. For objects the == operator will not check for value equality.

Upvotes: 2

Andy White
Andy White

Reputation: 88345

Your implementation of equals is just comparing the references, not the contents of the object. You're essentially comparing two pointers, and since you have created two separate objects, your equality will fail.

There are several articles on how to correctly implement Equals, here's a start:

http://weblogs.asp.net/tgraham/archive/2004/03/23/94870.aspx

For database entities, you can do a shortcut Equals implementation by just comparing the database IDs, assuming that two objects with the same ID are considered "equal" in your system.

Upvotes: 2

thecoop
thecoop

Reputation: 46098

Because your Id property is an object.

The 1 (as an int) gets boxed into a heap object, but each 1 gets boxed into a separate instance. As Id is an object, the base1.Id != base2.Id condition checks for reference equality rather than value equality, which is what you want. Changing Id to an int, or using Equals() rather than != should fix it.

Upvotes: 1

CookieOfFortune
CookieOfFortune

Reputation: 13974

Because e1.Id and e2.Id are different Objects. Even though they have the same value, they are not the same object, so the base1.Id == base2.Id fails.

Upvotes: 2

Related Questions