keep function definitions with variable declartions

[derikd]

I come from a background of C and am transitioning a project to C++. But I have also worked in Java for some time. I am having a problem understanding good usage of header files in C++, because it seems to be different than good usage in C.

To make it concrete, let's say I have a widget and the code for it in the C programming language is like this

The header file for this would be something like

// widget.h
void f1(void);
int f2(void);



and the code file might look like the following.

// widget.c
#include widget.h
static int var1, var2 var3;

void f1(void){
// do stuff
}

int f2(void){
// do stuff and return result
}

The variables are declared static so that there scope is explicitly restricted to file scope. They are not global. They are an implementation detail.


In C, I can now put a reference to widget.h in all my other files that need access to f1 and f2. I can change implementation off1 and f2 and I can do whatever I want with var1, var2, and var3 and I do not need to recompile the files that reference widget.h


But now I am converting to C++ and I want to make Widget into a class. My plan was to add the class wrapper around the contents of widget.h header file (maybe throw in the public: keyword). Then I would change function declarations in widget.c to associate them with that class with the :: operator.

header in C++


// widget.h
class widget {
public:
void f1(void);
int f2(void);
};


And the code file looks like this


// widget.cpp

#include widget.h
int widget::var1, widget::var2 widget::var3; // EDIT : deleted 'static' keyword

void widget::f1(void){
// do stuff
}

int widget::f2(void){
// do stuff and return result
}



okay now the problem is that according to the definition in widget.h, the widget class has no member called var1, var2 and var3. That is, there is no reference to them.

Now I could add them as private members to the widget.h file and then I must remove them from the widget.cpp file.

Now this creates several problems for me.
1. if I change the type (or even just the name) of the "private" variables, I must also change the header file, widget.h, and re-compile all the functions that use widget.h (which is most of a fairly large project). Okay that is a bummer but I can live with it.

2. even if I declare the variables private, I have exported (at least visually) all my private variables to the world via the header file, which is what causes problem #1 I guess so maybe that is the same thing.

3. I must now initialize all the variables in a constructor function rather than initialize them in the declaration as I would have done in C (or Java). To me initialization in the declaration seems cleaner than the "declaring in one place and initializing in different place (indeed a an entirely different file). Am I missing something here? but when I try to initialize in the header file my compiler complains. I don't mind creating a constructor for the widget class, but it just seems weird. Its annoying but not a deal breaker.

4. Finally, and this is the really big problem for me, when I am working on the functions of the widget class, I don't have easy access to the variable declarations. That is, if I want to know if var1 is unsigned char or a signed char I need to open up another file to find out. Now I am getting pressure to switch to Hungarian notation on all my variables (not just the member variables). I am in for a world of refactoring in the process of bringing this project over to C++, and the resulting code files will no longer be "cut and past" compatible with the older (but still active) C version of the code tree.

Is there any way to declare in the header file that a variable "var1" will be declared/defined, but actually declare it in the same file as the function definitions?

[ne555]

scope is explicitly restricted to file scope. They are not global.

I would consider them global.
You don't seem to realize that you can have several objects of the same class.

if I change the type (or even just the name) of the "private" variables, I must also change the header file

Check out pimpl

[derikd]

ooops,
ne555 is right. If I left static keyword in the declaration of the variables in the the C++ version, then they would be accessible by more than one instance of the class. In C, declaring them static has the effect of restricting scope of the variables to file scope. But the point is moot because the entire line is invalid. you can't delcare the variables of a class outside of the class definition (in the header file). Which is the whole point of this thread basically. I want to declare my PRIVATE member variables in something other than the header file - preferably the file where the functions are defined.


If I understand the pimpl pattern correctly it would help but does nothing at all for item #4, which is the big problem. My understanding might be limitied. The examples I was finding on-line were either so simplistic that they didn't include private variables (only private functions) or they were so complex that they felt they needed to explain operator overloading for explicit(?) contructors.

[ne555]

You are overreacting, but point 4 is also taken into account

//the header that you include
class foo_impl; //forward declaration
class foo{
private:
   smart<foo_impl> pimpl; //auto, copy, share, etc...
public:
   //the interface
   int bar();
};

//the source (all in the same file)
//The pimpl
class foo_impl{
private:
   /* the ever changing members */
   int value;
public:
   //the interface
   foo_impl();
   int bar();
};

//the implementation
foo_impl::foo_impl(): value(42){}

int foo_impl::bar(){
   return value++;
}

//the forwarding
int foo::bar(){
   return pimpl->bar();
}

