Is this some best practice?

M

Morgan Cheng

Once upon a time, I feel upset about private data member in class header
files, because it expose private data structure of the class. What's
more, if I change some non-static private data member, component using
my class has to be re-compiled.

Recently, I saw some code written by others. The code encapsulate all
private data member into a struct. So, in header file, only forward
declaraction of the struct is needed.
struct FooPrivate;
class Foo
{
public:
...
private:
FooPrivate fooData;
};

In the cpp file of the class, the struct is defined. So, the
implementation of Foo is totally invisible to users. And, change of
private data structure will not affect others.

I think this idea is perfect. What do you think?
 
S

Siemel Naran

Morgan Cheng said:
Recently, I saw some code written by others. The code encapsulate all
private data member into a struct. So, in header file, only forward
declaraction of the struct is needed.
In the cpp file of the class, the struct is defined. So, the
implementation of Foo is totally invisible to users. And, change of
private data structure will not affect others.

I think this idea is perfect. What do you think?

It is perfect.
 
A

Alf P. Steinbach

* Morgan Cheng:
Once upon a time, I feel upset about private data member in class header
files, because it expose private data structure of the class. What's
more, if I change some non-static private data member, component using
my class has to be re-compiled.

Recently, I saw some code written by others. The code encapsulate all
private data member into a struct. So, in header file, only forward
declaraction of the struct is needed.
struct FooPrivate;
class Foo
{
public:
...
private:
FooPrivate fooData;
};

In the cpp file of the class, the struct is defined. So, the
implementation of Foo is totally invisible to users. And, change of
private data structure will not affect others.

I think this idea is perfect. What do you think?

I may be missing something, but I think it's more likely that you've
missed a little "*" in there: as described the could shouldn't compile.

With that pointer it is the PIMPL idiom.

Google for pimpl; it is a trade-off used in particular to encapsulate
usage of "bad" header files.
 
R

Rolf Magnus

Alf said:
* Morgan Cheng:

I may be missing something, but I think it's more likely that you've
missed a little "*" in there: as described the could shouldn't compile.

With that pointer it is the PIMPL idiom.

Yes, and it requires the class to 'new' the private struct in the
constructor and delete it in the destructor. Also, the member access needs
an additional dereference.
So it will induce some memory and runtime overhead.
Google for pimpl; it is a trade-off used in particular to encapsulate
usage of "bad" header files.

And to avoid recompilation of other components after adding/removing
members. This can be important in interface classes of a library that needs
to stay binary compatible between versions. Note however that there are
other issues with binary compatibility. A good start is:

http://developer.kde.org/documentation/library/kdeqt/kde3arch/devel-binarycompatibility.html
 
E

E. Robert Tisdale

Morgan said:
Once upon a time, I [felt] upset
about private data member in class header files
because it expose private data structure of the class.
What's more, if I change some non-static private data member,
component using my class [must] be re-compiled.

Recently, I saw some code written by others.
The code encapsulate all private data member into a struct.
So, in header file, only forward declaraction of the struct is needed.

struct FooPrivate;
class Foo {
public:
...
private:
FooPrivate fooData;


Should be:

FooPrivate* fooData;
};

In the cpp file of the class, the struct is defined.
So, the implementation of Foo is totally invisible to users.
And, change of private data structure will not affect others.

I think this idea is perfect. What do you think?

Data hiding is *not* about encryption.
It's about preventing application programmers
from *accidently* accessing private data objects directly.

I used Google

http://www.google.com/

to search for

+"PIMPL idiom" +"C++"

and I found lots of stuff.

This is *not* a perfect idea.
It precludes optimizations such as
inlining functions that access private data members.
It's useful only in the implementation of dynamically linked libraries
when it isn't practical to recompile calling programs.
 
A

Alf P. Steinbach

* red floyd:
In the GoF book, it's called the "Bridge Pattern".

I had to look that up. It's not common terminology in C++, and it's _not_
an identity. Rather Pimpl and Bridge are two elaborations of a common idea.

In particular, in C++ Pimpl there's (usually) no interface defined for the
implementation.

In the days when I thought patterns were cool very few else seemed to think
so. Now just about anybody can rattle off pattern names (I don't remember
them in general) and, without understanding anything at all, identify some
problem or technique as a particular pattern. I think they're idiots.

Here's the Boost discussion of Pimpl:
<url: http://www.boost.org/libs/smart_ptr/sp_techniques.html#pimpl>

(unfortunately the code is incorrect or incomplete).

Here's the first GotW:
<url: http://www.gotw.ca/gotw/024.htm>

Here's the second GotW:
<url: http://www.gotw.ca/gotw/028.htm>
 
S

Siemel Naran

Yes, and it requires the class to 'new' the private struct in the
constructor and delete it in the destructor. Also, the member access needs
an additional dereference.
So it will induce some memory and runtime overhead.

Good points on the drawbacks on PIMPL. Also, one needs to take care of
operator= and the copy constructor, which may have to make a deep copy of
the object.
http://developer.kde.org/documentation/library/kdeqt/kde3arch/devel-binaryco
mpatibility.html

Nice link!
 
S

Siemel Naran

http://developer.kde.org/documentation/library/kdeqt/kde3arch/devel-binaryco
mpatibility.html

<Quote>
[To preserve binary compatibility you can] reimplement virtual
functions defined in one of the base classes if it is safe that
programs linked with the prior version of the library call the
implementation in the base class rather than the new one. This is
tricky and might be dangerous. Think twice before doing it.
</Quote>

What does it mean?


