3

I am having problems getting the desired behavior out of these few classes and interfaces.

Here is my problem,

//Inside a Unit Test that has access to internal methods and properties

INode  firstNode, secondNode;

INodeId  id = new NodeId (4);

first = new Node (id, "node");
second = new Node (id, "node");

Assert.IsTrue (first == second);

The assert above is failing because it seems to be going to the object class's equals method instead of the overloaded operator in the Node and NodeId classes.

If you have any suggestions on how I can get the desired behavior, that would be awesome.

Here is part of the Framework I am working on:

public interface IIdentifier<T> where T : class
{
    TKeyDataType GetKey<TKeyDataType> ();

    bool Equals (IIdentifier<T> obj;
}

public interface INode
{
    string name
    {
        get;
    }

    INodeId id
    {
        get;
    }
}

public interface INodeId : IIdentifier<INode>
{
}

public class Node : INode
{
    internal Node(INodeId  id, string name)
    { 
       //Work
    }

    public static bool operator == (Node n1, Node n2)
    {
        return n1.equals(n2);
    }

    public static bool operator != (Node n1, Node n2)
    {
        return !n1.equals(n2);
    }

    public bool Equals (INode  node)
    {
        return this.name == node.name &&
               this.id = node.id;
    }

    #region INode Properties

    }

public class NodeId : INodeId
{

    internal NodeId(int id)
    { 
       //Work
    }

    public static bool operator == (NodeId  n1, NodeId  n2)
    {
        return n1.equals(n2);
    }

    public static bool operator != (NodeId  n1, NodeId  n2)
    {
        return !n1.equals(n2);
    }

    public override bool Equals (object obj)
    {
        return this.Equals ((IIdentifier<INode>) obj);
    }

    public bool Equals (IIdentifier<INode> obj)
    {
        return obj.GetKey<int>() ==  this.GetKey<int>();
    }

    public TKeyDataType GetKey<TKeyDataType> ()
    {
        return (TKeyDataType) Convert.ChangeType (
            m_id,
            typeof (TKeyDataType),
            CultureInfo.InvariantCulture);
    }


    private int m_id;

}
Mark Canlas
  • 9,385
  • 5
  • 41
  • 63
Meiscooldude
  • 3,671
  • 5
  • 27
  • 30
  • Getting equality correct is quite tricky. You might be interested in these notes on some of the design considerations of different ways of computing equality: http://blogs.msdn.com/ericlippert/archive/2009/04/09/double-your-dispatch-double-your-fun.aspx – Eric Lippert Feb 02 '10 at 20:21
  • Just as an aside, you might want to consider overriding the `GetHashCode()` method as well. For this particular case, `NodeId`'s would simply return `m_id.GetHashCode()` and `Node`'s would compute a combination of the member fields. Jon Skeet has a great implementation here: http://stackoverflow.com/questions/263400/what-is-the-best-algorithm-for-an-overridden-system-object-gethashcode/263416#263416 – Jesse C. Slicer Jul 08 '10 at 15:54

3 Answers3

6

Operator overloads are resolved at compile time based on the declared types of the operands, not on the actual type of the objects at runtime. An alternate way of saying this is that operator overloads aren't virtual. So the comparison that you're doing above is INode.operator==, not Node.operator==. Since INode.operator== isn't defined, the overload resolves to Object.operator==, which just does reference comparison.

There is no really good way around this. The most correct thing to do is to use Equals() rather than == anywhere the operands might be objects. If you really, really need a fake virtual operator overload, you should define operator == in the root base class that your objects inherit from, and have that overload call Equals. Note, however, that this won't work for interfaces, which is what you have.

JSBձոգչ
  • 40,684
  • 18
  • 101
  • 169
  • Hey, I appreciate help. This will definitely put me in the right direction. – Meiscooldude Feb 02 '10 at 19:22
  • Note though that for the same reasons outlined in the answer, the compiler will pick `Object.Equals` and not `Node.Equals`, even if you call `first.Equals(second)`. – Fredrik Mörk Feb 02 '10 at 19:32
  • @Fredrik: `Equals` is virtual, though, so the method called will be the most highly-derived implementation of `Equals` on the actual runtime type. – JSBձոգչ Feb 02 '10 at 20:16
  • @JS: true. I found it odd, because the compiler picked `Object.Equals` when I tried out the given code sample, but then I realized that `Node` is not actually overriding `Object.Equals`. – Fredrik Mörk Feb 02 '10 at 21:10
1

I think you might need to override Equals(object) in Node like you did in NodeId. So:

public override bool Equals (object obj)
{
    if (obj is Node)
    {
        return this.Equals(obj as Node);
    }
    return false;
}

// your code (modified to take a Node instead of an INode)
public bool Equals (Node  node)
{
    return this.name == node.name &&
           this.id = node.id;
}
Mark Synowiec
  • 5,385
  • 1
  • 22
  • 18
  • This isn't correct (since Node is not sealed). If a class inherits from Node, then the reflexive property of Equals() will be lost. You need to check the EXACT type of obj and this. – Bryan Feb 02 '10 at 20:47
  • @Mark: you are missing the `override` keyword: `public override bool Equals (object obj)`. Apart from that, this answer solves the issue that OP is asking about. – Fredrik Mörk Feb 02 '10 at 21:22
  • @Brian - Yeah you're right, I copy and pasted the code under "// your code" and didn't realize it was taking an INode and not a Node. I'll update the post. Thanks! – Mark Synowiec Feb 02 '10 at 21:25
0

it's using the == from object because firstNode and secondNode aren't of type Node, they're of type INode. The compiler isn't recognizing the underlying types.

Since you can't overload an operator in an interface, your best bet is probably to rewrite the code so that it doesn't use the INode interface.

Jeff Hornby
  • 12,948
  • 4
  • 40
  • 61