Is this a good method to create class hierarchies?

Question

Is it a good design to store the type of the object in the base class as an enum? For example, consider the following hierarchy

Expr
--Unary 
--Binary
--Const
----Int
----Float

How to represent the above hierarchy in C++, so that I can inspect the type of an object of a subclass.

enum ExprType {
   Const, Binary, Unary
};

class Expr {
   ExprType type;
public:
   Expr(ExprType t) : type(t) { }
   ExprType GetType() { return type; }
};

class BinNode: public Expr {
   Expr* left, *right;
   Operator op;
public:
   BinNode() : Expr(ExprType::Binary) { }
};

class UnrNode: public Expr {
   Expr* operand;
   Operator op;
public:
   UnrNode() : Expr(ExprType::Unary) { }
};

enum ConstNodeType {
   Int, Float
};

class ConstNode: public Expr {
   ConstNodeType cntype;
public:
   ConstNode(ConstNodeType t) : Expr(ExprType::Const), cntype(t) { }
   ConstNodeType GetConstType() { return cntype; }
};

class IntNode: public ConstNode {
   int value;
public:
   IntNode() : ConstNode(ConstNodeType::Int) { }
};

class FloatNode: public ConstNode {
   float value;
public:
   FloatNode() : ConstNode(ConstNodeType::Float) { }
};

Now I can do,

Expr* node = new IntNode();
node->GetType(); // returns ExprType::Const
node->GetConstType(); // return ConstNodeType::Int

Is this is good idiomatic way to create a hierarchy of classes? I basically want to check the type of an object of any subclass of Expr.

I can do it in C# using the is operator without a special type member.

Expr node = new IntNode();
assert(node is ConstNode);
assert(node is IntNode);

In C++, I can use RTTI, but is this the idiomatic way to create a class of hierarchies? Is there any alternative way to express the same hierarchy?

Answer 1

Whenever you feel an urge to inspect the dynamic type of your polymorphic objects at run-time, you should question your design. This is true for any object-oriented language I know. The visitor pattern can be of great help in avoiding to bother with the dynamic type of an object.

Some people seem to think that cheating around type inspection by adding a tag to each object will make for a better design. I strongly disagree with these people. If you cannot (or are not willing to) avoid the run-time type inspection, admit it and use the (presumably most efficient) mechanisms for doing this provided natively by the language. Adding a type tag only increases run-time overhead and generally makes the code more complex introducing a new chance to add bugs.

Given the following hierarchy

struct Base { virtual ~Base(); };
struct Derived : Base { void foo(); };

and a pointer p to Base, the idiomatic way to check whether it actually points to a Derived object is to try a dymanic_cast like so:

bool
is_this_a_derived(Base * p)
{
  return dynamic_cast<Derived *>(p);
}

The dynamic_cast will return a Derived pointer to the object if it is one and a nullptr otherwise. The implicit conversion to bool will do the correct thing, then.

If you want, you can encapsulate this into a generic template

template <typename BaseT, typename DerivedT>
bool
is_instance_of(const BaseT * p) noexcept
{
  return dynamic_cast<const DerivedT *>(p);
}

and use it like this. Though personally I'd question the usefulness of this.

void
example(Base * p)
{
  if (is_instance_of<Derived>(p))
    std::cout << "We have a derived thing here.\n";
  else
    std::cout << "This thing is not derived.\n";
}

Often times, you will probably want to do something specific to the derived class. For example, invoke its foo member function in this example. The idiomatic way to do this would look like this:

void
invoke_foo_if_this_is_a_derived(Base * basep)
{
  if (auto derivedp = dynamic_cast<Derived *>(basep))
    derivedp->foo();
}

The code uses the fact that I can limit the scope of the derivedp to the body of the if statement – the only place where it is useful and guaranteed to be not a nullptr.

For these examples to work, it is crucial that Base has at least one virtual member. Usually, you will want to make at least the destructor of the base class virtual because otherwise deleteing a derived object through a base class pointer would invoke undefined behavior. The code in your question is therefore not safe (unless you plan to leak everything anyway) because Expr has no virtual destructor.

Answer 2

No, this is not idiomatic. If you absolutely must cast from base type to derived type, use the RTTI mechanism provided by the language. But first, question why you want to do that at all!

In most instances you shouldn't need to care what the type of the object is, you should be calling virtual member functions that the derived type implements to do the right thing for its type.

Answer 3

It is perfectly fine, as long as the class hierarchy on which you are doing this is small, self-contained, and not liable to be extended by someone who is not at liberty to refactor it.

The visitor pattern is not a bad idea, but it is a hassle to implement, and more importantly, every time you try to read it, you are forced to read a lot of code. Compare this to the simplicity, straightforwardness, and all-in-one-place neatness of switching on an enum, or calling "is_xyz()" methods on the base class.

You employ a pattern of this kind for convenience, not for keeping the purists happy.

There was a similar question here: Is it okay to have objects that cast themselves, even if it pollutes the API of their subclasses? about a similar mechanism, and my answer there suggesting that doing this is fine got a score of -5, (5 upvotes and 10 downvotes,) so it appears that the number of people who would disagree with me is twice the number of people who would agree. Nonetheless, I stand by my statement. I have not 3, not 5, not 10, but 30 years of uninterrupted full-time programming experience, and this is my opinion, for what it's worth.