Search code examples
c++templatesheap-memorydestructor

Can't delete a heap instance of my custom class (C++)


Here's what I've got:

class MyClass {
    int holder;
public:
    MyClass() {
        holder = 5;
    }
};

template<class T>
class First {
    std::vector<T> items;
public:
    First() {
        T* tmp;
        for (int i = 0; i < 20; i++) {
            tmp = new T();
            items.push_back(*tmp);
        }
    };
    ~First() {
        for (int i = 0; i < 20; i++) {
            delete items.at(i);
        }
    };
};

class Second {
    std::vector<std::deque<First<MyClass>>> items;
public:
    Second() {
        std::deque<First<MyClass>>* tmp;
        for (int i = 0; i < 10; i++) {
            tmp = new std::deque<First<MyClass>>;
            items.push_back(*tmp);
        }   
    };
    ~Second() {
        for (int i = 0; i < 10; i++) {
            for (int j = 0; j < items.at(i).size(); j++) {
                delete items.at(i).at(j); // this deletes the "First" instances
            }
            delete items.at(i); // this deletes the deque
        }
    };
};

In my main, I'm create an instance of Second and adding First instances to it (through methods that aren't included). At the end of main, I delete the instance of Second, which should delete all the instances of First and the deques. However, I'm getting the following errors:

error: cannot delete expression of type 'value_type' (aka 'MyClass')
error: cannot delete expression of type 'value_type' (aka 'First<MyClass>')
error: cannot delete expression of type 'value_type' (aka 'std::deque<First<MyClass>>')

Essentially, all my delete commands are throwing errors. What am I missing here? I need to manually implement a destructor because I created a bunch of stuff on the heap - correct?


Solution

  • You are never storing the result of the new expression. Your code should perhaps look like this:

    First() {
        T* tmp;
        for (int i = 0; i < 20; i++) {
            tmp = new T();
            items.push_back(*tmp);
            delete tmp;              // result of "new" is still accessible here
        }
    }
    
    ~First() { }
    

    Or like this:

    First() {
        for (int i = 0; i < 20; i++) {
            items.push_back(T());
        }
    }
    

    Or just like this:

    First() : items(20) {}