Is it a code smell to declare a function to be virtual in a derived class?

Question

In this code, I have defined three structs, base, derived and MoreDerived each inheriting from the previous member in the list.

However, I have defined the function f() to be virtual in the derived struct, NOT in the base struct.

#include <iostream>
struct base {
    void f() {
       std::cout << "base\n";
   }
};
struct derived : base {
    virtual void f()  { 
        std::cout << "derived\n";
    }
};

struct morederived : derived {
     void f()  { 
        std::cout << "more derived\n";
    }
};

int main()
{
    derived d;
    morederived md;

    base* dp = &d; 
    base* mdp = &md; 
    dp->f(); 
    mdp->f(); 

}

The code above prints base \n base as expected. And if I declare dp and mdp to be pointers of type derived*, the code prints derived \n more derived as expected again, since f is defined to be virtual in the derived.

Of course, this code is small and so the results are predictable. But in large code bases, is doing the above ever a good idea?

Note: I have not seen this in any codes yet, in my limited c++ experience, nor do I have any plans to use such a pattern (or anti-pattern :-D). I am asking this purely out of curiosity.

EDIT: I don't feel this question is not a duplicate to the one pointed out. I am not asking if the virtual keyword is necessary in a derived class. I am asking the effects/plusses/minuses of placing the virtual keyword on a function f() that has an implementation in the base class, but is declared virtual only in the derived class, and then is inherited by other classes. Specifically I am asking if this thing is a patter or anti-pattern.

Answer 1

In my book, it qualifies as a code smell.

derived::f() actually hides base::f() rather than overriding it.

This means that base_ptr->f() always calls base::f(), even if base_ptr points at an instance of a derived class, whereas derived_ptr->f() will use virtual function dispatch.

The fact of making derived::f() virtual and documenting that it can be overridden, encourages implementers of classes like morederived to expect that a call of the form pointer->f(), if pointer points at an instance of morederived, to believe their version will be called. In reality, which version will be called depends on the type of pointer (e.g. different version called if it is base * or derived *).

That can result in quite an unpleasant surprise for anyone who uses the classes, and doesn't closely inspect all classes in the hierarchy. In large class hierarchies, particularly if provided by someone else, developers will not typically inspect a complete class hierarchy provided by someone else that closely. The will assume it works as advertised, if they care at all about their productivity.

If this sort of thing was present in a commercial class library, the vendor could expect to receive bug reports from customers bitten by such a bug, to lose customers of their library (on the premise that they introduced a bug through bad development technique), or both.

Answer 2

Yes, this breaks the Is-a contract. Your derived is not playing by the same rules as base.

This is never a good idea. You might want to consider composition for derived. I.e.

struct derived {
    base b; // for whatever use you might have for base.
    virtual void f()  { 
        std::cout << "derived\n";
    }
};