Inherit from a template parameter and upcasting back in c++

Question

Hello, I have tried to use this code in VS2008 (and may have included too much context in the sample...):

class Base
{
    public:
    void Prepare() {
       Init();
       CreateSelectStatement();
       // then open a recordset
    }
    void GetNext() { /* retrieve next record */ }
    private:
    virtual void Init() = 0;
    virtual string CreateSelectStatement() const = 0;
};
class A : public Base
{
   public:
   int foo() { return 1; }
   private:
   virtual void Init() { /* init logic */ }
   virtual string CreateSelectStatement() { /* return well formed query */ }
};

template<typename T> class SomeValueReader : protected T
{
   public:
   void Prepare() { T::Prepare(); }
   void GetNext() { T::GetNext(); }
   T& Current() { return *this; } // <<<<<<<< this is where it is interesting
   SomeValue Value() { /* retrieve values from the join tables */ }
   private :
   string CreateSelectStatement() const
   {
   // special left join selection added to T statement
   }
};

void reader_accessAmemberfunctions_unittest(...)
{
   SomeValueReader<A> reader();
   reader.Prepare();
   reader.GetNext();
   A a = reader.Current();
   int fooresult = a.foo();
   // reader.foo()            >> ok, not allowed
   Assert::IsEqual<int>( 1, fooresult );
};

This works as expected, i.e. having access to "A" member functions and fooresult returning 1. However, an exception is thrown when objects are deleted at the end of the unittest function:

System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt

If I change the return type of Current() function to :

T* Current()
{
   T* current = dynamic_cast<T*>(this);
   return current;
}

then everything is ok and the unit test ends with no access violation. Does someone can tell me what was wrong with the first Current() implementation? Thanks, bouchaet.

Answer 1

After changing CreateSelectStatement to return a value for the implemented functions (not the pure virtual)

string CreateSelectStatement() const { return ""; }

and changing the declaration of reader (the declaration you have should strictly be interpreted as a function prototype in C++)

SomeValueReader<A> reader;

The above example compiles and executes without errors using gcc, leading me to believe the actual error may not be present in the source above. Unfortunately I'm unable to test with VC at the moment.

I can't see any obvious reason why the change you have suggested would resolve the issue, the only other error I can see is that Base does not have a virtual destructor declared meaning that if you ever delete a Base* (or some other derived class that is not the actual type) the incorrect destructor(s) will fire. You should declare it as

virtual ~Base() {}

even if it's empty.

Stylistically it's also a little odd to use templates and virtual functions in this fashion because here you're using the template to resolve functions at compile time. I can't see a reason why SomeValueReader needs to derive from T (rather than have a member variable) either.

Answer 2

I don't have access to visual studio but one problem with your code is that CreateSelectStatement() is not declared const in class A. So it has a different signature from the others in Base and SomeValueReader. That is ok as long as you do not try and instantiate an A (i.e., it is a pure virtual class just like Base). But you do instantiate one in reader_accessAmemberfunctions_unittest. I would have expected your compiler to generate an error for this.... g++ 4.4.1 does. Possibly this is not your issue; it is hard to tell because your example code contains several other bugs. You should try and keep examples truly as simple as possible while still being compilable (they should contain the header files you include for example). Your code contains superfluous statements as well as uncompilable pseudo code which makes the job of debugging it more difficult. You will learn a lot from reducing your code to its simplest form and often times you will be subsequently able to solve your issue without resorting to asking for assistance. It is a basic step in debugging your code.

Answer 3

Ok, my bad I have not try to compile the code from VS. I was simply typing it and deliberately omit some details. I must say that it was a very bad idea to not test the sample directly and only rely on a behaviour I was seeing in a real project. So a compilable version would be:

/* /clr option enabled */

class Base
{
public:
   void FuncA() {}
protected :
   Base() {}
};

class Derived : public Base
{
public:
   int foo() { return 1; }
};

template<typename T> class SomeValueReader : protected T
{
public:
   void FuncA() { T::FuncA(); }
   T& Current() { return *this; }
};


void main(char* args)
{
   SomeValueReader<Derived> reader;
   reader.FuncA();
   Derived derived;
   derived = reader.Current();
   int fooresult = derived.foo();
   //reader.foo()            >> ok, not allowed
};

Now, I have to say that I cannot make this sample to produce an access violation. So it is irrelevant. Nevertheless, the modification that I had proposed was the only wordaround I have found to my problem in my real project and I was wondering why.

Item 34: Prefer composition to inheritance
I am well aware of this general guideline and I did wish to define my SomeValueReader as a composition. However, the SomeValueReader do need to access protected functions and members of T to be able to adjust itself to T. To have only a member T would not give enough information to SomeValueReader to perform its responsability. Moreover, SomeValueReader do exploit the Base public non-virtual interface by implementing a set of private virtual functions. Otherwise, it would have to duplicate some code or logic. So, I end up with these options, I either :

• declare SomeValueReader a friend of Base, duplicate some logic and access T protected members;
• promote Base protected methods to public (and expose way too much information to all);
• or try this curious protected inheritance (but does add complexity).

I may have missed another option. Since I could not resolve myself to "cheat" with a friend class, I have decided to go with this template polymorphism. But I am open to suggestions.

The missing const and destructor
The const was an error due to inattention (and to not try to compile the code).
The missing destructor was an omission as I did not see it as a important detail. But few people do think that could have been error. In fact, this is what I would though either. A memory corruption leads us to reach for the error inside the destructors. But in the real project, the Base destructor is in fact public and virtual. Or could have been protected and non-virtual in this example since Base* was not used.