Put the `foo_impl' definition in the source because there is nobody that need to see it.
take a look at http://c2.com/cgi/wiki?PimplIdiom

In C, declaring them static has the effect of restricting scope of the variables to file scope.

that applies to C++ too.
But the point is that in that way you could only hold one instance at a time (by instance you could only open one file)

[derikd]

You are overreacting

I didn't mean to. Sorry. But I thank you for your patience for sticking with me even more. I think we we are getting somewhere!

But there are a few things that still don't click. First, as far as I can tell there is nothing linking any particular instance of a foo to any particular instantiation of the fooImpl class. Is that correct?

The foo class has no constructor. So I assume that the compiler makes some kind of null constructor. So how does pimpl get initialized to point towards a real instance of fooImpl?

Second, in your example you used this code:
foo_impl::foo_impl(): value(42){}

I am pretty sure this is not pertinent to the pimpl idiom but could you explain that to me? I know that the first part (up to the single colon) specifies that you are defining the constructor. But the second part, the bold part, what does that mean? Is it simply shorthand for "value = 42", and if so, shouldn't it be inside the curly brackets?

Thanks again in advance,


EDIT: Most of the descriptions I have read have described pimpl as meaning "pointer to implementation" for example: http://en.wikibooks.org/wiki/C%2B%2B_Programming/Idioms#Pointer_To_Implementation_.28pImpl.29

But maybe "smart<T>" is really not a pointer to something but rather an actual instance of pimple. In which case the "Private implementation" which I have also seen, would be a better description. Are there two kinds of pimpl?


In C++ is it okay to actually instantiate a class with just a forward reference? So for example would the following be legal?

class widget;
widget myWidget;

#include widget.h

myWidget.f1();

[Stewbond]

First, Each time someone makes a new foo object, they are making a fooImpl object inside. There is exactley 1 fooImpl per foo object.

Second, foo has a default constructor which is created implicitly. All it does is initialize all member objects with their default constructors.

foo_impl::foo_impl(): value(42){}is using an initializer list. In this case it is equivalent to
foo_impl::foo_impl(): { value = 42; }
There is no advantage for doing that with just an int member, but if you have a member with several constructors, you can specify which constructor that member will be created with and specify parameters for it. Once we've entered the curly brackets, all member objects have been initialized and cannot be constructed again (unless they are pointers).

Finally, In the code above, you have a problem: You cannot make objects based on forward declarations. The size is not known by the compiler at that time. You can only make pointers to objects. However to remove the smart<T> above, you can just do this:

//widget.h
class foo_impl;
class foo
{
private: 
  foo_impl* pimpl; // It's a pointer to the fwd declaration
public:
  int bar();
};

//the source (all in the same file)
//The pimpl
class foo_impl{
private:
   /* the ever changing members */
   int value;
public:
   //the interface
   foo_impl();
   int bar();
};

//the implementation
foo_impl::foo_impl(): { value = 42; }

int foo_impl::bar(){
   return value++;
}

foo::foo() { pimpl = new foo_impl(); } // setting pimple to a new object in the constructor.

//the forwarding
int foo::bar(){
   return pimpl->bar();
}[code]// widget.cpp

[ne555]

smart<foo_impl> is a `pointer'
The idea was simply to say:
_ In the constructor, the pointer will point to a valid instance
_ In the destructor, the memory will be freed
_ The copy constructor and assignment operator work or do not exist

[derikd]

EDIT: I solved this problem see next postWhen I use this code, I get errors at line 20 and line 25.


At line 20 it reports: error #70: incomplete type is not allowed
At line 25 it reports: error #393: pointer to incomplete class type is not allowed

This would seem to indicate that I am missing some key part of the class foo_impl such that the compiler thinks it is not fully defined. But since I get no other errors...

if I just comment out the code in the constructor, and change foo::bar into "return 1;" then the program will compile just fine. It won't work of course because there is now no instance of foo_impl associated with foo.

[derikd]

I corrected the problems above by moving the forward declaration up above the class definition. Hitherto, I had been placing the forward definition in the line immediately above the declaration of pimpl. That seemed like a logical place for it, and I have seen that pattern used many many times. See for instance this wikibooks page.
http://en.wikibooks.org/wiki/C%2B%2B_Programming/Idioms#Pointer_To_Implementation_.28pImpl.29

But I realize now that this was creating a reference to a class called foo_impl that was a part of foo. That is to say pimpl was of type foo::foo_impl * and not merely foo_impl *.

[derikd]

I was able to apply the pimpl idiom to a non-trivial class last night. This is a near perfect solution to all my problems.

Thank you NE555 !


I am marking this solved.