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:

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:

A::A(const A& rhs) : pImpl(std::make_unique<Impl>(*rhs.pImpl)) {}
A& A::operator=(const A& rhs) {
  *pImpl = *rhs.pImpl;
  return *this;
}

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

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:

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

// 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(); }
// 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 with the first approach, we don't need to recompile. This is inaccurate. It is true that we need to recompile less files:


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.