NFRCR
NFRCR

Reputation: 5580

Would this be a valid way to implement pimpl that supports inheritance?

#include <iostream>
#include <memory>
#include <cstdlib>

class IBase
{
public:
    IBase() = default;
    virtual ~IBase() = default;
    virtual void f1() = 0;
};

class IDerived
{
public:
    IDerived() = default;
    virtual ~IDerived() = default;
    virtual void f2() = 0;
};

class BaseImpl : public IBase
{
public:
    BaseImpl() = default;
    virtual ~BaseImpl() override = default;
    virtual void f1() override { /* serious code */}
};

class DerivedImpl : public BaseImpl, public IDerived
{
public:
    DerivedImpl() = default;
    virtual ~DerivedImpl() override = default;
    virtual void f2() override { /* serious code */}
};

class Base : public IBase
{
public:
    Base() : m_impl(std::make_shared<BaseImpl>()) {}
    virtual ~Base() override = default;
    virtual void f1() override { m_impl->f1(); }

protected:
    Base(const std::shared_ptr<BaseImpl>& impl) : m_impl(impl) {}
    std::shared_ptr<BaseImpl> m_impl;
};

class Derived : public Base, public IDerived
{
public:
    Derived() : Base(std::make_shared<DerivedImpl>()) {}
    virtual ~Derived() override = default;
    virtual void f2() override { impl()->f2(); }

private:
    std::shared_ptr<DerivedImpl> impl() { return std::dynamic_pointer_cast<DerivedImpl>(m_impl); }
};

int main()
{
    Base base;
    base.f1();

    Derived derived;
    derived.f1();
    derived.f2();

    std::cin.sync();
    std::cin.get();
    return EXIT_SUCCESS;
}

It works, but it looks so weird that I might just give up pimpl.

Upvotes: 2

Views: 345

Answers (2)

Fozi
Fozi

Reputation: 5145

Imagine having your Base defined like this:

class Base {
public:
    Base();
    virtual ~Base();
    virtual void f1();

protected:
    class Impl;
    Impl *p_impl; // or shared_ptr, or unique_ptr, or whatever you like.
};

Note that there is nothing defined of Base::Impl. This is a very important part of the PIMPL idiom as you might have to use elements in the Impl class that require #include-ing things you don't want to include in the header class.

The derived class would then look like this:

class Derived: public Base {
public:
    Derived();
    ~Derived();
    virtual void f1(); // or not, depends
    virtual void f2();

protected:
    class Impl2;
    Impl2 *p_impl2; // note that Derived::Impl2 might inherit from Base::Impl
};

This still hides the implementation details of both Base and Derived in Base::Impl and Derived::Impl2, while giving you complete freedom to implement Derived in any way you like (including inheritance).

Upvotes: 2

segfault
segfault

Reputation: 5939

Your Impl classes should only contain private variables and methods.

Don't put anything that that need to be accessible from children class there. Sub-classing the Impl class is wrong way to go in C++ because that defeats the purpose of this pattern.

Upvotes: 1

Related Questions