CODEWITHSUNDEEP
c++operator-overloading
Michael Kristofik
Michael Kristofik

Reputation: 35188

What's the right way to overload operator== for a class hierarchy?

Suppose I have the following class hierarchy:

class A
{
    int foo;
    virtual ~A() = 0;
};

A::~A() {}

class B : public A
{
    int bar;
};

class C : public A
{
    int baz;
};

What's the right way to overload operator== for these classes? If I make them all free functions, then B and C can't leverage A's version without casting. It would also prevent someone from doing a deep comparison having only references to A. If I make them virtual member functions, then a derived version might look like this:

bool B::operator==(const A& rhs) const
{
    const B* ptr = dynamic_cast<const B*>(&rhs);        
    if (ptr != 0) {
        return (bar == ptr->bar) && (A::operator==(*this, rhs));
    }
    else {
        return false;
    }
}

Again, I still have to cast (and it feels wrong). Is there a preferred way to do this?

Update:

There are only two answers so far, but it looks like the right way is analogous to the assignment operator:

Any user attempt to compare two objects of different types will not compile because the base function is protected, and the leaf classes can leverage the parent's version to compare that part of the data.

Upvotes: 82

Views: 180123

Answers (6)

Amir Kirsh
Amir Kirsh

Reputation: 13750

Below is a C++20 version of a solution similar to the one proposed by @MarkRansom, which:

  1. Reduces the boilerplate even more, getting operator!= generated automatically from operator==.
  2. Allowing == and != to be constexpr thus allowing checking equality inside static_assert.
  3. Using our own RTTI and static_cast instead of dynamic_cast, for allowing the operation to be constexpr.
  4. Allowing polymorphic comparison.

Code: https://coliru.stacked-crooked.com/a/0e68596a6becd1f6

The Base class:

using IdentityType = const int;
using Identity = IdentityType*;

class Base {
    int i; // just as an example that we can deal with base members
public:
    constexpr Base(int i): i(i) {}
    constexpr virtual ~Base() {}
    constexpr Base(const Base&) = default;
    constexpr Base& operator=(const Base&) = default;
    constexpr Base(Base&&) = default;
    constexpr Base& operator=(Base&&) = default;
    constexpr bool operator==(const Base& b) const {
        return getIdentity() == b.getIdentity() && equal(b);
    }
protected:
    constexpr virtual bool equal(const Base& b) const {
        return selfEqual(b);
    }
    constexpr virtual bool selfEqual(const Base& b) const {
        return i == b.i;
    }
private:
    constexpr virtual Identity getIdentity() const {
        return (Identity)0;
    }
};

The CRTP level:

template<typename ActualType, typename BaseAncestor, typename Base = BaseAncestor>
class Comparable: public Base {
public:
    template<typename... Args>
    constexpr Comparable(Args&&... args): Base(std::forward<Args>(args)...) {}
protected:
    constexpr bool equal(const BaseAncestor& b) const override {
        const auto& other = static_cast<const ActualType&>(b);
        return Base::equal(other) && selfEqual(other);
    }
    constexpr virtual bool selfEqual(const ActualType& other) const = 0;
    using Base::selfEqual;
private:
    constexpr static IdentityType identity = 0;
    constexpr virtual Identity getIdentity() const {
        return &identity;
    }
};

Actual Types:

class A: public Comparable<A, Base> {
    struct Content {
        int k;
        int j;
        constexpr bool operator==(const Content&) const = default;
    } content;
    using MyBase = Comparable<A, Base>;
public:
    constexpr A(int i, int k, int j): MyBase(i), content{.k=k, .j=j} {}
protected:
    constexpr bool selfEqual(const A& other) const override {    
        return content == other.content;
    }
};

class A2: public Comparable<A2, Base, A> {
    int m;
    using MyBase = Comparable<A2, Base, A>;
public:
    constexpr A2(int i, int k, int j, int m): MyBase(i, k, j), m(m) {}
protected:
    constexpr bool selfEqual(const A2& other) const override {    
        return m == other.m;
    }
};

