views:

446

answers:

8

When comparing two objects (of the same type), it makes sense to have a compare function which takes another instance of the same class. If I implement this as a virtual function in the base class, then the signature of the function has to reference the base class in derived classes also. What is the elegant way to tackle this? Should the Compare not be virtual?

class A
{
    A();
    ~A();
    virtual int Compare(A Other);
}

class B: A
{
    B();
    ~B();
    int Compare(A Other);
}

class C: A
{
    C();
    ~C();
    int Compare(A Other);
}
A: 

If you mean that the Compare() in class B or C should always be passed an object of class B or C, no matter what the signature says, you can work with pointers to instances instead of instances, and try to downcast the pointer in the method's code using something like

int B::Compare(A *ptr)
{
   other = dynamic_cast <B*> (ptr);
   if(other)
      ...  // Ok, it was a pointer to B
}

(Such an overloading would be necessary only for those derived classes that add to the state of their parent something that influences the comparison.)

Federico Ramponi
A: 

A compare must be reflective, so:

let a = new A
let b = new B (inherits from A)

if (a.equals(b))
 then b.equals(a) must be true!

So the a.equals(b) should return false, since B probably contains fields that A doesn't have which means b.equals(a) will probably be false.

Thus, in C++ the comparison should be virtual I guess, and you should use type-checking to see that the parameter is of the "same" type as the current object.

Hugo
Comparison, not equals. think strcmp().
redmoskito
A: 

In addition to the dynamic_cast, you also need to pass a reference or a pointer, probably const. The Compare function can also probably be const.

class B: public A
{
    B();
    virtual ~B();
    virtual int Compare(const A &Other) const;
};


int B::Compare(const A &Other) const
{
    const B *other = dynamic_cast <const B*> (&Other);
    if(other) {
        // compare
    }
    else {
        return 0;
    }
}

EDIT: Must compile before posting...

David Norman
warning: in B::Compare, Other is an object, thus your code tries to convert an object to a pointer. Moreover, returning zero would mean equality; I would raise some exception instead
Federico Ramponi
you would raise some exception in this case? That's awful.
coppro
Thanks. The code should compile now.
David Norman
It's perfectly reasonable to assume that you might want to compare A and B of different types, inherited from the same base type. Your example seems to assume we only care about comparing when they are of the same type. Also, it's unclear what you intend to happen when they are of different types. It almost looks like you're implementing an equality operation, not comparison, and you're returning zero to indicate inequality and presumably 1 to indicate equality. That's different than what was asked in the question, which is a greater/less-than operation.
redmoskito
A: 

I hardly have this issue in C++. Unlike Java, we are not required to inherit all our classes from a same root Object class. When dealing with comparable (/value semantics) classes, it is very unlikely to have them come from a polymorphic hierarchy.

If the need is real in your particular situation, you are back in to a double-dispatch/multimethods problem. There are various ways to solve it (dynamic_cast, tables of functions for the possible interactions, visitors, ...)

Luc Hermitte
+1  A: 

I would implement it like this:

class A{
    int a;

public:
    virtual int Compare(A *other);
};


class B : A{
    int b;

public:
    /*override*/ int Compare(A *other);
};

int A::Compare(A *other){
    if(!other)
        return 1; /* let's just say that non-null > null */

    if(a > other->a)
        return 1;

    if(a < other->a)
        return -1;

    return 0;
}

int B::Compare(A *other){
    int cmp = A::Compare(other);
    if(cmp)
        return cmp;

    B *b_other = dynamic_cast<B*>(other);
    if(!b_other)
        throw "Must be a B object";

    if(b > b_other->b)
        return 1;

    if(b < b_other->b)
        return -1;

    return 0;
}

This is very similar to the IComparable pattern in .NET, which works very well.

EDIT:

One caveat to the above is that a.Compare(b) (where a is an A and b is a B) may return equality, and will never throw an exception, whereas b.Compare(a) will. Sometimes this is what you want, and sometimes it's not. If it's not, then you probably don't want your Compare function to be virtual, or you want to compare type_infos in the base Compare function, as in:

int A::Compare(A *other){
    if(!other)
        return 1; /* let's just say that non-null > null */

    if(typeid(this) != typeid(other))
        throw "Must be the same type";

    if(a > other->a)
        return 1;

    if(a < other->a)
        return -1;

    return 0;
}

