Pointers or references for dynamically allocated members that always exist?

Question

I have a class CContainer that has some members CMemberX, CMemberY, which are independent of each other and other CClientA, CClientB classes that use CContainer.

#include "MemberX.h"
#include "MemberY.h"

class CContainer
{
public:
    CMemberX & GetX() const { return m_x; }
    CMemberY & GetY() const { return m_y; }

private:
    CMemberX m_x;
    CMemberY m_y;
};

I want to avoid having to recompile all CClient classes when modifying one of the CMember classes using forward declarations and dynamic allocation of m_x and m_y.

Initially, I made the members pointers:

// Container.h
class CMemberX;
class CMemberY;

class CContainer
{
public:
    CContainer();
    ~CContainer();

    CMemberX & GetX() const { ASSERT(m_pX != NULL); return *m_pX; }
    CMemberY & GetY() const { ASSERT(m_pY != NULL); return *m_pY; }

private:
    CMemberX* m_pX;
    CMemberY* m_pY;
};

// Container.cpp
#include "Container.h"
#include "MemberX.h"
#include "MemberY.h"

// Allocate members on heap
CContainer::CContainer() : m_pX(new CMemberX()), m_pY(new CMemberY()) {}
CContainer::~CContainer() { delete m_pX; delete m_pY; }

Then I thought, that I could as well use references instead of pointers, so it looks more like the original code:

// Container.h
class CMemberX;
class CMemberY;

class CContainer
{
public:
    CContainer();
    ~CContainer();

    CMemberX & GetX() const { return m_x; }
    CMemberY & GetY() const { return m_y; }

private:
    CMemberX & m_x;
    CMemberY & m_y;
};

// Container.cpp
#include "Container.h"
#include "MemberX.h"
#include "MemberY.h"

// Allocate members on heap
CContainer::CContainer() : m_x(*new CMemberX()), m_y(*new CMemberY()) {}
CContainer::~CContainer() { delete &m_x; delete &m_y; }

What I don't like about the pointer members is that it looks like the pointers could be NULL or the objects be replaced at runtime, which is not the case.

What I don't like about the references is that the code in the CTor and DTor looks a bit hacky.

Which approach is preferable? Is there a better solution?

Note regarding copying/assigning: Instances of the CContainer class will not under any circumstances be copied or assigned to each other.

Answer 1

There have been a lot of questions here about the desirability of using references as members (for example http://stackoverflow.com/questions/892133/should-i-prefer-pointers-or-references-in-member-data), and it seems to me that the majority opinion (which also happens to be mine) is - don't. If you don't want the pointers to be changed make them const - I can't see how, given your code, they can possibly be NULL.

Answer 2

Hi,

Dynamic allocation does nothing for you in respect of what you want: not having to recompile CClient and CContainer.

The only allowed use when using forward declarations is declaring pointer(s) to the forward-declared type.

As soon as you use a method or a member of the forward-declared type it will not compile: the compiler MUST know the full type of whatever it is using.

In short: either you never have to recompile [you are only declaring pointers to the forward declared type] OR you always have to recompile, in case you actually DO use CContainer.

Answer 3

I think that's what the const variables are for:

CMember * const m_x;

Cannot change m_x after initialization...

Answer 4

In the second block of code in your question you have private member pointers which are initialised and destroyed along with the parent class. This information should be enough to the reader of your code to realise what is going on.

In addition you could declare the pointers const: CMember* const m_pX; to indicate that they cannot be changed after initialisation. Now the compiler will catch accidental changes.

Answer 5

I think it's a little surprising to use a reference when there are ownership semantics. Doesn't necessarily make it a bad idea, all things considered, but it does weigh against.

I think I've only ever used references as members in cases where both:

• an object is provided to the constructor, which is required to outlive this object.
• assignment is forbidden anyway.

So for example, injected dependencies such as factory or service objects could be suitable. As against that, in C++ you'd often prefer to inject dependencies with template parameters rather than objects, so the issue might not arise.

I also find that the longer I use C++, the more I want types to be Assignable unless there's a really good reason not to be. The usual trick for reducing compile-time dependencies in the way you want is "Pimpl", not "Rimpl", for a reason. By switching from an object member to a reference member, you are making your class non-default-copyable, where previously perhaps it was copyable. This implementation detail shouldn't constrain the class's interface. With Pimpl you can cleanly implement assignment and swap. With these references you would have to assign or swap both members. If the second swap fails, you've lost the strong exception guarantee: although if your CMemberX and CMemberY classes have no-fail assignment and swap, this doesn't matter.

So I don't think I like the reference in this case, but then I haven't seen the rest of your code. Maybe there's some reason why none of the concerns about assignment apply - for instance if CContainer is itself a RAII class, then usually the only lifecycle operations it should support are construction and destruction.

Answer 6

You are not really buying yourself anything.
(Slightly shorter compile time in limited situations).

But you are heaping a whole lot of other code that needs to be maintained onto your plate.

If these objects are natural members then leave them as members.
By creating them on the stack and storing them as pointers or references you have to ask a whole bunch of sticky questions that need code to answer them.

• What happens when you copy construct the object.
  • I would normally expect the obejct and all its members to be copied.
    Using pointers or references you are going to have to do extra work to replicate this functionality that is already provided.
• What happens when you assign the objects.
  • References will not work (though you get around this by using boost references).
    But you still have the problem with expecting the members to be copied.
• What happens when you delete the object.
  • If you have implemented all the above correctly to make copies fine.
    Otherwise you need to start thinking about shared pointers to implement the functionality you need.

As it stands versions 2 and 3 (of the code in the question) are seriously flawed and the only version that actually works is 1.

In my mind the simple fact that maintenance costs will be so much lower with version 1, that recommending either of version 2 or 3 is counter productive. The extra time to compile one more class when a member is changed is relatively small in comparison to the complexity you are adding to the code.

Also you mention in somebody else s comment that the code is not quite as clean as that described above. This just emphasizes my point that this is bad optimization that will make it hard to get the class working correctly and keep it maintained in that state.

Answer 7

Steve Jessop already mentioned the pImpl idiom in passing, but I think you should check this out if you haven't already come across it: Compilation Firewalls