3

In a C++ class, should I be placing my private functions and variables in the private section of the class header definition, or in the class source file and why?

For example:

// Header
class MyClass {
public:
    void doSomething();
private:
    int a = 0;
}

// Source
void MyClass::doSomething()
{
    // Do something with `a`
}

or

// Header
class MyClass {
public:
    void doSomething();
}

// Source
int a = 0;

void MyClass::doSomething()
{
    // Do something with `a`
}

I've always thought, when programming it's best to make the scope of a function/variable as small as possible. So shouldn't restricting the scope of the var a to the scope of the source file be best?

Ari Seyhun
  • 11,506
  • 16
  • 62
  • 109
  • 5
    Your `a`in the second example is much "less" scoped than in the first, it's not a member variable at all but declared at global scope. – Mat Feb 08 '18 at 08:27
  • 7
    `a` isn't a member in the second version. The two aren't equivalent. – StoryTeller - Unslander Monica Feb 08 '18 at 08:27
  • `int a = 0` is a way to initialize it "by default" if it is not initialized in a constructor. There's no real best practice about that but putting it in the class definition itself may help to make it clearer (since you define the member's default value right where the said member is defined). Although what you propose is not equivalent, if you want to define it in the source file you'd need `int MyClass::a = 0` (not sure this would work, since it's for static members only). – Vivick Feb 08 '18 at 08:28
  • Dupe of [Split a C++ class declaration](https://stackoverflow.com/questions/10456362/split-a-c-class-declaration) – Yola Feb 08 '18 at 08:28
  • @Vivick I see, I am asking this question because some of my class header files can be filled with private variables and methods that are only used in the one source file, and it makes things quite messy. – Ari Seyhun Feb 08 '18 at 08:30
  • @Acidic sadly `int MyClass::a = 0` is only for static members, you'll have to stick with default values in the header file. – Vivick Feb 08 '18 at 08:33
  • Maybe you are looking for [pimpl](http://en.cppreference.com/w/cpp/language/pimpl) –  Feb 08 '18 at 09:35

2 Answers2

2

They are not equivalent. First example

// Header
class MyClass {
public:
    void doSomething();
private:
    int a = 0;
}

// Source
void MyClass::doSomething()
{
    ++a;
    cout << a << endl;
}

int main()
{
    MyClass x, y;
    x.doSomething();
    y.doSomething()
}

Output

1
1

Second example

// Header
class MyClass {
public:
    void doSomething();
}

int a = 0;

// Source
void MyClass::doSomething()
{
    ++a;
    cout << a << endl;
}

int main()
{
    MyClass x, y;
    x.doSomething();
    y.doSomething()
}

Output

1
2

In the first example a is a class variable so x and y have their own copy of a. In the second example there is only one global variable a so the output is different.

john
  • 85,011
  • 4
  • 57
  • 81
  • That makes sense, but what about some basic things like a struct or function? – Ari Seyhun Feb 08 '18 at 08:35
  • @Acidic Yes, you can forward declare member types and member functions in the definition of a class, and later provide definitions for them. – Caleth Feb 08 '18 at 09:23
  • @Acidic if a struct or a function is only used in one file then it makes sense to define and declare then in that one file instead of using a header file. Functions that are only used in one file should be declared static so they aren't visible outside of the file. – john Feb 08 '18 at 10:27
  • @john declared as static inside the file but *not* as a part of the class? Just inside the source file use `static void myFunction() { ... }`? – Ari Seyhun Feb 08 '18 at 15:35
  • @Acidic That's what I had in mind. But of course if a function needs to be part of a class then this isn't an option. – john Feb 08 '18 at 15:56
0

You could use the pimpl idiom... Alternatively, you could use this variant of the pimpl idiom, where the memory of the implementation is directly provided by the interface class:

In file MyClass.hpp :

class MyClass{
  private:
    std::byte buffer[N];
  public:
    MyClass();
    void public_method();
    ~MyClass();
  };

In class MyClass.cpp:

#include "MyClass.hpp"
namespace{
  struct MyClassImpl{
     private:
       int val=0;
     public:
       void secret_method(){/*...*/}
     };
  inline const MyClassImpl& 
  get(const std::byte* buffer){
    //In theory, in C++17 you should use std::launder here to be standard compliant
    //in practice, compilers generate the expected code without std::launder
    //and with std::launder they generate horrible and inefficient code!
    return *reinterpret_cast<const MyClassImpl*>(buffer);
    }
  inline MyClassImpl& 
  get(const std::byte* buffer){
    //idem here to be c++17 standard compliant launder is necessary
    //in practice, it would be a mistake.
    return *reinterpret_cast<MyClassImpl*>(buffer);
    }
 }

 MyClass::MyClass(){
   new(buffer) MyClassImpl{};
   }
 MyClass::~MyClass(){
   get(buffer).~MyClassImpl();
   }
 void
 MyClass::public_method(){
    /*....*/
    get(buffer).secret_method();
    /*....*/
    }

Compared to classical pimpl idiom:

  • Pros: less memory access, no memory allocation on the heap, more efficient

  • Cons: error prone, the size of the implementation "leak" in the interface.

Oliv
  • 17,610
  • 1
  • 29
  • 72