It may seem too complicated, but it's important to note the simplicity of the derived classes (e.g. A and A2) which get the polymorphic comparison just by implementing selfEqual which can be implemented with an inner Content helper class, as shown in A.

Main:

(for additional asserts see link to code above).

constexpr A a1(1, 2, 3);
constexpr A a2(1, 2, 3);
constexpr A a3(1, 2, 4);

static_assert(a1 == a2);
static_assert(a1 != a3);

constexpr Base base1(1);
constexpr Base base2(2);
static_assert(base1 != base2);
assert(base1 != a1);
assert(a1 != base1);

const Base* pba1 = &a1;
const Base* pba2 = &a2;
const Base* pba3 = &a3;

assert(*pba1 == *pba2);
assert(*pba1 != *pba3);
assert(*pba1 == a2);
assert(a2 == *pba1);

constexpr A2 a2_1(1, 2, 3, 4);
constexpr A2 a2_2(1, 2, 3, 5);

static_assert(a2_1 == a2_1);
static_assert(a2_1 != a2_2);

C++17 version would be quite similar, dropping the constexpr and adding implementation for operator!=: https://coliru.stacked-crooked.com/a/635c08f0b02622ac

Above still uses our own RTTI, for cases RTTI is disabled, but can be easily replaced with using typeid: https://coliru.stacked-crooked.com/a/357cf10c1176d857

C++23 version can use typeid instead of our own RTTI and still be constexpr, as in C++23 type_info::operator== is constexpr: https://coliru.stacked-crooked.com/a/617514b41fca93ac

Upvotes: 3

CB Bailey
CB Bailey

Reputation: 791879

For this sort of hierarchy I would definitely follow the Scott Meyer's Effective C++ advice and avoid having any concrete base classes. You appear to be doing this in any case.

I would implement operator== as a free functions, probably friends, only for the concrete leaf-node class types.

If the base class has to have data members, then I would provide a (probably protected) non-virtual helper function in the base class (isEqual, say) which the derived classes' operator== could use.

E.g.

bool operator==(const B& lhs, const B& rhs)
{
    return lhs.isEqual( rhs ) && lhs.bar == rhs.bar;
}

By avoiding having an operator== that works on abstract base classes and keeping compare functions protected, you don't ever get accidentally fallbacks in client code where only the base part of two differently typed objects are compared.

I'm not sure whether I'd implement a virtual compare function with a dynamic_cast, I would be reluctant to do this but if there was a proven need for it I would probably go with a pure virtual function in the base class (not operator==) which was then overriden in the concrete derived classes as something like this, using the operator== for the derived class.

bool B::pubIsEqual( const A& rhs ) const
{
    const B* b = dynamic_cast< const B* >( &rhs );
    return b != NULL && *this == *b;
}

Upvotes: 38

marcinj
marcinj

Reputation: 49986

If you dont want to use casting and also make sure you will not by accident compare instance of B with instance of C then you need to restructure your class hierarchy in a way as Scott Meyers suggests in item 33 of More Effective C++. Actually this item deals with assignment operator, which really makes no sense if used for non related types. In case of compare operation it kind of makes sense to return false when comparing instance of B with C.

Below is sample code which uses RTTI, and does not divide class hierarchy into concreate leafs and abstract base.

The good thing about this sample code is that you will not get std::bad_cast when comparing non related instances (like B with C). Still, the compiler will allow you to do it which might be desired, you could implement in the same manner operator< and use it for sorting a vector of various A, B and C instances.

live

#include <iostream>
#include <string>
#include <typeinfo>
#include <vector>
#include <cassert>

class A {
    int val1;
public:
    A(int v) : val1(v) {}
protected:
    friend bool operator==(const A&, const A&);
    virtual bool isEqual(const A& obj) const { return obj.val1 == val1; }
};

bool operator==(const A& lhs, const A& rhs) {
    return typeid(lhs) == typeid(rhs) // Allow compare only instances of the same dynamic type
           && lhs.isEqual(rhs);       // If types are the same then do the comparision.
}

