Are heaped links a terrible idea? What for?

To better explain this problem, I built a simple example. Let's say I have a Blob class as follows:

 class Blob { string personalName; string& familyName; } 

A Blob can be spawned by the Creator (aka Programmer), after which he gets the choice of personalName , and since he has the privilege of being the 1st generation of Blob , he gets his own familyName .

Alternatively, a Blob can be created by creating an existing Blob , after which it selects its own personalName , but shares a familyName with all the other Blob that have been cloned into this family. If one Blob changes the name, all other family members automatically change this name.

While this sounds good and good, until the Blob constructor is written, I see this:

 Blob::Blob() : personalName(pickName()), familyName(pickFamilyName()) { } ... string& Blob::pickFamilyName() { return *(new string("George")); } // All Blobs have family name "George" in this example 

Ik! Allocating memory on the heap and then assigning it a reference variable ?! It looks scary!

Are my instincts correct that something is wrong with this, or does it just seem strange to me, because this is not a common template? If something is wrong, what is it? Why is this a bad design?

Note. It would be important to free the memory allocated by the heap by counting and deleting the memory when deleting the last Blob or in some other way.

+7
source share
3 answers

The only time it makes sense to store a link as a member of a class when:

  • The data does not belong to the class, and the class is not responsible for its release;
  • The lifetime of the mentioned object is guaranteed longer than the lifetime of the containing object.

Your example violates rule 1.

+14
source

I think the "smelly" part of this file stores this variable as a string reference, as it can be difficult to keep track of whether it is a valid object. Why not use something like:

 boost::shared_ptr<std::string> Blob::pickFamilyName() { return boost::shared_ptr<std::string>(new std::string("George")); } 

EDIT

According to Praetorian's suggestion, you may not have to allocate memory at all manually:

 boost::shared_ptr<std::string> Blob::pickFamilyName() { return boost::make_shared<std::string>("George"); } 
+8
source

One argument will be as follows: operator new returns a pointer, and operator delete accepts a pointer, so it is expected that the type used to refer to dynamically allocated objects will also be a pointer, not a link. This is a serious argument: if you are inconsistent and disagree with the habits of programmers for a good reason, you mix them and make mistakes. Usually no one expected a function that returns a link to create new objects on the heap that the calling code should delete, so sooner or later someone will forget to do it.

But there are pragmatic reasons that pointers can be reassigned and that they can be set to null, which makes them more convenient for processing dynamically distributed objects. In your example, the Blob class is responsible for calling delete on the reference element. Usually you do this in a destructor. But imagine that you want to free up memory earlier: with pointers you can assign them null assignments after calling delete, and then let their destructor safely call delete again, and with member elements you will remain with a sagging link that you cannot make nothing about.

An even more serious problem is exception safety: if Blob had a longer list of initializers or a non-empty body, the constructor might throw after calling pickFamilyName (). In this case, the destructor is not called, and you have a memory leak. Ideally, you would use RAII for this, but with pointers you can also assign a pointer to zero in the initializer list, and then point it to the newly created object in the constructor body in the try / catch block, which ensures the object is deleted even if the constructor throws and no call to the destructor. It is also impossible to do with links.

+1
source

All Articles