Re: Pimpl-ization

Hi,

On Wed, Dec 10, 2014 at 09:09:04PM -0500, Kohei Yoshida wrote:
> 1) To reduce the size of our shared objects.
> 2) To improve our build time.
> [...]
> Please share your opinions.

Both are valuable goals and doers are deciders, so sounds good to me.

As a note though, I personally prefer an abstract base class plus factory
function over pimpl though. For me, it removes boilerplate (and leaves it for
the compiler to handle) and is more readable:

Pimpl 

(HXX):

class AnotherThing;
class RealThing;

struct Thing {
    Thing(AnotherThing* pAnother);
    void DoSomething()
        { m_pImpl->DoSomething() }
    void DoSomethingElse()
        { m_pImpl->DoSomethingElse() }
    ...
    std::unique_ptr<RealThing> m_pImpl;
}

(CXX):

struct RealThing SAL_FINAL {
    RealThing(AnotherThing* pAnother)
        : m_pAnother(pAnother) {}
    void DoSomething();
    void DoSomethingElse();
    AnotherThing* m_pAnother;    
}

Thing::Thing(AnotherThing* pAnother)
    : m_pImpl(new(RealThing(pAnother)))
{}

RealThing::DoSomething()
{ .... }
RealThing::DoSomethingElse()
{ .... }

ABC plus factory function

(HXX):

class AnotherThing;
struct Thing {
    static Thing* Create(AnotherThing* pAnother);
    virtual void DoSomething()=0;
    virtual void DoSomethingElse()=0;
    ...
    virtual ~Thing() {};
}


(CXX):

struct RealThing SAL_FINAL {
    RealThing(AnotherThing* pAnother)
        : m_pAnother(pAnother) {}
    virtual void DoSomething();
    virtual void DoSomethingElse();
    AnotherThing* m_pAnother;
}

Thing* Thing::Create(AnotherThing* pAnother)
    { return new(RealThing(pAnother)); }

RealThing::DoSomething()
{ .... }
RealThing::DoSomethingElse()
{ .... }

Both of these do one heap alloc. Cosmetically the ABT is already shorter
LOC-wise, Pimpl needs more boilerplate for each added member function and the
ABC looks much better in e.g. doxygen. Its true that you can use the Pimpl
Thing-class as a "value type" and not the ABC, but the former is just a wrapper
around a pointer anyway, so this isnt of much practical relevance AFAICS.

To repeat: Im fine with adding Pimpls were sensible. But Id like to suggest to
maybe also consider ABCs were sensible.

Best,

Bjoern