9

In my workplace we have this convention: almost every class (with very few exceptions) is implemented with unique_ptrs, raw pointers or references as member variables.

This is because of compilation times: in this way you just need a forward declaration for the class in your header file and you only need to include the file in your cpp. In addition if you change the .h or the .cpp of a class that you are including with unique_ptr you will not need to recompile.

I think that this pattern has at least these downsides:

  • it forces you to write your own copy constructors and assignment operators and you have to manage each variable individually, if you want to keep the semantic of copying.
  • the syntax of the code become very cumbersome, for example you will have std::vector<std::unique_ptr<MyClass>> instead of the much more simple std::vector<MyPimplClass>.
  • the constness of the pointer is not propagated to the pointed object, unless you use std::experimental::propagate_const, which I cannot use.

So it came to my mind to propose to use the pImpl idiom for the classes being included as a pointer instead of using pointers everywhere. In this way I thought we can have the best of both worlds:

  • Faster compile times: pimpl reduces compilation dependencies
  • To write your copy constructor and your copy assignment operator you just do this:
A::A(const A& rhs) : pImpl(std::make_unique<Impl>(*rhs.pImpl)) {}
A& A::operator=(const A& rhs) {
  *pImpl = *rhs.pImpl;
  return *this;
}
  • The constness is propagated to the member object.

At this point I had a discussion with my coworkers, they are arguing that pImpl is that it is not better than using pointers everywhere because of the following:

  • It reduces compilation dependencies less than using pointers, because if you are using pImpl you have to recompile the classes that includes your pImpl class when you change your public interface: if you were using just a pointer instead of the pImpl class you won't need to recompile even when changing the header file.

Now I am a little confused. I think that our actual convention is not really better than pImpl but I am not able to argue why.

So I have some questions:

  • Is the pImpl idiom a good alternative in this scenario?
  • Are there other downsides of the pattern we are using, except the ones that I mentioned?

Edit: I am adding some examples to clarify the two approaces.

  • Approach with unique_ptr as member:
// B.h
#pragma once
class B {
    int i = 42;
public:
    void print();
};

// B.cpp
#include "B.h"
#include <iostream>
void B::print() { std::cout << i << '\n'; }

// A.h
#pragma once
#include <memory>
class B;
class A {
    std::unique_ptr<B> b;
public:
    A();
    ~A();
    void printB();
};

// A.cpp
#include "A.h"
#include "B.h"
A::A() : b{ std::make_unique<B>() } {}
A::~A() = default;
void A::printB() {  b->print(); }
  • Approach with pImpl:
// Bp.h
#pragma once
#include <memory>
class Bp {
    struct Impl;
    std::unique_ptr<Impl> m_pImpl;
public:
    Bp();
    ~Bp();
    void print();
};

// Bp.cpp
#include "Bp.h"
#include <iostream>
struct Bp::Impl {
    int i = 42;
};
Bp::Bp() : m_pImpl{ std::make_unique<Impl>() } {}
Bp::~Bp() = default;
void Bp::print() {  std::cout << m_pImpl->i << '\n'; }

// Ap.h
#pragma once
#include <memory>
#include "Bp.h"
class Ap {
    Bp b;
public:
    void printB();
};

// Ap.cpp
#include "Ap.h"
#include "Bp.h"
void Ap::printB() { b.print(); }

Main:

// main.cpp
#include "Ap.h"
#include "A.h"

int main(int argc, char** argv) {
    A a{};
    a.printB();

    Ap aPimpl{};
    aPimpl.printB();
}

Moreover I want to be more precise when I say that the first approach we don't need to recompile, this is inaccurate. It is true that we need to recompile less files:

  • If we change B.h we need to recompile only A.cpp, B.cpp.
  • If we change Bp.h we need to recompile Ap.cpp, Bp.cpp and main.cpp
Laurel
  • 5,965
  • 14
  • 31
  • 57
Dundo
  • 714
  • 8
  • 12
  • "if you where using just a pointer instead of the pImpl class you did not need to recompile even when changing the header file." No, this isn't true. If you were just passing a pointer through, then yes, it's true, but you can still pass the pointer through if your class is implemented with the pimpl pattern. Otherwise, if you actually needed to do something with the class, then you'd still need to recompile if you change the header file. – Justin May 20 '21 at 07:37
  • To make the use of unique_ptr easier, you can use `using MyClass_ptr = std::unique_ptr` and you can set the functions you've mentioned with `= default`. – JMRC May 20 '21 at 07:41
  • `std::vector` and `std::vector>` are completely different things – Raildex May 20 '21 at 07:44
  • @JMRC it won't compile with `= default`. – Evg May 20 '21 at 07:49
  • @Justin I added some examples to clarify. My point is that with unique_ptr as member we have to compile less files. – Dundo May 20 '21 at 09:13
  • @Raildex Please argue. I am saying that we have `std::vector` versus `std::vector>`, where `MyPimplClass` is `MyClass` implemented with the pImpl idiom. In this case I think that the two approaches are not so different, in the pImpl case the unique_ptr is hidden inside the implementation. – Dundo May 20 '21 at 09:29
  • As far as I can see, your "convention" is just "public pimples", and your colleagues' argument essentially boils down to wanting to reuse the same pimple across multiple classes. On the other hand, perhaps that's the point, and your hiding of details would go against this point. – molbdnilo May 20 '21 at 09:30
  • 2
    It doesn’t answer your question, but it might help to use something like the proposed [`std::indirect_value`](http://open-std.org/JTC1/SC22/WG21/docs/papers/2020/p1950r1.html) for **either** of these approaches. – Davis Herring May 20 '21 at 15:27
  • @DavisHerring thank you, this is a very interesting read :) – Dundo May 21 '21 at 09:30
  • This question should probably be given a better title, although I'm not able to think of one at the moment. "Better" implies a subjective judgment, which is out of scope here; the details of the question suggest specific criteria. – Karl Knechtel May 28 '22 at 23:03

1 Answers1

0

After a while I have a broader understanding of the problem and finally I can answer to my own question.

It turned out that what I was saying was not completely correct.

In fact in the code below only Bp class is pImpl. If we change Ap to be pImpl aswell we obtain that, if we change Bp.h we need to recompile only Ap.cpp, Bp.cpp, which is the same of the corresponding solution with unique_ptrs.

Said that, I think I can say that the solution with pImpl seems in general better than the solution with unique_ptrs (we just have to pImpl the correct classes!).

For this reason we decided to switch to pImpl idiom as default for our classes.

Dundo
  • 714
  • 8
  • 12