Note that derived classes' Compare functions don't need to change, since they should call the base class's Compare, where the type_info comparison will occur. You can, however, replace the dynamic_cast in the overridden Compare function with a static_cast.

P Daddy
The issue there is that if the B part of the object is different, but the A part is the same, it returns equality.
coppro
How do you figure?
P Daddy
@coppro, I don't agree. Remember that 0 means equality.
Wimmel
I like this solution best of all the proposed solutions, but it assumes that the value of a is more significant than the value of b in comparison, and that b only matters when both a's are equal. This may be true, but it is a strong assumption, and it isn't always the case. Unfortunately, I still think the only answer to this question is, "it depends".
redmoskito
+1  A: 

Probably, I'd do it like this:

class A
{
 public:
  virtual int Compare (const A& rhs) const
  {
    // do some comparisons
  }
};

class B
{
 public:
  virtual int Compare (const A& rhs) const
  {
    try
    {
      B& b = dynamic_cast<A&>(rhs)
      if (A::Compare(b) == /* equal */)
      {
        // do some comparisons
      }
      else
        return /* not equal */;
    }
    catch (std::bad_cast&)
    {
      return /* non-equal */
    }
  }
};
coppro
There's still an issue that when you return "not equal" when the types are different, what do you return? You say "not equal", but your choices for "not equal" are -1 for greater-than or 1 for less-than. Neither makes sense in this case. Also, like "P-daddy's solution, this assumes that type B's value can't override type A's. Consider a comparison of an animal's "fierceness", where type A is "Animal" which compares strength alone, and type B is "Predator", which (for the sake of argument) is always more fierce than other animals, regardless of strength. B's value overrides A.
redmoskito
A: 

I would suggest to not make it virtual. The only disadvantage is that you explicit have to say which compare to use if the classes are not the same. But because you have to, you might spot an error (at compile time) which otherwise could cause a runtime error...

class A
{
  public:
    A(){};
    int Compare(A const & Other) {cout << "A::Compare()" << endl; return 0;};
};

class B: public A
{
  public:
    B(){};
    int Compare(B const & Other) {cout << "B::Compare()" << endl; return 0;};
};

class C: public A
{
  public:
    C(){};
    int Compare(C const & Other) {cout << "C::Compare()" << endl; return 0;};
};

int main(int argc, char* argv[])
{
    A a1;
    B b1, b2;
    C c1;

    a1.Compare(b1);     // A::Compare()
    b1.A::Compare(a1);  // A::Compare()
    b1.Compare(b2);     // B::Compare()
    c1.A::Compare(b1);  // A::Compare()

    return 0;
}
Wimmel
+1  A: 

It depends on the intended semantics of A, B, and C and the semantics of compare(). Comparison is an abstract concept that doesn't necessarily have a single correct meaning (or any meaning at all, for that matter). There is no single right answer to this question.

Here's two scenarios where compare means two completely different things with the same class hierarchy:

class Object 
{
    virtual int compare(const Object& ) = 0;
    float volume;
};

class Animal : Object 
{
    virtual int compare(const Object& );
    float age;
};

class Zebra  : Animal 
{
    int compare(const Object& );
};

We can consider (at least) two ways of comparing two Zebras: which is older, and which has more volume? Both comparisons are valid and easily computable; the difference is that we can use volume to compare a Zebra with any other Object, but we can only use age to compare Zebras with other Animals. If we want compare() to implement the age comparison semantics, it doesn't make any sense to define compare() in the Object class, since the semantics aren't defined at this level of the hierarchy. It's worth noting that neither of these scenarios require any casting, whatsoever, as the semantics are defined at the level of the base class (whether it be Object when comparing volume, or Animal when comparing age).

This raises the more important issue -- that some classes aren't suited to a single catch-all compare() function. Often it makes more sense to implement multiple functions that explicitly state what is being compared, like compare_age() and compare_volume(). The definition of these functions can occur at the point in the inheritance hierarchy where the semantics become relevant, and it should be trivial to adapt them to child classes (if the need adapting at all). Simple comparison using compare() or operator==() often only make sense with simple classes where the correct semantic implementation is obvious and unambiguous.

Long story short... "it depends".

redmoskito