using virtual function vs dynamic_cast

Question

Instead of using a virtual function, is it fine to use something like:

void BaseClass::functionName () {  // BaseClass already has virtual functions
    // some LONG code true for all derived classes of BaseClass
    // ...
    if (typeid (*this) == typeid (DerivedClass1))
        // use functions of DerivedClass1 on dynamic_cast<DerivedClass1*>(this)
    else if (typeid (*this) == typeid (DerivedClass2))
        // use functions of DerivedClass2 on dynamic_cast<DerivedClass2*>(this)

    // some LONG code true for all derived classes of BaseClass
    // ...
 }

It's just that I feel it's not a good idea to use virtual functions for something like the above when it is only a small section that is specialized for the derived classes. The long code that is used for all the derived classes will then need to be used over and over for all the derived classes (suggesting a helper function(s) just for that). Of course, I've tested my method and it works (and I suppose with no loss in performance), but I wonder if this is questionable practice. What if the if-else-if part is used more than once in the function?

And if the common code for all derived classes is relavitively SHORT, then it is better to use virtual functions then, right?

Answer 1

Why not do this:

void BaseClass::functionName () {
    // some LONG code true for all derived classes of BaseClass
    // ...

    this->some_protected_virtual_member_function();

    // some LONG code true for all derived classes of BaseClass
    // ...
 }

So the common part is not duplicated and the behavior can still easily have extensions in your children classes without having to add another if to your parent class

Answer 2

Your code will not work at all unless the classes have virtual functions. C++ provides only limited reflection: typeid(DerivedClass1)==typeid(DerivedClass2) if there are no virtual functions. The above code also may be slower than simply accessing a virtual function: you'll get a new branch for each type rather than a constant time pointer lookup.

However, the biggest issue with the above code is that it looses polymorphism and encapsulation. The using code must be aware or what DerivedClass1 and DerivedClass2 need to do. It needs to be aware of the structures inside DerivedClass1 and DerivedClass2. Also, all the code is piled into one place, making this function possibly hundreds of lines.

Answer 3

I think you're looking for the template method pattern here: Just use your existing non-virtual function and have it call a virtual function only for the small section of code that differes between concrete classes. It has the advantage of looking prettier too.

void BaseClass::functionName () {
    // some LONG code true for all derived classes of BaseClass
    // ...

    functionName_impl();  // Will be virtual (private or protected) and overriden in each child class to do the right work.

    // some LONG code true for all derived classes of BaseClass
    // ...
 }

Answer 4

This is a degenerate case of the Template Method Pattern:

class Base {
public:
    void templated() {
        // do some stuff

        this->hook1();

        // other stuff

        if (/*cond*/) { this->hook2(); }

        size_t acc = 0;
        for (Stuff const& s: /*...*/) { acc += this->hook3(s); }

        // other stuff
    }

private:
    virtual void hook1() {}
    virtual void hook2() {}
    virtual size_t hook3(Stuff const&) { return 0; }
}; // class Base

And then a Derived class can customize the behavior of the hooks.

A word of warning: this is extremely rigid, by nature, since the templated method is not virtual; this is a both a virtue and a problem of this pattern, it is good because if you need to change the templated method then it is defined in a single place and it is annoying if the hooks provided are not sufficient to customize the behavior.