PIMPL with subclasses

[daviddoria]

I am trying to remove the need to #include stl elements in my header (a requirement for a project I work on). I wrote a "wrapper" for std::vector called "MyVector". I declare a pointer to a MyVector as a member of MyClass. Then when I try to use this in MyDerivedClass, I get

"invalid use of incomplete type 'struct MyVector'
forward declaration of 'struct MyVector'

Can anyone see what I have done wrong below?

Test.cxx

#include "MyDerivedClass.h"

int main(int, char *[])
{
  MyDerivedClass a;
  a.DoSomething();
  
  return 0;
}

MyClass.h

#ifndef myclass_h
#define myclass_h

struct MyVector; //forward declaration of 'struct MyVector'

class MyClass
{
  public:
    MyClass();
    ~MyClass();

    MyVector* vecPtr;
};

#endif

MyClass.cxx

#include "MyClass.h"
#include <vector>

struct MyVector
{
  std::vector<double> v;
};

MyClass::MyClass()
{
  this->vecPtr = new MyVector;
}

MyClass::~MyClass()
{
  delete this->vecPtr;
}

MyDerivedClass.h

#ifndef myderivedclass_h
#define myderivedclass_h

#include "MyClass.h"

class MyDerivedClass : MyClass
{
  public:
  void DoSomething();
};

#endif

MyDerivedClass.cxx

#include "MyDerivedClass.h"

void MyDerivedClass::DoSomething()
{
  this->vecPtr->v.push_back(1); //"invalid use of incomplete type 'struct MyVector'
}

Thanks,

David

[Ancient Dragon]

you have to include <vector> in MyDerivedClass.cxx because its not included in any of the header files you created.

[daviddoria]

Hm I had tried that (and just tried it again) and it didn't help :(

[mike_2000_17]

I think, you have to make the pointer a private or protected member, because the compiler will say it is incomplete because it should be accessible to an outside part (like main()) which has no idea what the real declaration of myVector is. Since the VecPtr can be dereferenced in the main() function then main() needs to know its type. By making it private or protected you only require that myClass or myClass and all its derived classes know of the complete declaration of myVector, respectively. May I suggest you make the class myVector as nested in myClass (by forward-declaring in either the private or protected part), there will be less chance of name clashes that way.

[daviddoria]

Those are good ideas/suggestions, but it still doesn't work!

Here are the files that changed:

MyClass.h

#ifndef myclass_h
#define myclass_h

class MyClass
{
  public:
    MyClass();
    ~MyClass();

  protected:
    struct MyVector;
    MyVector* vecPtr;
};

#endif

MyClass.cxx

#include "MyClass.h"

#include <vector>

struct MyClass::MyVector
{
  std::vector<double> v;
};

MyClass::MyClass()
{
  this->vecPtr = new MyVector;
}

MyClass::~MyClass()
{
  delete this->vecPtr;
}

The errors are actually identical.

David

[Ancient Dragon]

Since you put the declaration of MyVector in MyClass.cxx it is not known to MyDerivedClass.cxx, therefore causing the error you got.

Move the declaration of MyVector from MyClass.cxx into the header file MyClass.h and include <vector> in that file too.

[daviddoria]

That is the whole problem - I am not allowed to #include <vector> in any header file.

[Ancient Dragon]

Then include it in each of the *.cpp or *.cxx files. And put it before myclass.h so that vector has been defined.

#include <vector>
#include "MyDerivedClass.h"
// rest of code here

[mike_2000_17]

You still have the problem that myDerivedClass is not aware of the struct myVector (at least not its full declaration). I would suggest:

//in MyClass.h

#ifndef myclass_h
#define myclass_h

class MyClass
{
  public:
    MyClass();
    ~MyClass();

  protected:
    struct MyVector;
    MyVector* vecPtr;
};

#endif

//in MyClass_details.h
#include <vector>

struct MyClass::MyVector
{
  std::vector<double> v;
};


//in MyClass.cxx
#include "MyClass.h"

#include "MyClass_details.h"

MyClass::MyClass()
{
  this->vecPtr = new MyVector;
}

MyClass::~MyClass()
{
  delete this->vecPtr;
}

//in MyDerivedClass.h
#ifndef myderivedclass_h
#define myderivedclass_h

#include "MyClass.h"

class MyDerivedClass : MyClass
{
  public:
  void DoSomething();
};

#endif

//in MyDerivedClass.cxx

#include "MyDerivedClass.h"
#include "MyClass_details.h"

void MyDerivedClass::DoSomething()
{
  this->vecPtr->v.push_back(1); //"invalid use of incomplete type 'struct MyVector'
}

[Ancient Dragon]

