8
\$\begingroup\$

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.

\$\endgroup\$
  • 3
    \$\begingroup\$ Why did you make operator== as non-virtual method? \$\endgroup\$ – innochenti Mar 31 '12 at 15:41
  • \$\begingroup\$ Since I had the Equals() method, there was no need for operator== to be virtual. Isn't it ? But then, we don't need the Equals() method and we could just override the operator== by making the operator== in base class pure virtual there by forcing the implementation in derived classes. \$\endgroup\$ – Jagannath Mar 31 '12 at 23:31
  • \$\begingroup\$ looks like redundant to me. what benefits are you going to get from extra equals method? \$\endgroup\$ – innochenti Apr 1 '12 at 15:49
  • \$\begingroup\$ I agree. Equals() method does not bring any extra benifit. \$\endgroup\$ – Jagannath Apr 2 '12 at 0:34
5
\$\begingroup\$

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.

\$\endgroup\$
  • 2
    \$\begingroup\$ dynamic_cast will throw a bad_cast in case of reference type. \$\endgroup\$ – innochenti Mar 31 '12 at 15:37
  • \$\begingroup\$ @innochenti Erm … yes. Of course. \$\endgroup\$ – Konrad Rudolph Mar 31 '12 at 16:39
  • \$\begingroup\$ now ok. :) also, better auto other = dynamic_cast<const Circle*>(&c); 'cause there is type in dynamic_cast \$\endgroup\$ – innochenti Apr 1 '12 at 9:35
  • \$\begingroup\$ @innochenti Yes, I agree. I didn’t see the C++11 tag. \$\endgroup\$ – Konrad Rudolph Apr 1 '12 at 14:47
6
\$\begingroup\$

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.

\$\endgroup\$
5
\$\begingroup\$
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.

    }
};
\$\endgroup\$
  • \$\begingroup\$ do you mean that "other" exceptions should be caught outside equals function? \$\endgroup\$ – innochenti Apr 1 '12 at 9:49
  • \$\begingroup\$ Please do not spread Urban legends (or provide a reference that is not 20 years old). Exceptions are not expensive in modern implementations. This may have been a problem with the initial implementation of exceptions and caused a lot of bad rep to the use of exceptions but modern implementations are highly efficient. In the above code it is not the cost of the exception that worries me but the cost of the dynamic_cast. \$\endgroup\$ – Martin York Apr 1 '12 at 11:54
  • \$\begingroup\$ @LokiAstari, my wording came out stronger then I intended. My understanding is that throwing exceptions is more expensive then ordinary execution. Its not huge, but the performance difference is still there. \$\endgroup\$ – Winston Ewert Apr 1 '12 at 14:05
  • \$\begingroup\$ I would argue with "throwing exceptions is more expensive". If you handle exceptions; then If you take into account the extra code you have to manually insert to achieve the same affect as the exception handling code I doubt that exceptions are any more expensive than the manual code. If you don't handle the exception then sure it is cheaper. But it is a couple of years since I did this test and I am unsure how the exception mechanism has changed in the interim. \$\endgroup\$ – Martin York Apr 1 '12 at 19:57
  • \$\begingroup\$ @LokiAstari, see stackoverflow.com/questions/1018800/…. Of course this comes down to how the compiler/runtime implement exceptions, and will vary from case to case. \$\endgroup\$ – Winston Ewert Apr 1 '12 at 20:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.