Confused about deleting pointers in stack

Question

I'm trying to free up memory whenever possible, but I keep triggering breakpoints when calling delete. It's been awhile since I've done c++ so I don't remember the exact way to do this.

main.cpp

int _tmain(int argc, _TCHAR* argv[])
{
    LIFO lifo = *new LIFO();

    LIFO::Element element1  = *new LIFO::Element(1);
    LIFO::Element element2  = *new LIFO::Element(2);
    LIFO::Element element3  = *new LIFO::Element(3);
    LIFO::Element element4  = *new LIFO::Element(4);
    LIFO::Element element5  = *new LIFO::Element(5);
    LIFO::Element element6  = *new LIFO::Element(6);

    lifo.Push(element1);
    lifo.Push(element2);
    lifo.Push(element3);
    lifo.Push(element4);
    lifo.Push(element5);
    lifo.Push(element6);

    lifo.Peek(lifo.mSize);

    lifo.Pop();
    lifo.Pop();
    lifo.Pop();

    lifo.Peek(lifo.mSize);

    lifo.Push(element1);
    lifo.Push(element2);
    lifo.Push(element3);
    lifo.Push(element4);
    lifo.Push(element5);
    lifo.Push(element6);

    lifo.Peek(lifo.mSize);

    lifo.Empty();

    lifo.Peek(lifo.mSize);

    return 0;
}

LIFO.h

class LIFO
{
public:

    unsigned int mSize;

    LIFO(void)
        :mSize(0)
    {
        mTop = nullptr;
    }


    ~LIFO(void)
    {

    }

    class Element
    {
    public:
        Element* mNextElement;
        int mData;

        Element()
            :mData(0)
            ,mNextElement(nullptr)
        {

        }

        Element(int data)
            :mData(data)
            ,mNextElement(nullptr)
        {

        }

        ~Element()
        {

        }

    };

    Element* mTop;

    void Push(Element& element)
    {
        if(mTop != nullptr)
        {
            Element* newElement = &element;
            newElement->mNextElement = mTop;
            mTop = newElement;

            delete newElement;
            newElement = nullptr;
        }
        else
        {
            mTop = &element;
        }

        ++mSize;
    }

    void Pop()
    {
        if(mTop != nullptr)
        {
            Element* oldElement = mTop;
            mTop = mTop->mNextElement;
            --mSize;

            delete oldElement;
            oldElement = nullptr;
        }
    }

    void Empty()
    {
        while(mTop != nullptr)
        {
            Pop();
        }
    }

    void Peek(unsigned int depth)
    {
        Element* current = mTop;

        for(unsigned int i = 0; i < depth; ++i)
        {
            if(current != nullptr)
            {
                std::cout << current->mData << '\n';

                if(current->mNextElement != nullptr)
                {
                    current = current->mNextElement;
                }
                else
                {
                    break;
                }
            }
            else
            {
                break;
            }
        }

        delete current;
        current = nullptr;
    }

};

I also tried these in the destructors at one point and they didn't work right either

~LIFO(void)
{
    Element* current = mTop;

    while(current != nullptr)
    {
        Element* next = current->mNextElement;
        delete current;
        current = next;
    }

    current = nullptr;
}

How do I safely free up memory in this stack?

`void Push(Element& element)` why don't you take Element as pointer instead ? — Jagannath, Jan 12 '15 at 06:09
You may get Your Solution from here.. [link](http://stackoverflow.com/questions/16532712/template-stack-and-lifo-c) — Hardik Bharadava, Jan 12 '15 at 06:11
Lines like `LIFO lifo = *new LIFO();` clearly indicate that you need to go back to the basics. Working through the examples in a text book or a site like http://www.tutorialspoint.com/cplusplus/ would be good for your C++ skills. — R Sahu, Jan 12 '15 at 06:13
"I keep triggering breakpoints when calling delete." I think "breakpoint" is now what you mean here. — John Zwinck, Jan 12 '15 at 06:17

score 1 · Accepted Answer · answered Jan 12 '15 at 06:26

Your understanding of Peek is wrong. Peek doesn't delete any elements in the stack. Remove the delete statement in Peek. The interface is still wrong though.

Coming to Push, try this modified Push.

void Push(Element* element)
{
    if(mTop != nullptr)
    {
        Element* currTop = mTop;            
        element->mNextElement = currTop;
        mTop = element;            
    }
    else
    {
        mTop = element;
    }
    ++mSize;
}

Now the usage of your class should be

LIFO* lifo = new LIFO();
LIFO::Element* element1  = new LIFO::Element(1);

Your code in the destructor ~Lifo() is fine. After you're done with your stack operations, delete the lifo using the statement delete lifo;.

A suggestion is, don't use raw pointers to own the memory use smart pointers like `std::unique_ptr`, `std::shared_ptr` even if it meant for learning purpose. — Jagannath, Jan 12 '15 at 07:04

Confused about deleting pointers in stack

1 Answers1