2

I am using visual studio on windows10 using C++.

And I'm learning about pimpl idiom, what confuses me is that if without destructor of employee, compiler would bring out error: can't delete an incomplete type. I don't understand why I need a destructor for employee, isn't it enough having a destructor for struct impl because of unique_ptr.

this is the website which introduce pimpl idiom, it says: "Since std::unique_ptr is a complete type it requires a user-declared destructor and copy/assignment operators in order for the implementation class to be complete." But I still don't understand and didn't get much information about the definition of complete type it says.

https://www.geeksforgeeks.org/pimpl-idiom-in-c-with-examples/

employee.h

#pragma once
#include<iostream>
#include<string>
using std::string;
using std::unique_ptr;

//pimple idiot
namespace employee {
    class employee {
    public:
        employee(const string& s);
        //~employee();//if without this line, compiler would bring out error: can't delete an incomplete type
        employee(const employee& e);
        employee& operator=(employee e);
        
        void setSalary(float s);
        float getSalary();
    private:
        struct impl;
        unique_ptr<impl> pimpl;
    };
}

employee.cpp

#include"employee.h"

namespace employee {
    struct employee::impl {//
        impl(const string& s) :name(s),salary(0){}
        ~impl(){}//for unique_ptr

        string name;
        float salary;
    };
    employee::employee(const string& s):pimpl(new impl(s)){
    }
    //employee::~employee() = default;
    employee::employee(const employee& e) : pimpl(new impl(*e.pimpl)) {
    }
    employee& employee::operator=(employee e) {
        std::swap(this->pimpl, e.pimpl);
        return *this;
    }

    void employee::setSalary(float s) {
        pimpl->salary = s;
    }
    float employee::getSalary() {
        return pimpl->salary;
    }
}
ShadowRanger
  • 143,180
  • 12
  • 188
  • 271
keepGoing
  • 31
  • 2

2 Answers2

1

You need to know how to delete the class once the scope exits. If definition is hidden then there is no knowledge how to call destructor. You can do so without defining impl by writing a custom deleter for unique_ptr.

Simply add class impl_deleter having operator ()(impl *) that calls delete - needs to defined where impl is fully defined. And use unique_ptr<impl, impl_deleter>.

Seek C++ reference for details on unique_ptr.

Or just write full declaration of struct impl in the header.

ALX23z
  • 4,456
  • 1
  • 11
  • 18
  • Obviously writting `impl`in the header remove the main benefit of the idiom of hiding that code and reduce compilation dependencies. – Phil1970 Aug 29 '21 at 00:56
0

The reason it need the destructor is essentially so that the template code get instanciated.

As te type is defined is defined in the source file and the destructor of employee need to destroy the unique_ptr it need to know how to do it.

Compiler know know to do this if impl is a complete type. So you only need to define the destructor after the definition of impl.

Thus the correct way is to define the destructor as defaulted in source file if you don't need it otherwise.

employee.cpp

namespace employee
{
    struct employee::impl { .... };
    employee::~employee() = default; // If you need only for pimpl idiom
}
Phil1970
  • 2,605
  • 2
  • 14
  • 15
  • That's insufficient. The code would not work were it to have move constructor / assignment - and it should've had. Defining them in .cpp would make the normally fast move operations into slow non-inlinable function calls. – ALX23z Aug 29 '21 at 03:26
  • @ALX23z Usually when one use the pimpl idiom, one extra non inline call would not matter much as all calls to `impl` will be indirect as the main purpose of the idiom is to hide implementation details more that what you would get with `private` section directly in main class (`employee` here). Most of the time when `pimpl` is used, it is on relatively large classes that are not copiable or moveable anyway. – Phil1970 Aug 29 '21 at 23:22
  • Not all methods of `pimpl` need to be slow - some can and need to be fast to the extent that an additional very shallow handles are made to represent the `pimpl` in some way just the calls could be inlined and be quick. And it isn't just `pimpl` case. It can generally be used to hide dependencies from the header. And in some cases, it is desirable to utilise the class and expose its dependencies in another .cpp file. However, unless one knows how to destroy the `unique_ptr` it will be difficult to pass them from one to another. Basically custom destructor is a more flexible technique. – ALX23z Aug 30 '21 at 11:18
  • @ALX23z **Simpler, more readable code** is preferable to **premature optimization**. Defaulted (or empty body destructor) in source file works great in many applications. – Phil1970 Aug 31 '21 at 01:15