Search code examples
c++compile-timepimpl-idiom

Is pimpl idiom better than using always unique_ptr as member variables?


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

Solution

  • 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.