That will work too except that <vector> must be include in the *.cpp/*.cxx files before any of the other header files.

[daviddoria]

@Mike - I can't add another header like that :(

@AncientDragon - That didn't work either - same errors:

MyClass.h

#ifndef myclass_h
#define myclass_h

class MyClass
{
  public:
    MyClass();
    ~MyClass();

  protected:
    struct MyVector;
    MyVector* vecPtr;
};

#endif

MyClass.cxx

#include <vector>
#include "MyClass.h"

struct MyClass::MyVector
{
  std::vector<double> v;
};

MyClass::MyClass()
{
  this->vecPtr = new MyVector;
}

MyClass::~MyClass()
{
  delete this->vecPtr;
}

MyDerivedClass.h

#ifndef myderivedclass_h
#define myderivedclass_h

#include "MyClass.h"

class MyDerivedClass : public MyClass
{
  public:
  void DoSomething();
};

#endif

MyDerivedClass.cxx

#include <vector>
#include "MyClass.h" //I even added this!
#include "MyDerivedClass.h"

void MyDerivedClass::DoSomething()
{
  this->vecPtr->v.push_back(1);
}

[Ancient Dragon]

It compiled perfectly ok for me using vc++ 2010 express. Note that MyVector is in MyClass.h, not in MyClass.cxx. You still have the same error because you failed to move it.

MyDerivedClass.cx

#include <vector>
#include "MyDerivedClass.h"


void MyDerivedClass::DoSomething()
{
  this->vecPtr->v.push_back(1); //"invalid use of incomplete type 'struct MyVector'
}

MyClass.cxx

#include <vector>
#include "MyClass.h"


MyClass::MyClass()
{
  this->vecPtr = new MyVector;
}

MyClass::~MyClass()
{
  delete this->vecPtr;
}

MyClass.h

#ifndef myclass_h
#define myclass_h

struct MyVector
{
  std::vector<double> v;
};


class MyClass
{
  public:
    MyClass();
    ~MyClass();

    MyVector* vecPtr;
};

#endif

[daviddoria]

Right, you have #included <vector> in the header. This is not allowed in the library I am working on.

[mike_2000_17]

"MyClass_details.h" is not a "real" header because it is only included by the cxx files of the derived classes. You can change the extension to .cxx if it makes you happier but it makes no difference. As long as MyClass_details.h is not included in any other libraries, you are not breaking the rules.

@AD: stop talking about putting the myVector class definition in the MyClass.h, that is exactly what david wants to avoid. Of course, doing that solves the compilation error, but it does not solve the problem.

[daviddoria]

If that is really the only way to do it, I'll have to try to fight the fight. However, they really like their structure (there are 2000 classes, so consistency is nice), so they would really like to see a single .h and a single .cxx for each set of functionalities (class).

[vijayan121]

> That is the whole problem - I am not allowed to #include <vector> in any header file

You don't have to #include <vector> in any header file, you just have to declare std::vector<> in the header.

1. Declare std::vector<> in Myclass.h
2. #include <vector> (define std::vector<>) in MyClass.cc and MyDerivedClass.cc
3. provide a protected accessor to the vector for the benefit of derived class authors.

///////// MyClass.h ////////////

namespace std
{
    template < typename T > struct allocator ;
    template < typename T, typename A > class vector ;
}

class MyClass
{
    // ...

    private:
       struct impl ;
       impl* pimpl ; //consider using a std::tr1::unique_ptr instead of a raw pointer 

    protected:
       std::vector< double, std::allocator<double> >& vector() ;
};

///////// MyDerivedClass.h ////////////

#include "MyClass.h"

class MyDerivedClass : MyClass
{
    public:
        void DoSomething();
};

///////// MyClass.cc ////////////

#include "MyClass.h"
#include <vector>

struct MyClass::impl
{
   std::vector<double> v ;
   // ...
   // ...
};

// ...

// ...

std::vector< double, std::allocator<double> >& MyClass::vector()
{ return pimpl->v ; }

///////// MyDerivedClass.cc ////////////

#include "MyDerivedClass.h"
#include <vector>

void MyDerivedClass::DoSomething()
{
    vector().push_back(1.3);
}

[daviddoria]

Ah, I actually saw that solution on StackOverflow - the problem was that in order to use std::allocator , you had to #include <memory> . If you are going to include 'memory', you might as well include 'vector', right?

David

[mike_2000_17]

But std::allocator is also forward-declared in MyClass.h, so you don't need to include <memory> in the header.

[daviddoria]

Wow you're right, that does work! Thanks all!

[daviddoria]

While that did work, this seems even better to me :)

Test.cxx

#include "MyDerivedClass.h"

int main(int, char *[])
{
  MyDerivedClass a;
  a.DoSomething();

  return 0;
}

MyClass.h

#ifndef myclass_h
#define myclass_h

namespace std
{
    template < typename T > struct allocator ;
    template < typename T, typename A > class vector ;
}

class MyClass
{
  public:
    MyClass();
    ~MyClass();

  protected:
    std::vector< double, std::allocator<double> >* MyVector;
};

#endif

MyClass.cxx

#include "MyClass.h"
#include <vector>

MyClass::MyClass()
{
  this->MyVector = new std::vector<double>;
}

MyClass::~MyClass()
{
  if(this->MyVector)
    {
    delete this->MyVector;
    }
}

MyDerivedClass.h

#ifndef myderivedclass_h
#define myderivedclass_h

#include "MyClass.h"

class MyDerivedClass : MyClass
{
    public:
        void DoSomething();
};
#endif

MyDerivedClass.cxx

#include "MyDerivedClass.h"
#include <vector>
#include <iostream>

void MyDerivedClass::DoSomething()
{
    this->MyVector->push_back(1.3);

    std::cout << (*this->MyVector)[0] << std::endl;
}

Thanks again!