Unhandled Exception error with Stack Pop() function

514 views Asked by At

I'm working on a homework assignment involving linked-lists and I'm having trouble with the Pop() function. Here's my code:

void CardStack::Pop( )
{
    if ( m_top->m_next == NULL )
    {
        delete m_top;
        m_top = NULL;
    }

    else
    {
        Node* ptrNode = m_top;

        // gets 2nd to last node
        while ( ptrNode->m_next != m_top )
        {
            ptrNode = ptrNode->m_next;
        }
        // delete last node, then set second to last to null
        delete m_top;
        m_top = NULL;

        ptrNode->m_next = NULL;
        m_top = ptrNode;
    }
    m_size--;
}

I keep getting this runtime error when I run the program, breaking at the if statement:

Unhandled exception at 0x008e2009 in Assignment 7 - CS201.exe: 0xC0000005: Access violation reading location 0x00000004.

A friend of mine recommended removing m_top = NULL from the program, which makes the if statement work. I'd rather keep the null value intact, but it got the program moving. Anyways, it then breaks on the while loop with this error:

Unhandled exception at 0x00b91f4a in Assignment 7 - CS201.exe: 0xC0000005: Access violation reading location 0xfeeefeee.

I tested it with a cout statement, and the error is coming from the body of the while loop; ptrNode = ptrNode->m_next. I'm not really sure how to get around this. Any advice would be greatly appreciated.

Also, here's most of the rest of my program, if it helps:

Header:

class Node
{
public:
    friend class CardStack;

private:
    Node* m_next;
    int m_data;
};

class CardStack
{
public:
    CardStack();
    ~CardStack();

    void Push( int value );
    void Pop();
    int Top();
    int GetSize();

private:
    int m_size;
    Node* m_top;
};

Constructor, destructor, Push():

#include <iostream>
#include "CardStack.h"
using namespace std;

CardStack::CardStack( )
{
    m_top = NULL;
    m_size = 0;
}

CardStack::~CardStack( )
{
    while ( m_top != NULL )
    {
        Pop( );
    }
}

void CardStack::Push( int value )
{
    //creates new node
    Node* newNode = new Node;
    newNode->m_data = value;
    newNode->m_next = NULL;

    // tests if the list is empty
    if ( m_top == NULL )
    {
        m_top = newNode;
    }
    // if the list does contain data members and a last node exists
    else
    {
        m_top->m_next = newNode;
        m_top = newNode;
    }
    m_size++;
}

Main:

#include <iostream>
#include <string>
#include <fstream>
#include "CardStack.h"
using namespace std;

void loadDeck ( ifstream&, CardStack& );
void playGame ( CardStack& );

void loadDeck ( ifstream& inFile, CardStack *card )
{
    int currentCard;

    while( !inFile.eof() )
    {
        inFile >> currentCard;
        card->Push( currentCard );
    }
}

void playGame( CardStack *card )
{
    int score[ 4 ];
    int player;
    while( card->GetSize() != 0 )
    {
        player = card->GetSize() % 4;
        score[ player ] += card->Top();
        card->Pop();
    }

    for ( player = 0; player < 4; player++ )
        cout << "Player " << player << "'s score is " << score[ player ] << endl;
}

int main()
{
    CardStack *card = new CardStack;
    string fileName;
    ifstream inFile;

    do
    {
        cout << "Input the name of the card file (not including extension): ";
        cin >> fileName;

        fileName += ".txt";

        inFile.open( fileName );

        if ( !inFile.is_open() )
            cout << "File could not be opened. Reenter the file name." << endl;
    }
    while( !inFile.is_open() );

    loadDeck( inFile, card );
    playGame( card );

    inFile.close();

    system( "pause" );
    return 0;
}
2

There are 2 answers

1
yosim On

Is this a circular linked list? if not, why are you testing in the loop :

while ( ptrNode->m_next != m_top )

ans not:

while ( ptrNode->m_next != NULL )

If it is, why are you setting:

ptrNode->m_next = NULL;

and not to the "original" m_top->next??

0
Daniel King On

You have only make sure the first time in the loop

while ( ptrNode->m_next != m_top )
    {
        ptrNode = ptrNode->m_next;
    }

ptrNode will not be NULL, because you have check it in the if block, but when the second time in the loop, ptrNode may be NULL, `cause now it is actually

m_top->m_next->m_next

But how can you make sure m_top->m_next is not NULL, when it's NULL, it will crash

Maybe you should changed it to

while ( ptrNode->m_next != NULL )
{
    ptrNode = ptrNode->m_next;
}

Additional, there is another problem, in you push function

else
{
    m_top->m_next = newNode;
    m_top = newNode;
}

should be

else
{
    newNode->m_next = m_top;
    m_top = newNode;
}