container of pointer destructor

[Vasthor]

Click Here

"Having a container of pointers raises the question of how to destroy it. If you simply "do the right thing" and define a FilmList destructor that visits each of its pointers and deletes it, you must then worry about client code that contains a function with a FilmList value parameter. Recall that each value parameter causes a copy of the corresponding argument to be made. That copy is destroyed when the function returns. How should you deal with copying and destroying FilmList objects?"
- Intro to design patterns in c++ with qt 2nd edi.

if the function param did take a copy, and it would destroy the each copied object's pointer, how it should bother with original pointer? even I test it with a simpler replica of this design shown that the pointer in the original one is not deleted. Tested by printing out the current value of the pointer which shown it's address after each call to value-parametered function is called on it. The result is the same everytime which mean the pointer is not deleted / re-assigned. So what does the original question even mean?

simple replica:

class FractionList : public QList<Fraction*> {
public:
    ~FractionList() {
        qDeleteAll(*this);
        clear();
    }
};

btw, I'm really bothered of the book author decision on inheriting every single time as possible. It's a good habit?

[Vasthor]

bump

[ravenous]

Hmm, I'm not sure what your original question is asking exactly. I do have some comments on your snippet though.

1. If you use QList to hold stuff, then the QList is actually holding pointers to the stuff for you. So, if own the objects in the list, then you should just have a QList of the objects themselves. So, not QList< Fraction* >, but QList< Fraction > will do the job.

2. Don't inherit from QList, it's not designed to be used that way. You can tell this because it doesn't have a virtual constructor. If you publicly inherit from something without a virtual destructor, you risk something called "object slicing".

I don't know if that's any help, but I hope it's interesting :)

[mike_2000_17]

How should you deal with copying and destroying FilmList objects?

The answer that this question is begging for is "deep copy". You can read more about it in my tutorial.

even I test it with a simpler replica of this design shown that the pointer in the original one is not deleted. Tested by printing out the current value of the pointer

When pointers are "deleted", what it really means is that the memory they point to is released, i.e., it is made available again for future objects or arrays being allocated dynamically. Deleting a pointer does not change its value (it still points to the same place) and it generally does not change the memory it points to either (i.e., if you print the content of the memory immediately after deleting it, it is probably the same). The only thing that changes is that later on, the same memory could be allocated to other things.

So, once you've deleted a pointer, you no longer have the right to access that memory. It is no longer yours.

So, if you simply copy the container of pointers, and delete all the pointers of the original one, then your copy will have to bunch of pointers to forbidden memory, and anything you do with them is illegal / ill-defined / undefined behavior / really really bad!!!! In other words, that copied container is complete garbage.

So what does the original question even mean?

The original question tries to get you to understand how to avoid the pitfall. You didn't get the question because you didn't understand what this pitfall was about.

btw, I'm really bothered of the book author decision on inheriting every single time as possible. It's a good habit?

You are right to be bothered by that. It is a really terrible habit. I would not let such code pass any kind of code review. You should never inherit code like this, especially not standard containers (or similar, like Qt containers). No one with decent coding standards would condone this.

The general saying is "inherit, not to reuse, but to be reused!". It's well explained here.

[Vasthor]

Been read your tutorial in the past. I would have used the RAII principle instead (or just use the smart_pointer), but not answering the question.

It's my fault though i've been forgetting how pointer is managed. been programming with other (non-pointer) language for a while.
But, this should have caught it though (based on given link also show the bug appear), well, it's "undefined" anyway, sigh.

int main() {
    Fraction frac1(1,2), frac2(1,3), frac3(2,3);

    FractionList duh;
    duh.push_back(&frac1);
    duh.push_back(&frac2);
    duh.push_back(&frac3);

    std::cout << sumFrac(duh) << std::endl;  // sumFrac do value copy.
    std::cout << **duh.begin() << std::endl;
    std::cout << *duh.begin() << std::endl;
    std::cout << frac1 << std::endl;
    std::cout << &frac1 << std::endl;
    std::cout << std::endl;

    std::cout << sumFrac(duh) << std::endl;
    std::cout << **duh.begin() << std::endl;
    std::cout << *duh.begin() << std::endl;
    std::cout << frac1 << std::endl;
    std::cout << &frac1 << std::endl;
    std::cout << std::endl;

    duh.push_back(&frac2);
    std::cout << sumFrac(duh) << std::endl;
    std::cout << **duh.begin() << std::endl;
    std::cout << *duh.begin() << std::endl;
    std::cout << frac1 << std::endl;
    std::cout << &frac1 << std::endl;

    return 0;
}

but nah, It seems pointer-based bug doesn't simply being detected. I always trying to debug this kind of silly error myself. This error being passed on for weeks already till I decided to put it here. How do you detected any error that is pointer-based.

[mike_2000_17]

How do you detected any error that is pointer-based.

You use a memory debugger / profiler tool. Some IDEs have such a tool integrated (e.g. Visual Studio), but I prefer to use Valgrind. It will supervise the execution of the program (it's a virtual machine) and tell you if you have memory leaks (forgot to delete memory), heap corruption (delete memory twice or from the wrong heap), and if you access any memory that was not allocated, or was freed, or is otherwise not supposed to be accessed.