Private member should be defined in .h or .cpp?

Question

Style 1:

A.h

#include "MyClass.h"
class A{
    private:
        int myInt;
        float myFloat;
        MyClass myClass;
};

Style 2:

A.h

class A{

};

A.cpp

#include "MyClass.h"
int myInt;
float myFloat;
MyClass myClass;

Style 3:

A.h

class A{
    private:
        int myInt;
        float myFloat;
};

A.cpp

#include "MyClass.h"
MyClass myClass;

I know Style 1 is most simple, but it has at least 1 problem: need #include "MyClass.h" in A.h

Style 2 moved all private members in .cpp, it prevents #include "MyClass.h" exposed to A.h and hides more details from .h, but it has at least a disadvantage: the code is less readable, I cannot know all details about this class if I reads A.h only.

Style 3 seems get the balance, but I believe the code is less straight forward than Style 2 and I believe it is less maintainable.

which style is preferred?

Dunno what you are trying to do, but these three 'Styles' are completely different programs. In Style2 your variables are not even members. — kylecorver, May 20 '15 at 08:16
Hoping I understood your question: Style 3 looks sensible to me. Also check for [pimpl idiom](http://en.wikipedia.org/wiki/Opaque_pointer). This may be interesting for you. — Mohit Jain, May 20 '15 at 08:19
@Biffen because if `MyClass.h` changes all users of `A.h` have to recompile. If `MyClass` was just forward-declared in `A.h` and the `MyClass.h` include line was moved to `A.cpp` this wouldn't be necessary. — RJFalconer, May 20 '15 at 08:41
You can use private pointer to private structure, if you really must hide private members from header. Just use forward declaration in header and define structure itself in cpp file. Best aproach is to include private members in headers and generate easy to read documentation containing only public members. For example by doxygen. — Pihhan, May 20 '15 at 08:45
For future reference, please lead with the question. And the illustrate using the class:) — laurisvr, May 20 '15 at 09:10

molbdnilo · Answer 1 · 2015-05-20T09:07:16.747

1

Your three programs don't mean remotely the same thing.

In the first "style", myInt, myFloat, and myClass are members.
In the second, none of them is a member.
In the third, myInt and myFloat are members, and myClass isn't.

If you want all three to be members, you must declare that in the class definition, as in your first example.

If you don't want your header to depend on the definition of MyClass, use a forward declaration of the class and add an indirection:

class MyClass;

class A{
    private:
        int myInt;
        float myFloat;
        MyClass* myClass;
};

edited May 20 '15 at 09:07

answered May 20 '15 at 08:31

molbdnilo

64,751
3
43
82

I've rolled this answer back, as I don't appreciate edits that break formatting, are badly spelled, or don't add anything useful. – molbdnilo May 20 '15 at 09:08

ypx · Answer 2 · 2015-05-20T08:56:24.087

0

My takes on the different styles:

Style 1: Nothing wrong with that. But requires the user to include MyClass.h whenever they include A.h

Style 2: You've changed the private members to 'global' variables. Their values and instances are shared between all instances of class A. This will become a problem when you start defining methods in A that manipulate those variables and then create different instances of A.

Style 3: Similar to Style 2, except that it only applies to the MyClass object. The others are safe.

To shield the user from including the MyClass header. You can use forward declaration and change your MyClass member to be of type MyClass* (a pointer). Shielding a user of a class from private definitions is not a bad thing. People sometimes use it to deliver more self-contained code and lift the burden from providing all dependencies. It could also help in reducing compile times.

Your A.h would look like this:

#ifndef _A_H_
#define _A_H_

class MyClass;

class A {
    A();

    ~A();

    private:
        int myInt;
        float myFloat;
        MyClass *myClass;
};

#endif // _A_H_

and your A.cpp

#include "A.h"

#include "MyClass.h"

A::A() {
  myClass = new MyClass();
}

A::~A() {

  delete myClass;
}

edited May 20 '15 at 08:56

answered May 20 '15 at 08:32

ypx

1,459
11
19

‘*But requires the user to include MyClass.h whenever they include A.h*’ But it's already included in `A.h`...? – Biffen May 20 '15 at 08:38
@Biffen I mean that the user needs to provide the MyClass header to compile A.h. – ypx May 20 '15 at 08:40
Ah, perhaps the word ‘include’ confused me. But since it's included with quotes it's hopefully in the same directory. – Biffen May 20 '15 at 08:41
@Biffen It doesn't necessarily have to be in the same directory. It only needs to be in the compilers' include paths. in the case of gcc, whatever you add to the -I arg. – ypx May 20 '15 at 08:43
Yes, hence ‘hopefully’. – Biffen May 20 '15 at 08:44
@Biffen Yes :-), but it won't take long for the compiler to complain if it can't find it. – ypx May 20 '15 at 08:45

Private member should be defined in .h or .cpp?

2 Answers2