container of pointers in c++

Question

I am reading the book "an introduction to design patterns in c++ with qt". In chapter 6, (see the book link https://www.ics.com/designpatterns/book/containersofpointers.html), the author is trying to write a library class which is inherited from QList. In the example 6.35 where the addRefItem function is defined. The author has very weird (at least for me) way to deal with the pointers.

void Library::addRefItem(RefItem*& refitem) {

here, the author used a pointer reference *&, he explained "so that null assignment after delete is possible". this is related to the last two lines, I assume.

   QString isbn(refitem->getISBN());
   RefItem* oldItem(findRefItem(isbn));
   if(oldItem==0)
      append(refitem);
   else {
      qDebug() << isbn << " Already in list:\n"
               << oldItem->toString()
               << "\nIncreasing number of copies " 
               << "and deleting new pointer." ;
      int newNum(oldItem->getNumberOfCopies() + refitem->getNumberOfCopies());
      oldItem->setNumberOfCopies(newNum);
      delete refitem;                         
      refitem = 0;                            
   }
}

I don't understand what the last two lines do. Why refitem need to be deleted. It will be destructed anyway after the function return, right? and then why refitem need to be assigned to be 0.

In the removeRefItem function, there is also a similar line, delete ref, see below. Can anyone help me understand all of these? A lot of thanks.

int Library::removeRefItem(QString isbn) {
   RefItem* ref(findRefItem(isbn));
   int numCopies(-1);
   if(ref) {
      numCopies = ref->getNumberOfCopies() - 1;
      if(numCopies== 0) {
         removeAll(ref);
         delete ref;
      }
      else
         ref->setNumberOfCopies(numCopies);
   }
   return numCopies;
}

Any stack allocated object will be destructed when it's scope is exitted. Your **big** misunderstanding is thinking that in the case of a pointer that means the pointer will be deleted. That's not the case. How could it be? You can't assume that any given pointer points to a heap allocated object or even if it does that it's desireable to delete it. What if there is a copy of the pointer which you still want to use? When a pointer is destroyed, nothing happens. — john, Jun 17 '19 at 04:50
great thanks, your are right, I thought delete means the pointer will be destroyed. It turns out it just releases the memory. The pointer is still there until its scope is exitted. — ptr2love, Jun 17 '19 at 18:37
That's the improtant thing about pointers, what happens to the pointer and what happens to the thing being pointed at are different. `delete ptr` does destroy what is being pointed at (and releases the memory) it does nothing at all to the pointer itself. Similarly when the pointer is destroyed it does nothing to what is being pointed at. — john, Jun 17 '19 at 19:44

user4581301 · Answer 1 · 2019-06-17T23:34:41.627

You find this weird. You should. Don't do stuff like this if you can avoid it. In general, Avoid manual memory management like the plague. See if you can replace this with std::shared_ptrs. It will take a bit of work, but the result will be much more robust.

It will be destructed anyway after the function return, right?

No. RefItem*& refitem provides a reference to a pointer, but because a reference is provided, you know that any object passed into the function is not scoped within addRefItem because it came from elsewhere. If it is to be automatically destroyed it will be destroyed in this elsewhere.

I don't understand what the last two lines do. Why "refitem" need to be deleted. It will be destructed anyway after the function return, right? and then why "refitem" need to be assigned to be 0.

You don't know how the object refitem points at was allocated, whether it is automatically or dynamically allocated so you don't know when it will go out of scope, but it won't be automatically destroyed in addRefItem. The usage of refitem, specifically delete refitem;, suggests that it was dynamically allocated. If it wasn't the program is doomed to Undefined Behaviour.

Why is ref's object destroyed? We already have one. Why have two? This code aggregates RefItems that are the same into a single RefItem, maintains a count of the number of times this object has been duplicated, that is stored in the list. The now-redundant object is destroyed. This makes RefItem a reference-counted object.

Code block 1 shows that if the item is already in the list, the provided object is destroyed and freed with delete refitem; and the pointer to it is nulled with refitem = 0; so that it's easier to detect that the object is no more. Should the caller of this function try to use the nulled pointer, Undefined Behaviour will occur, but the vast majority of systems, everything I've worked on over the past 20 years-or-so, will detect the usage as invalid and crash the program.

This I don't quite understand. Rather than nulling the pointer, I would have updated the pointer to point at the item in the list that absorbed and replaced the pointer passed in. A more complete example may explain this choice better.

As an aside, don't use 0 to null a pointer. It's harder to discern what the code is up to with a 0 when compared to a nullptr (C++11 or newer) or a NULL (pre C++11). 0 has many meanings. nullptr has one.

In the removeRefItem function, there is also a similar line, "delete ref", see below. Can anyone help me understand all of these? A lot of thanks.

In the second code sample, removeRefItem destroys and frees ref if the number of outstanding copies, the reference count, is reduced to 0.

Addendum: Why recommend a std::shared_ptr in the preamble to this answer when this code could easily be implemented with std::unique_ptr? Because this code appears to be implementing a reference-counted pointer. I could be dead wrong and in that case std::unique_ptr is the way to go. Simply remove the unique_ptr from the container and let it drop out of scope to let the system handle destruction for you. If this is a system for checking out and in pointers to the same object from a list, std::shared_ptr does all that for you, and does it well and safely. Why kit-bash a std::unique_ptr into doing the job?

shared_ptr? really? I'd use unique_ptr unless there is a good reason to share. — Thomas, Jun 17 '19 at 15:17
Normally I'd agree with you, @Thomas , and think you could rewrite this with a `unique_ptr` and be just as happy. It's the whole deal with the reference counter that suggests folks are checking out and holding pointers, and that sort of behaviour is what `shared_ptr` is intended to make easy. The list can go away, all of the scaffolding around the list can go away.... There's just something weird about this example. When all copies of a book are returned to the library, the book doesn't cease to exist. — user4581301, Jun 17 '19 at 16:01

zar · Answer 2 · 2019-06-17T20:40:41.707

0

The author used a pointer reference *&, he explained "so that null assignment after delete is possible". this is related to the last two lines, I assume.

What this means is if you just pass it as a pointer, you can still do refitem = 0; but that value will not be carried over when the function returns. So passing as pointer reference achieve that.

I don't understand what the last two lines do. Why "refitem" need to be deleted.

The refItem is a pointer (to an allocated memory by design) and it has to be deleted somewhere. Looking at the code, the author is assigning this function the responsibility to delete it.

The key issue is that the author want to make sure when addRefItem() function returns, the value of refitem should be set to null if it was successfully deleted. This wouldn't have been possible if it the pointer was not passed with reference.

edited Jun 17 '19 at 20:40

answered Jun 17 '19 at 15:55

zar

9,241
10
74
145

thanks, now I understand. The author want to make null the pointer which RefItem is copied from outside the function call. Without using reference, it only make Refitem null. – ptr2love Jun 17 '19 at 19:00
@ptr2love The author want to make sure when `addRefItem()` function returns and if has deleted refitem, it;s value would be null. If it's not passed with reference, it's value will be the same that was passed into the function. – zar Jun 17 '19 at 20:36
Please upvote and accept one of the answers as accepted answers if it answers what you were looking for. – zar Jun 17 '19 at 20:37

container of pointers in c++

2 Answers2