3

I am wondering if I am using the good approach in the following :

  • I want to construct a parent class (class A), this class should own an instance of a given "Foo" class
  • I want the parent class to own a child class member (class B) and this member should have a reference to the foo member of the parent class.

The code below seems to works, however I am wondering whether I was just "lucky" that the compiler was sympathetic enough.

For clarity, I added comment and my question in the comments below.

Thanks !

struct Foo
{
  std::string mValue;
};

class B
{
public:
  B(const Foo & foo) : mFoo_External(foo) {}
private:
  const Foo & mFoo_External; //this is an external reference to the member 
                             //(coming from A)
};

class A
{
public:
  //Here is the big question 
  //Shall I use : 
  //  A(const Foo & foo) : mFoo(foo), mB(mFoo) {}  
  //  or the declaration below
  A(const Foo & foo) : mFoo(foo), mB(foo) {}
private:
  //According to my understanding, the declaration 
  //order here *will* be important
  //(and I feel this is ugly)
  const Foo  mFoo;
  B mB;
};



void MyTest()
{
  std::auto_ptr<Foo> foo(new Foo());
  foo->mValue = "Hello";
  A a( *foo);
  foo.release();

  //At this point (after foo.release()), "a" is still OK 
  //(i.e A.mB.mFooExternal is not broken, although foo is now invalid)
  //
  //This is under Visual Studio 2005 : 
  //was I lucky ? Or is it correct C++ ?
}
Pascal T.
  • 3,866
  • 4
  • 33
  • 36
  • 1
    I don't see any parent or child classes :/ do you just mean one owns the other or did you intend some inheritance? – John Humphreys Oct 12 '11 at 21:38
  • "Parent"/"child" is a terrible analogy for inheritance. We're not talking about who gets the house when the parent dies. OO inheritance is about being a more specific version of something. A child isn't a more specific version of a parent, though. The present "ownership" (or "membership") relation is actually more suitable for a traditional family analogy. – Kerrek SB Oct 12 '11 at 21:48
  • @PascalT, I don't see how "the big question" marked in your code (whether to initialize mB with foo or with mFoo) has anything to do with the declaration order of mFoo and mB, which you claim to be contemplating. – Danra Oct 12 '11 at 21:53
  • @Danra : if you look at the correct constructor : A(const Foo & foo) : mFoo(foo), mB(mFoo) {} , you have to know that mFoo is constructed before mB *only* thanks to the declaration order, not thanks to the way the constructor is written – Pascal T. Oct 12 '11 at 21:58
  • @Danra But you are right, my real big question was about the declaration order, not about the constructor – Pascal T. Oct 12 '11 at 22:00
  • @PascalT, I meant the "big question" wasn't your stated big question. When you have the initializer list "out of order" with the declaration order, are you not getting a warning in Visual Studio (With warning level 4)? XCode does warn about this by default. If most modern compilers do give out warnings about this then commenting probably isn't *that* important. – Danra Oct 12 '11 at 22:10

2 Answers2

6

No, this is broken. Your mB will hold a reference to whatever you passed to the constructor of the A object, not to mFoo. Instead, you should say:

A(const Foo & foo) : mFoo(foo), mB(mFoo) { }

Note that mB is a copy of the constructor argument, and not a reference, so your MyTest function is fine.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • This was my first version. However, do you have any idea on how to remove the dependency on the declaration order ? – Pascal T. Oct 12 '11 at 21:39
  • @PascalT.: Why would you want that? No, there's not. `mB::mFoo_External` can't refer to something that doesn't exist, so you have to construct that first. – Kerrek SB Oct 12 '11 at 21:40
  • I just want my code to resist to anyone who would want refactor my class and innocently change the declaration order in the header. I guess I will be adding a warning in the header. – Pascal T. Oct 12 '11 at 21:42
  • Or you could redesign the class to reduce the coupling, if that's an option... it all depends. There's nothing inherently wrong with the current setup, though. – Kerrek SB Oct 12 '11 at 21:46
  • This seems like a bad idea design wise. B is completely dependent on the user passing in a value which won't be destructed while B is still alive - if anyone uses it for anything (besides a intrinsic member of A) it'll be very easy to make a bad mistake (i.e. get a dangling reference). Is there a reason it needs to be coupled like that? – John Humphreys Oct 12 '11 at 21:46
  • Is there a reason it needs to be coupled like that? I don't know for sure, this might only be a transitory state in my current refactoring :-) For now, this is the option I will use – Pascal T. Oct 12 '11 at 21:51
  • Thank you all, it is always nice to exchange with knowledgeable fellows. – Pascal T. Oct 12 '11 at 21:54
3

Since you want your B object to hold a reference to the parent's member, you must initialize mB with mFoo not foo.

You are correct that the order of the member variables is important, since it determines the order of the initialization. It might come as a surprise that the order of the initializers in the constructor does not determine the order they are called! See Constructor initialization-list evaluation order.

Community
  • 1
  • 1
Mark Ransom
  • 299,747
  • 42
  • 398
  • 622