views:

1028

answers:

6

I was recently working on a piece of C++ code for a side project (the cpp-markdown library, for the curious), and ran into a coding question that I'd like some opinions on.

cpp-markdown has a base class called Token, which has a number of subclasses. Two of the main subclasses are Container (which holds collections of other Tokens) and TextHolder (used as a base class for Tokens that contain text, of course).

Most of the processing is handled via virtual functions, but some of it was better handled in a single function. For that, I ended up using dynamic_cast to down-cast the pointer from a Token* to one of its subclasses, so I could call functions that are specific to the subclass and its child classes. There's no chance the casting would fail, because the code is able to tell when such a thing is needed via virtual functions (such as isUnmatchedOpenMarker).

There are two other ways I could see to handle this:

  1. Create all of the functions that I want to call as virtual functions of Token, and just leave them with an empty body for every subclass except the one(s) that need to handle them, or...

  2. Create a virtual function in Token that would return the properly-typed pointer to this when it's called on certain subtypes, and a null pointer if called on anything else. Basically an extension of the virtual function system I'm already using there.

The second method seems better than both the existing one and the first one, to me. But I'd like to know other experienced C++ developers' views on it. Or whether I'm worrying too much about trivialities. :-)

+9  A: 

#1 pollutes the class namespace and vtable for objects that don't need it. Ok when you have a handful of methods that will generally be implemented, but plain ugly when only needed for a single derived class.

#2 is just dynamic_cast<> in a polka-dot dress and lipstick. Doesn't make client code any simpler, and tangles up the entire hierarchy, requiring the base and each derived class to be semi-aware of every other derived class.

Just use dynamic_cast<>. That's what it's there for.

Shog9
+4  A: 

If you want to get clever, you could also build a double dispatch pattern, which is two-thirds of the visitor pattern.

  • Create a base TokenVisitor class containing empty virtual visit(SpecificToken*) methods.
  • Add a single virtual accept(TokenVisitor*) method to Token that calls the properly-typed method on the passed TokenVisitor.
  • Derive from TokenVisitor for the various things you will need to do in varying ways over all tokens.

For the full visitor pattern, useful for tree structures, have the default accept methods iterate over children calling token->accept(this); on each.

Jeffrey Hantin
+4  A: 

If you know the conversion can't be invalid then just use static_cast.

Hans Passant
Agreed, if you're already checking the type with the virtual function, then dynamic_cast is useless. Both of your other solutions are just white washing that your hierarchy is broken. If your going to have a hack, at least use the faster hack.
BigSandwich
static_cast won't work if the derived class uses virtual inheritance to inherit from Token. dynamic_cast would not be useless in this case.
bk1e
+2  A: 

Why do you want to avoid using dynamic_cast? Is it causing an unacceptable bottle neck on your application? If not it might not be worth doing anything to the code right now.

If you are ok with trading some amount of safety for a bit of speed in your particular situation you should be fine doing a static_cast; however, this is cementing your assumption that you know the type of the object and there is no chance of the cast being bad. If your assumption becomes wrong later, you could end up with some mysterious crash bugs in your code. Going back to my original question, are you really sure the trade off is worth it here?

As for the options you listed: The first doesn't really sound like a solution as much as a last minute hack I would expect to see thrown in when someone was writing code at 3 AM. Functionality trickling its way towards the base of the class hierarchy is one of the most common anti-patterns hit by people new to OOP. Don't do it.

For the second option you listed any option like that are really just reimplementing dynamic_cast - if you are on a platform with only crap compilers available (I've heard stories about the Gamecube's compiler taking up a quarter of the system's available RAM with RTTI info) this might worthwhile, but more than likely you are just wasting your time. Are you really sure this is something worth concerning yourself about?

Andrew Khosravian
No, I'm not sure that this is worth concern. That's why I asked. :-)
Head Geek
Like mentioned above: static_cast fails if using virtual inheritance.
rstevens
+1  A: 

The true clean way to prevent dynamic_cast is to have pointers of the right type at the right place. Abstraction should take care of the rest.

IMHO, the reason that dynamic_cast has this reputation is because its performance degrades a little bit every time you add another sub-type down in your class hierarchy. If you have 4-5 classes in the hierarchy, it's nothing to worry about.

total
+1  A: 

Fun stuff. The dynamic_cast in the tokenizer implies that you want to actually split Token into something that creates the Token based on the current position in the text stream and the logic in Token. Each token will either be self-contained or have to create a Token just like above to process text correctly.

That way you pull out the generic stuff and still can parse based on the hierarchy of tokens. You can even make this whole parser data-driven without using dynamic_cast.

MSN