Equals method implemented in the class

Question

Is this a proper way to implement an equality check? In Equals method, I am relying on bad_cast exception to know if the objects are of the same class or not.

Is there any other way to implement Equals() in C++?

class Shape
{
public:
    virtual ~Shape() = 0;
    bool operator==(const Shape& s) 
    {
        return Equals(s);
    }

    virtual bool Equals(const Shape& s) = 0;        
};



class Circle : public Shape
{


    bool Equals(const Shape& c) override
    {
        try
        {
            const Circle& other = dynamic_cast<const Circle&>(c);
                      // condition to check equality.
        }
        catch(std::bad_cast&)
        {
            return false;
        }
        catch(...)
        {
            throw;
        }
    }
};

class Square : public Shape   
{
    bool Equals(const Shape& s)  override
    {       
        try
        {
            const Square& other = dynamic_cast<const Square&>(s);
                      // condition to check equality.
        }
        catch(std::bad_cast&)
        {
            return false;
        }
        catch(...)
        {
            throw;
        }       
    }
};

typedef std::shared_ptr<Shape> ShapePtr;
typedef std::vector<ShapePtr> Shapes;

Shapes LoadShapes()
{
    Shapes shapes;
    shapes.push_back(std::make_shared<Circle>(42));
    shapes.push_back(std::make_shared<Circle>(52));
    shapes.push_back(std::make_shared<Circle>(62));
    shapes.push_back(std::make_shared<Square>(10));
    return shapes;
}

int main()
{
    auto circle = std::make_shared<Circle>(42);
    auto shapes = LoadShapes();
    for ( auto& shape : shapes)
    {
        if ( *shape == *circle)
        {
            std::cout << *shape << "\n";
        }
    }
}    

EDIT

Made the operator== as non-virtual method.

Answer 1

dynamic_cast on pointers will never throw. Instead, it will return 0.

Furthermore, why do you actually have the Equals function? You can make operator == virtual directly. This leaves us with:

class Circle : public Shape
{
    bool operator ==(const Shape& c) override
    {
        auto other = dynamic_cast<const Circle*>(&c);
        return other != 0 and /* check equality */;
    }
};

Apart from that, the code catch (...) { throw; } never makes sense. If you’re going to rethrow the exception without any action anyway, why catch it in the first place?

As per the comments, the declaration of other can be abbreviated. This is recommended in C++11, especially since the exact type of the variable is already explicitly mentioned in the initialiser expression.

Answer 2

Sorry to bump this one year old question, but there is something inherently wrong in your equality check if you ever extend to more than one level of inheritance.

Consider a class Foo deriving from Circle, assuming the same implementation (with changed types of course).

Foo foo;
Circle circle;
Shape & shapeFoo = foo;
Shape & shapeCircle = circle;

// these two should behave the same, but they don't:
std::cout << (shapeFoo == shapeCircle) << std::endl;
std::cout << (shapeCircle == shapeFoo) << std::endl;

Why does this fail?

shapeFoo == shapeCircle will call shapeFoo.Equals(shapeCircle) which will be dispatched on the implementation of Equals in class Foo. There the dynamic_cast<Foo const *> will fail, correctly returning false.

However shapeCircle == shapeFoo will call shapeCircle.Equals(shapeFoo) which will be dispatched on the implementation of Equals in class Circle. There the dynamic_cast<Circle *> will succeed (as Foo is a descendant of Circle). Then the method will compare only the attributes of Circle, probably returning true.

Answer 3

class Circle : public Shape
{


    bool Equals(const Shape& c) override

Why not a virtual "operator =="?

    {
        try
        {
            const Circle& other = dynamic_cast<const Circle&>(c);

Switching to pointer based dynamic_cast and checking for NULL might be better. Throwing and catching the exception is probably slightly more expensive than checking for null in and if. Its also not a particulairly common c++ idiom to catch exceptions as a logic decision like this.

                      // condition to check equality.
        }
        catch(std::bad_cast&)
        {
            return false;
        }
        catch(...)
        {
            throw;
        }

Why? If you don't want to do anything with other exceptions, don't catch them.

    }
};