views:

210

answers:

7

I have a questing around such staff.

There is a class A which has a object of type class B as it's member. Since I'd like B to be a base class of group of other classes I need to use pointer or reference to the object, not it's copy, to use virtual methods of B inside A properly. But when I write such code

class B
  {public:
     B(int _i = 1): i(_i) {};
     ~B() 
       {i = 0; // just to indicate existence of problem: here maybe something more dangerous, like delete [] operator, as well! 
        cout << "B destructed!\n";
       };
     virtual int GetI () const  {return i;}; // for example
   protected:
     int i;
  };

class A
  {public:
      A(const B& _b): b(_b) {}
      void ShowI () {cout <<b.GetI()<<'\n';};
   private:
      const B& b;
  }; 

and use it this way

B b(1);
A a(b);
a.ShowI();

it works perfectly:

1
B destructed!

But

A a(B(1));
a.ShowI();

give very unwanted result: object b creates and a.b is set as reference to it, but just after constructor of A finished, object b destructs! The output is:

B destructed!
0

I repeat again, that using copy of instead of reference to b in A

class A
  {public:
      A(B _b): b(_b) {}
      void ShowI () {cout <<b.GetI()<<'\n';};
   private:
      B b;
  }; 

won't work if B is base class and A calls it's virtual function. Maybe I'm too stupid since I do not know not know proper way to write necessary code to make it works perfectly (then I'm sorry!) or maybe it is not so easy at all :-(

Of course, if B(1) was sent to function or method, not class constructor, it worked perfect. And of course, I may use the same code as in this problem as described here to create B as properly cloneable base or derived object, but doesn't it appears too hard to for such easy looking problem? And what if I want to use class B that I could not edit?

+2  A: 

Use some sort of smart pointer, instead of a reference.

I suggest Boost Smart Pointers.

Autopulated
A: 

C++ Standard 8.5.3 Paragraph 5

The constructor that would be used to make the copy shall be callable whether or not the copy is actually done. [Example:

struct A { };
struct B : public A { } b;
extern B f();
const A& rca = f();  // Either bound to the A sub-object of the B rvalue,
                     // or the entire B object is copied and the reference
                     //  is bound to the A sub-object of the copy
—end example]

The thing is that A a(B(1)); is very bad. struct A get reference to the temporary.

Alexey Malistov
A: 

It is understandable and correct behavior. I could recommend to change allocation from stack to heap. Smart-pointers can help you to avoid explicit destructor calling.

std::auto_ptr<B> b(new B())
A a(*b); //using overriden operator * to get B& 
Dewfy
+6  A: 

The temporary is destructed as soon as the initialization of a is complete. And the reference member will be dangling, referring to a not anymore existing object.

The solution is not difficult or complicated at all. A polymorphic object usually is not temporary, but it exists for a longer time. So you just use a smart pointer to store a reference to the passed object and create the object before-hand with new. Notice that for this, the class B must have a virtual destructor. The shared_ptr will care about destroying the object when your parent object ends lifetime. When copied, shared_ptr will share the pointed to object with the copy, and both point to the same object. This is not necessarily obvious to users of OnwsPolymorphics and not intended for many use cases, so you may wish to make its copy constructor and assignment operator private:

struct OwnsPolymorphics {
  explicit OwnsPolymorphics(B *b):p(b) { }

private:
  // OwnsPolymorphics is not copyable. 
  OwnsPolymorphics(OwnsPolymorphics const&);
  OwnsPolymorphics &operator=(OwnsPolymorphics);

private:
  boost::shared_ptr<B> p;
};

OwnsPolymorphics owns(new DerivedFromB);

Or you use a reference member and store the object on the stack and pass it along to the constructor like you were showing.


Lengthening the lifetime of a temporary is only done when directly assigning to a const reference:

B const& b = DerivedFromB(); 

Then you can also get away with a non-virtual destructor and may call virtual functions on b - provided they are const member functions. This special handling is not done for class members though, and it has the downside of requiring a working copy constructor (polymorphic objects usually are not copyable or designed to be so - they are are identified not by value, but by identity). Anything temporary of other nature is destroyed after the end of the full expression in which it appears - so happening for your temporary B object.

Johannes Schaub - litb
I never heard about boost shared pointers before... Can't see if I should call delete of object of shared pointer p somewhere and if this model will still working if I use 'DerivedFromB b; OwnsPolymorphics owns(b);' together with 'OwnsPolymorphics owns(new DerivedFromB);' because it won't if I use usual pointer 'B* p;' and 'delete p' in destructor.
Nick
You don't need to call `delete`. The smart pointer will do that job for you if its destructor is called by the containing object's destructor. It won't work for `DerivedFromB b; OwnsPolymorphics owns(b);` - that code does not seem to make much sense. Just do `OwnsPolymorphics owns(new DerivedFromB);` instead. There *are* ways to make it work (like using a custom deleter with `shared_ptr`), but they will all make the interface more ugly. You should be clear on the interface - what is the purpose of that other way of construction?
Johannes Schaub - litb
Sorry, the idea of my example was to make both constructions mentioned in question "A a(B(1));" and "B b(1);A a(b);" working properly. Like I can write, for example, "double b=2.0; double c = b**b; cout <<sqrt(b);" as well as "double b = 2.0; cout << sqrt(b*b);" without errors. Thanks you a lot for suggestion, but if it is not possible make both working I'd like to prefer my variant without smart pointers.
Nick
It's a different thing with doubles. You can copy a `double` around and it stays a double. But if you copy a `B` around, you slice it.
Johannes Schaub - litb
You don't need Boost or a `shared_ptr` since the object isn't really shared. You should use an `auto_ptr`.
Potatoswatter
I'm preferring easier to use tools to more simple tools. `boost::shared_ptr` has proven to be easy to use and flexible, to me. `auto_ptr` has many pitfalls (using an inline destructor will cause undefined behavior with incomplete types as used with the pimpl idiom, for example), and will be deprecated in the next Standard version.
Johannes Schaub - litb
+2  A: 

When you write the following:

A a(B(1));
a.ShowI();

the B object passed to A's constructor is a temporary object which is destructed as soon as A's constructor finishes (as you observed). You require the B object to live as long as it is required by A (since A uses a reference to the object).

You could use std::auto_ptr (if you want transfer of ownership semantics i.e. A object to have ownership of the B object that is passed to it's constructor) or otherwise a shared pointer (such as Boost shared_ptr).

Yukiko
+1  A: 

So your problem is that you are using a reference to an object that is allocated on the stack via automatic allocation. That means that when that object leaves scope it is automatically deallocated. Which in the case of the code you're using the object's scope is the same line as you construct the object.

The danger of using references inside of classes is that you must ensure that the object you are referencing has greater scope than the reference if it is automatically allocated, or that it is not deallocated if it is dynamically allocated.

For what you are trying to do it probably makes more sense to store a pointer in the class and have the class be responsible for deallocation. I wouldn't go with a smart pointer or static reference because really this is a simple problem that can be solved with proper allocation and deallocation via a pointer.

If you switched to a pointer you would add a delete call to A's destructor and your instantiation would look like this:

A a(new B(1));
Chris
Well, as far as I see it would work in the case of calling 'A a(new B())' but won't work properly if 'B b(); A a(b);' called because destructor of b is called twice in such case: once from a and second from scope of existence 'B b();'
Nick
+1  A: 

This is a standard issue, don't fear.

First, you could retain your design with a subtle change:

class A
{
public:
  A(B& b): m_b(b) {}
private:
  B& m_b;
};

By using a reference instead of a const reference, the compiler will reject the call to A's constructor that is made with a temporary because it is illegal to take a reference from a temporary.

There is no (direct) solution to actually retain the const, since unfortunately compilers accept the strange construct &B() even though it means taking the address of a temporary (and they don't even shy to make it a pointer to non-const...).

There are a number of so-called smart pointers. The basic one, in the STL is called std::auto_ptr. Another (well-known) one is the boost::shared_ptr.

Those pointers are said to be smart because they allow you no to worry (too much) about the destruction of the object, and in fact guarantee you that it WILL be destroyed, and correctly at that. Thus you never have to worry about the call to delete.

One caveat though: don't use std::auto_ptr. It's a mean beast because it has a unnatural behavior regarding copying.

std::auto_ptr<A> a(new A());  // Building
a->myMethod();                // Fine    

std::auto_ptr<A> b = a;       // Constructing b from a
b->myMethod();                // Fine
a->myMethod();                // ERROR (and usually crash)

The problem is that copying (using copy construction) or assigning (using assignment operator) means transfer of ownership from the copied toward the copying... VERY SURPRISING.

If you have access to the upcoming standard, you can use std::unique_ptr, much like an auto pointer except from the bad behavior: it cannot be copied or assigned.

In the mean time, you can simply use a boost::shared_ptr or perhaps std::tr1::shared_ptr. They are somewhat identical.

They are a fine example of "reference counted" pointers. And they are smart at that.

std::vector< boost::shared_ptr<A> > method()
{
  boost::shared_ptr<A> a(new A());        // Create an `A` instance and a pointer to it
  std::vector< boost::shared_ptr<A> > v;
  v.push_back(a);                         // 2 references to the A instance
  v.push_back(a);                         // 3 references to the A instance
  return v;
}                                         // a is destroyed, only 2 references now

void function()
{
  std::vector< boost::shared_ptr<A> > w = method(); // 2 instances
  w.erase(w.begin());                               // remove w[0], 1 instance
}                                                   // w is destroyed, 0 instance
                                                    // upon dying, destroys A instance

That's what reference counted means: a copy and its original point to the same instance, and they share its ownership. And as long as there is one of them still alive, the instance of A exists, being destructed by the last one of them to die so you don't have to worry about it!!

You should remember though, that they do share the pointer. If you modify the object using one shared_ptr, all its relatives will actually see the change. You can do a copy in the usual mode with pointers:

boost::shared_ptr<A> a(new A());
boost::shared_ptr<A> b(new A(*a)); // copies *a into *b, b has its own instance

So to sum up:

  • don't use an auto_ptr, you'll have bad surprises
  • use unique_ptr if available, it's your safer bet and the easiest to deal with
  • use share_ptr otherwise, but beware of the shallow copy semantics

Good luck!

Matthieu M.