2

The PIMPL Idiom is a technique for implementation hiding in which a public class wraps a structure or class that cannot be seen outside the library the public class is part of. This hides internal implementation details and data from the user of the library.

But is it possible to implement the same making use of reference?

MCanvasFont.h

namespace Impl {
    class FontDelegate;
}

class MCanvasFont
{
public:
    MCanvasFont();
    virtual ~MCanvasFont();

protected:
    // Reference count
    long m_cRef;

    // agg font delegate
    const Impl::FontDelegate& m_font;
}

MCanvasFont.cpp

// helpers
#include "ImplHelpers/FontDelegate.h"

MCanvasFont::MCanvasFont()
: m_cRef(1),
  m_font(Impl::FontDelegate() )
{
    // constructor's body
}

P.S. This code compiles without any problems with G++.

ezpresso
  • 7,896
  • 13
  • 62
  • 94

1 Answers1

5

There is an error in your program, and it's in the constructor's initialiser list:

MCanvasFont::MCanvasFont()
: m_cRef(1),
  m_font(Impl::FontDelegate() ) // <--- BANG
{

The problem with Impl::FontDelegate() is that it constructs a temporary object. This won't outlive the constructor - in fact it's actually destroyed before entering the constructor body, as it's lifetime is that of the expression in which it appears. Thus your m_font reference is instantly invalid.

While you could initialise it using a manually allocated object (*new Impl::FontDelegate()) you're in undefined territory if that allocation fails unless you have exceptions enabled in your runtime. You'll also still have to delete the object in your destructor anyway. So the reference really doesn't buy you any advantages, it just makes for some rather unnatural code. I recommend using a const pointer instead:

const Impl::FontDelegate* const m_font;

EDIT: Just to illustrate the problem, take this equivalent example:

#include <iostream>

struct B
{
    B() { std::cout << "B constructed\n"; }
    ~B() { std::cout << "B destroyed\n"; }
};

struct A
{
    const B& b;
    A() :
        b(B())
    {
        std::cout << "A constructed\n";
    }
    void Foo()
    {
        std::cout << "A::Foo()\n";
    }
    ~A()
    {
        std::cout << "A destroyed\n";
    }
};

int main()
{
    A a;
    a.Foo();
}

If you run this, the output will be:

B constructed
B destroyed
A constructed
A::Foo()
A destroyed

So b is invalid almost immediately.

pmdj
  • 22,018
  • 3
  • 52
  • 103
  • 1
    If allocation fails, you get an exception, so there are no issues there. (barring the usual exception-in-constructor issues) –  May 28 '12 at 12:13
  • One can use a singleton pattern to allocate an object statically. It's GetInstance would return a reference to a static object. – Steed May 28 '12 at 12:15
  • The temporary's lifetime is actually extended to cover the constructor's body; but as you say, it certainly doesn't cover the object's lifetime. – Mike Seymour May 28 '12 at 12:53
  • @MikeSeymour hmm, GCC seems to disagree - when I run the example with the logged constructor/destructor, the temporary is destroyed before the constructor's body. – pmdj May 28 '12 at 13:19
  • @Hurkyl true, though many disable exceptions in which case new just returns NULL. – pmdj May 28 '12 at 13:19
  • @pmdj: "I recommend using a const pointer instead:". And I recommend using [std::unique_ptr](https://isocpp.org/wiki/faq/cpp11-library#unique-ptr) and not dealing with raw pointers and manual resource handling, unless there is a very good reason to. – Андрей Беньковский Oct 24 '15 at 19:21