Overriding a member variable in C++

Question

I have run into a bit of a tricky problem in some C++ code, which is most easily described using code. I have classes that are something like:

class MyVarBase
{
}

class MyVar : public MyVarBase
{
    int Foo();
}

class MyBase
{
public: 
    MyBase(MyVarBase* v) : m_var(v) {}
    virtual MyVarBase* GetVar() { return m_var; }
private:
    MyVarBase* m_var;
}

I also have a subclass of MyBase that needs to have a member of type MyVar because it needs to call Foo. Moving the Foo function into MyVarBase is not an option. Does it make sense to do this:

class MyClass : public MyBase
{
public:
    MyClass(MyVar* v) : MyBase(v), m_var(v) {}
    MyVar* GetVar() { return m_var; }
private:
    MyVar* m_var;
}

This seems to work but looks really bad and I'm not sure if it's going to cause a memory leak or break a copy constructor. My other options might be to name the MyVar variable in MyClass something else but have it be equal to the m_var pointer in the base, or to templatise MyBase on the MyVar type.

All these options don't seem ideal so I wanted to know if anyone else has run into a situation like this and if there is a good way to make it work.

Answer 1

The correct way to do this is to have the variable only in the base class. As the derived class knows it must be of dynamic type MyVar, this is totally reasonable:

class MyClass : public MyBase
{
public:
    MyClass(MyVar* v) : MyBase(v) {}
    MyVar* GetVar() { return static_cast<MyVar*>(MyBase::GetVar()); }
}

Since MyVar is derived from MyVarBase, the different return-types of GetVar would still work if GetVar was virtual (as is the case here). Note that with that method, there must be no function in MyBase that can reset the pointer to something different, obviously.

Note that static_cast is the right cast in that case. Using dynamic_cast, as proposed by one commenter, will tell the readers and users of GetVar that MyBase::GetVar() could return a pointer to an object not of type MyVar. But that doesn't reflect our intention, as you only ever pass MyVar. To be consequent is the most important thing in software development. What you could do is to assert it is non-null. It will abort at runtime with an error-message in debug-builds of your project:

MyVar* GetVar() { 
    assert(dynamic_cast<MyVar*>(MyBase::GetVar()) != 0);
    return static_cast<MyVar*>(MyBase::GetVar()); 
}

Answer 2

Without knowing more of the context, it's hard to say for sure, but I'd reconsider whether you need this class hierarchy in the first place. Do you really need the MyVarBase and MyBase classes? Could you get away with composition instead of inheritance? Maybe you don't need the class heirarchy at all, if you template the functions and classes that work with the above classes. Maybe CRTP can help too. There are plenty of alternatives to using virtual functions (and to some extent, inheritance in general) in C++.

Anyway, your own suggested solution certainly shouldn't leak any memory (since it doesn't allocate any), and wouldn't break the default copy constructor either (since it just performs a member-wise copy. Now you just have an extra member in the derived class, but that gets copied too). Of course, you get the additional problem of keeping the two variables in sync, so I still don't like that solution much.

Alternatively, you may be able to remove the variable from the base class. Make the GetVar() function pure virtual, so it is only implemented in derived classes, which can define the member variable in any way they like.