0

Yesterday, I wrote some code and i would really appreciate a judgement if this is good or bad practice. And if its bad, what could go wrong.

The construct is as followed:

The base class A comes from an API sadly as a template. The goal was to be able to put derived classes from A in a std::vector. I achieved this with the base class B from which all derived classes that inherit from A will inherit. Without the pure-virtual function in B the foo() function from A was called.

With the pure-virtual function in B the foo() version from C get called. This is exactly what I want.

So is this bad practice?

EDIT:

To clear some misunderstandings in my example code out:

template <class T> 
class A 
{
public:
    /* ctor & dtor & ... */
    T& getDerived() { return *static_cast<T*>(this); }
    void bar() 
    {
        getDerived().foo(); // say whatever derived class T will say
        foo(); // say chicken
    }
    void foo() { std::cout << "and chicken" << std::endl; }
};

class B : public A<B>
{
public:
    /* ctor & dtor */
    virtual void foo() = 0; // pure-virtual 
};

class C : public B
{
public:
    /* ctor & dtor */
    virtual void foo() { std::cout << "cow"; }
};

class D : public B
{
public:
    /* ctor & dtor */
    virtual void foo() { std::cout << "bull"; }
};

The std::vector shall contain both, C as well as D and

void main()
{
std::vector<B*> _cows;
_cows.push_back((B*)new C()));
_cows.push_back((B*)new D()));

for(std::vector<B*>::size_type i = 0; i != _cows.size(); ++i)
   _cows[i].bar();

}

with output

cow and chicken
bull and chicken

is desired.

As far as I know is there no other way to store classes derived from a template class in a container than the use of a base class. If I use a base class for my derived classes, e.g., B i must cast every instance within the vector back to its proper class. But at the time bar() is called, I don't know the exact type.

Michael Haidl
  • 5,384
  • 25
  • 43
  • 3
    No virtual destructor in base classes. – Mark Garcia Jun 11 '13 at 07:10
  • you've hidden the void A::foo() in B. In A it's not virtual – spiritwolfform Jun 11 '13 at 07:12
  • @spiritwolfform ok, but is this a bad practice? can something go terrible wrong with that? and yes no virtual destructors as this is an outtake and not the original code. – Michael Haidl Jun 11 '13 at 07:15
  • this means you have two methods now, so you can do `_cows[0].foo()` to print `cow` and `_cows[0].A::foo()` to print `chicken`, these are two independent methods now – spiritwolfform Jun 11 '13 at 07:19
  • @kronos: Why have `virtual` functions when you're using CRTP which is mainly for static polymorphism? http://stackoverflow.com/questions/262254/crtp-to-avoid-dynamic-polymorphism – legends2k Jun 11 '13 at 07:25
  • @legends2k thanks for the link. now i know at least what the visitor pattern (in my example class A) is made of. The real A uses booth implementations of foo(). However, without the virtual functions (and without B) derived classes from A wont go into one single container. – Michael Haidl Jun 11 '13 at 07:48

1 Answers1

0

I call it bad practice for doing possibly confusing and obfuscating things.

1) A function name should in all base and sub-classes either be virtual or non-virtual.
mixing overriding and overloading within the inheritance hierachry is a very bad idea.
No one really expects something like that.
Thus if A::foo should be virtual as well, or B and C foo should not.

2) Preferably base classes should be interfaces.
As such A:foo should be pure virtual and not B::foo.
2b) If a base class has already provided a default implementation don't declare the function pure virtual in a sub-class.

tlvs
  • 2,835
  • 2
  • 13
  • 7