C++ - Good or bad practice?

Question

I'm facing situations where i'm finding it handy to store the type (as an enum) of an object in the base class to further cast a pointer to that base class into a subclass pointer depending on the value of that type.

For example :

class CToken
{
public:
   token_type_e eType;
};

class COperatorToken : public CToken
{
public:
    // Sub class specific stuff
};

class CLiteralToken : public CToken
{
public:
    // Sub class specific stuff
};

And then

vector<CToken *> aTokens;

//...

for( size_t nI = 0, nMaxI = aTokens.size(); nI < nMaxI; ++nI )
{
   switch( aTokens[ nI ]->eType )
   {
   case E_OPERATOR :
      // Do something with sub-class specific stuff.
      break;
   case E_LITERAL :
      // Do something with sub-class specific stuff.
      break;
   }
}

Is it a bad practice ?

Thank you :)

EDIT:

Say i'm analyzing my token list. At some point i'll want to check if the current token is an operator using, as people suggest, a virtual function virtual bool isOperator(). Now if it IS an operator, i will want to access that sub class specific stuff to find out, for example, which type of operator it is. In that case, what can i do ? I can't add a method getOperatorType() in my base class, that wouldn't make sense. Is there another way than casting to subclass to retrieve that sub class member value ?

Answer 1

A type-field really defeats the object orientated nature of C++. Normally you can solve this with polymorphism:

In CToken define a function virtual doSomething(), with appropriate arguments and return type.

Implement that function in COperatorToken and CLiteralToken.

The runtime will call the appropriate function if you then use aTokens[ nI ]->doSomething();

Answer 2

Sounds like your trying to emulate an algebraic data type. Usually, you wouldn't do it this way, but by putting a pure virtual function on the base class and implementing the actual behavior in overrides in the derived classes.

Also, if you do ever need the pattern you propose, the language runtime knows the type so you don't need to store it:

#include <iostream>
#include <typeinfo>

class Token { };

class Operator : public Token { };

class Literal : public Token { };

int main()
{
    Token *tok = new Literal();

    if (typeid(*tok) == typeid(Literal)) {
        std::cout << "got a literal\n";
    }
    else if (typeid(*tok) == typeid(Operator)) {
        std::cout << "got an operator\n";
    }
}

Answer 3

I'd ask a different question: Why are you trying to reinvent the wheel rather than using existing possibilities, i.e. virtual members (polymorphism)? So I'd call it bad practice if there isn't some strong reason for doing it this way.

You can overload virtual members and even if your pointer is of the base class, you'll still call the member of the actual (sub) class (note that you'll typically want a virtual destructor as well, but I'm skipping this for simplicity):

class Token {
public:
    virtual void somethingSpecial() {
        std::cout << "Hello!" << std::endl;
    }
}

class Literal : public Token {
public:
    virtual void somethingSpecial() {
        std::cout << "I'm a literal!" << std::endl;
    }
}

class Operator : public Token {
public:
    virtual void somethingSpecial() {
        std::cout << "I'm an operator!" << std::endl;
    }
}

Then, in your iteration, you can do something as simple as this:

std::vector<Token*> tokens;

tokens.push_back(new Literal());
tokens.push_back(new Operator());
tokens.push_back(new Literal());

for (std::vector<Token*>:iterator a = tokens.begin(); a != tokens.end(); ++a)
    a->somethingSpecial();

The result will be similar to your code. The actual code being run will be based on the actual implementation in the sub class. In this case, you'd end up with the following output:

I'm a literal!

I'm an operator!

I'm a literal!

If a sub class doesn't implement the virtual function, the base class' version would be called (unless you make it abstract; then it would be a must-have).

Answer 4

You could simply do:

class CToken {
  public:
    virtual bool isLiteral(void) const = 0;
//...
};

and define it on the two subclasses.

And then, if you want to use the fact that's an operator or a literal, the solution is to declare a getValue() function, but NEVER use a switch(...) for that. In fact, if you want to go thought the Obect-Oriented practice, you should creeate a virtual function in CToken which allow subclasses to auto-interpret themselves, operators as operators and values as values.

Answer 5

You should never down cast - except when you need to.

An easy example of when it could be appropriate is passing messages around a framework. Say every message type must be serialisable so the base class interface would provide a virtual 'serialise()' method overridden by dervied classes, and all messages of any type are treated polymorphically by the messaging framework.

However, what those messages might represent and so the attributes and behaviours they possess could be wildly different - anything from GPS coordinates to an email to ephemeris data and that being the case trying to capture all that diversity in the base class's interface makes little sense. Once an agent has received a message it knows it's interested in (according to the message type) then (and only then) downcasting to the actual correct type in order to access meaningful message contents is appropriate - as you have then reached the point where the message cannot be treated in a purely abstract fashion.

Downcasting isn't in itself a failure. Using RTTI comes with a cost far higher than adding a simple enum field, and the oft recommended "keep dynamic casting until something works" deserves a far crueller punishment than I have the imagination to invent!