<Quote>
[To preserve binary compatibility you can] change an inline function or
make an inline function non-inline if it is safe that programs linked
with the prior version of the library call the old implementation. This
is tricky and might be dangerous. Think twice before doing it. Because
of this, classes that are supposed to stay binary compatible should
always have non-inline destructor, even if it's empty.
</Quote>

What does it mean?


<Quote>
[To preserve binary compatibility you cannot] change the access rights
to some functions or data members, for example from private to public.
With some compilers, this information may be part of the signature. If
you need to make a private function protected or even public, you have
to add a new function that calls the private one. Note: for KDE code it
is accepted to make methods (not data members) more accessible (i.e.
changing private->protected->public). The only compiler affected by
this, that we know of, is MSVC++, and KDE doesn't compile on Windows
anyway.
</Quote>

I don't think this is correct: you should never be able to change the
access level of one member data. The C++ standard allows that members
from different blocks of access levels can be re-arranged differently.
For example,

class S {
private:
int i;
char c;
protected:
float f;
};

The compiler could arrange the members as { i, b, d } or { d, i, b }.
Most arrange as { i, c, f }, in order of declaration.

Now suppose we want to make 'c' public.

class S {
private:
int i;
public:
char c;
protected:
float f;
};

The compiler could arrange the members as { i, b, d } or { d, i, b } as
before, but also as { c, i, f } or { c, f, i } or any of the other
permutations. Most arrange as { i, c, f } still, in order of
declaration.

From this it follows that changing the access of level of one data
member is unsafe for preserving binary compatibility, but in today's
compilers it should be OK anyway.
 
O

Owen Jacobson


<Quote>
[To preserve binary compatibility you can] change an inline function or
make an inline function non-inline if it is safe that programs linked
with the prior version of the library call the old implementation. This
is tricky and might be dangerous. Think twice before doing it. Because
of this, classes that are supposed to stay binary compatible should
always have non-inline destructor, even if it's empty. </Quote>

What does it mean?

It means that code compiled against the old version of the function will
still use that old code, even if the library version has newer code,
because there's no function call in the generated binary. The old
function has been placed directly in the generated binary.

Therefore, if you rewrite an inline function, assume that the original
implementation still exists somewhere and ensure that it still functions
correctly.
 
R

Rolf Magnus

Siemel said:
http://developer.kde.org/documentation/library/kdeqt/kde3arch/devel-binaryco
mpatibility.html

<Quote>
[To preserve binary compatibility you can] reimplement virtual
functions defined in one of the base classes if it is safe that
programs linked with the prior version of the library call the
implementation in the base class rather than the new one. This is
tricky and might be dangerous. Think twice before doing it.
</Quote>

What does it mean?

If you have a base class and a class derived from it in the library, and in
the new version of the library, you decide to override one of the virtual
functions of the base class, then a program linked to the old version of
the library will not call that new implementation from the derived class,
but the one from the base class.
<Quote>
[To preserve binary compatibility you can] change an inline function or
make an inline function non-inline if it is safe that programs linked
with the prior version of the library call the old implementation. This
is tricky and might be dangerous. Think twice before doing it. Because
of this, classes that are supposed to stay binary compatible should
always have non-inline destructor, even if it's empty.
</Quote>

What does it mean?

The problem about inline functions in public interfaces of libraries is that
- when inlined - the code gets copied into the main program, so if you - as
developer of the library - now decide to change that function in the new
library version, the main program will still use the old function. If
that's ok, then you can do so. But it's best to avoid inline functions in
the public interface. In the case of the destructor, it means that the
compiler-generated one is likely to get inlined automatically, so you have
to define an empty non-inline one to make sure it can later be changed if
needed.
<Quote>
[To preserve binary compatibility you cannot] change the access rights
to some functions or data members, for example from private to public.
With some compilers, this information may be part of the signature. If
you need to make a private function protected or even public, you have
to add a new function that calls the private one. Note: for KDE code it
is accepted to make methods (not data members) more accessible (i.e.
changing private->protected->public). The only compiler affected by
this, that we know of, is MSVC++, and KDE doesn't compile on Windows
anyway.
</Quote>

I don't think this is correct: you should never be able to change the
access level of one member data. The C++ standard allows that members
from different blocks of access levels can be re-arranged differently.

That's what the above quote says, isn't it?
For example,

class S {
private:
int i;
char c;
protected:
float f;
};

The compiler could arrange the members as { i, b, d } or { d, i, b }.
Right.

Most arrange as { i, c, f }, in order of declaration.

Now suppose we want to make 'c' public.

class S {
private:
int i;
public:
char c;
protected:
float f;
};

The compiler could arrange the members as { i, b, d } or { d, i, b } as
before, but also as { c, i, f } or { c, f, i } or any of the other
permutations. Most arrange as { i, c, f } still, in order of
declaration.

From this it follows that changing the access of level of one data
member is unsafe for preserving binary compatibility, but in today's
compilers it should be OK anyway.

Well, the quote says that it's unsafe. It just doesn't say that it should be
OK on most compilers anyway.
 
O

Old Wolf

Rolf said:
Morgan said:
[description of PIMPL]
I think this idea is perfect. What do you think?

Yes, and it requires the class to 'new' the private struct
in the constructor and delete it in the destructor. Also,
the member access needs an additional dereference.

So it will induce some memory and runtime overhead.

If a compiler was clever, could it optimise out the runtime
overhead?

One idea I haven't seen mentioned much: is it feasible to
make the 'pimpl' a reference rather than a pointer? Then
you can't forget to initialize it in a constructor. It might
make it easier for the compiler to optimise. A drawback is
that you have to use ugly syntax in the destructor.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,764
Messages
2,569,565
Members
45,041
Latest member
RomeoFarnh

Latest Threads

Top