CODEWITHSUNDEEP
c++
user425496
user425496

Reputation: 21

Is it OK to initialize a reference member with dynamically allocated memory?

Am I causing a memory leak here or is it OK to do like this? Should I use a smart pointer member instead of a reference?

class A
{
public:

   A() : b_(*(new B))
   {}

private:
   B& b_;
};

int main()
{
    A a;
    return 0;
}

Upvotes: 2

Views: 219

Answers (4)

Pavel Chernov
Pavel Chernov

Reputation: 1817

Yes, it is allowed in C++.

Of course, it is more safe to use smart pointers.

Then You shouldn't worry about clearing allocated memory in destructor.

Like this:

#include <memory>

class A {
public:
   A() : p_b( new B() )
   {}
   someMethod() {
       return p_b->something();
   }
private:
   std::auto_ptr<B> p_b;
};
int main()
{
    A a;
    return 0;
}

Upvotes: 1

Mike Seymour
Mike Seymour

Reputation: 254691

You certainly are leaking memory; every new needs to be matched with a delete.

Unless there's a good reason for dynamically allocating it, you'd be better off just making b_ an object. Otherwise, use a smart pointer as you suggest, or (not recommended) delete it in your destructor, remembering to make the copy constructor and copy assignment behave correctly. In the last case, it's valid (but a bit unusual) for it to be a reference rather than a pointer.

The decision really comes down to how you want the class to behave when you copy it. In the first case, it will copy the entire object; in the second, it will behave as defined by the smart pointer; in the third, it will behave as defined by the copy constructor/assignment that you implement.

Upvotes: 5

linuxuser27
linuxuser27

Reputation: 7353

The only thing I would add to what has been said, is that even though you are allocating memory in the constructor which means that b will always start with a value, in the destructor I would change slightly what Justin wrote. I always check for NULL before I call delete and then set the pointer to NULL after I free it. In the destructor setting the pointer to NULL is kind of pointless, but is there just for consistency.

class A
{
public:

   A():b_(new B)
   ~A() { if (b) { delete b; b = NULL; } }
   {}

private:
   B* b_;

}

Upvotes: 0

Thanatos
Thanatos

Reputation: 44334

The language allows this, however, as you have it written, there is a memory leak. You have a new, but no corresponding delete - you need to write a destructor here, something like:

A::~A() {
    delete &b_;
}

Now, while this is legal, it is weird. A pointer will do just as well, and will probably convey better what's going on. A smart pointer would have saved you from a leak, and may be applicable.

Upvotes: 1

Related Questions