class B : public A {
    int val2;
public:
    B(int v) : A(v), val2(v) {}
    B(int v, int v2) : A(v2), val2(v) {}
protected:
    virtual bool isEqual(const A& obj) const override {
        auto v = dynamic_cast<const B&>(obj); // will never throw as isEqual is called only when
                                              // (typeid(lhs) == typeid(rhs)) is true.
        return A::isEqual(v) && v.val2 == val2;
    }
};

class C : public A {
    int val3;
public:
    C(int v) : A(v), val3(v) {}
protected:
    virtual bool isEqual(const A& obj) const override {
        auto v = dynamic_cast<const C&>(obj);
        return A::isEqual(v) && v.val3 == val3;
    }
};

int main()
{
    // Some examples for equality testing
    A* p1 = new B(10);
    A* p2 = new B(10);
    assert(*p1 == *p2);

    A* p3 = new B(10, 11);
    assert(!(*p1 == *p3));

    A* p4 = new B(11);
    assert(!(*p1 == *p4));

    A* p5 = new C(11);
    assert(!(*p4 == *p5));
}

Upvotes: 19

Denis Yaroshevskiy
Denis Yaroshevskiy

Reputation: 1327

  1. I think this looks weird:

    void foo(const MyClass& lhs, const MyClass& rhs) {
  if (lhs == rhs) {
    MyClass tmp = rhs;
    // is tmp == rhs true?
  }
}

  2. If implementing operator== seems like a legit question, consider type erasure (consider type erasure anyways, it's a lovely technique). Here is Sean Parent describing it. Then you still have to do some multiple-dispatching. It's an unpleasant problem. Here is a talk about it.

  3. Consider using variants instead of hierarchy. They can do this type of things easyly.

Upvotes: 1

Mark Ransom
Mark Ransom

Reputation: 308206

If you make the reasonable assumption that the types of both objects must be identical for them to be equal, there's a way to reduce the amount of boiler-plate required in each derived class. This follows Herb Sutter's recommendation to keep virtual methods protected and hidden behind a public interface. The curiously recurring template pattern (CRTP) is used to implement the boilerplate code in the equals method so the derived classes don't need to.

class A
{
public:
    bool operator==(const A& a) const
    {
        return equals(a);
    }
protected:
    virtual bool equals(const A& a) const = 0;
};

template<class T>
class A_ : public A
{
protected:
    virtual bool equals(const A& a) const
    {
        const T* other = dynamic_cast<const T*>(&a);
        return other != nullptr && static_cast<const T&>(*this) == *other;
    }
private:
    bool operator==(const A_& a) const  // force derived classes to implement their own operator==
    {
        return false;
    }
};

class B : public A_<B>
{
public:
    B(int i) : id(i) {}
    bool operator==(const B& other) const
    {
        return id == other.id;
    }
private:
    int id;
};

class C : public A_<C>
{
public:
    C(int i) : identity(i) {}
    bool operator==(const C& other) const
    {
        return identity == other.identity;
    }
private:
    int identity;
};

See a demo at http://ideone.com/SymduV

Upvotes: 10

mtvec
mtvec

Reputation: 18316

I was having the same problem the other day and I came up with the following solution:

struct A
{
    int foo;
    A(int prop) : foo(prop) {}
    virtual ~A() {}
    virtual bool operator==(const A& other) const
    {
        if (typeid(*this) != typeid(other))
            return false;

        return foo == other.foo;
    }
};

struct B : A
{
    int bar;
    B(int prop) : A(1), bar(prop) {}
    bool operator==(const A& other) const
    {
        if (!A::operator==(other))
            return false;

        return bar == static_cast<const B&>(other).bar;
    }
};

struct C : A
{
    int baz;
    C(int prop) : A(1), baz(prop) {}
    bool operator==(const A& other) const
    {
        if (!A::operator==(other))
            return false;

        return baz == static_cast<const C&>(other).baz;
    }
};

The thing I don't like about this is the typeid check. What do you think about it?

Upvotes: 17

Related Questions