Re: Using 'this' in constructor

On Fri, 17 Oct 2008 09:59:15 -0700 (PDT), bb <muralibala68@gmail.com>
wrote:


I see two issues. One is the order of the code, as Victor pointed out.
The other is that you haven't defined AImpl - only declared it.

Assuming that's significant as opposed to a rushed example mix-up, you
may need to define the constructor separately from the declaration.
For example, you might have...

// In header

class A
{
  private:
    struct AImpl;

    AImpl* pimpl_;
      // should probably use std::auto_ptr<A_impl>
      // doing so would avoid the need for the destructor

  public:
    A ();
    virtual ~A();
};

// In cpp file

struct A::AImpl
{
  A& owner_ref;

  AImpl (A* p_owner_ref) : owner_ref (*p_owner_ref);
  {
  }
};

A::A ()
{
  pimpl_ = new AImpl (this);
}

A::~A ()
{
  delete pimpl_;
}

I've got the feeling I'm doing something wrong in that but I'm not
sure what - maybe just tired - but the principles about right.

As Victor also pointed out, this isn't ideal. If the AImpl constructor
does anything with the instance of A (as opposed to just the pointer)
it is probably broken, since the instance isn't considered constructed
at this point.

Issues like this lead to the common style rule that constructors
should do the minimum work necessary to set up a self-consistent
state. More substantial initialisation should be done by a separate
method in a separate call.

I don't stick to the rule all that well myself, but it is something to
bear in mind. On this principle, one alternative (not necessarily a
preferred one) is...

// In header

class A
{
  private:
    struct AImpl;
    AImpl* pimpl_;

  public:
    A ();
    virtual ~A();

    void Initialise ();
};

// In cpp file

struct A::AImpl
{
  A& owner_ref;

  AImpl (A* p_owner_ref) : owner_ref (*p_owner_ref);
  {
  }
};

A::A ()
{
  pimpl_ = 0;
}

void A::Initialise ()
{
  if (pimpl_) throw "Oops - double initialised";
  pimpl_ = new AImpl (this);
}

A::~A ()
{
  if (pimpl_) delete pimpl_;
}