c++ Linked List Stack Pop Function Crashing

I am having the worlds most difficult time understanding this linked list stack. I have been given a fixed ".h" file that I am not allowed to edit, and I am to write the push and pop functions. My pop function, however, is crashing the program and I am unsure why. My pop function is as follows:

 int IntStack::pop(){
    int result = -1;
    if(isEmpty()){
        result = head->data; //program crashes here
        Node *temp;
        temp = head->next;
        delete head;
        head = temp;
    }
    return result;
}

My nodes are the following;

struct Node{
     int data;
     Node* next;
};

Any insight as to why this is crashing and how I can fix it would be appreciated. Also, I am not allowed to have a parameter in the function. Thank you

EDIT: I made the changes suggested to the function, and here is the rest of my code.

 #include <iostream>
 #include <fstream>
 #include <string>
 #include "IntStack.h"

 using namespace std;

 bool ArePair(int first,char last){
     if(first == '(' && last == ')') return true;
     else if(first == '{' && last == '}') return true;
     else if(first == '[' && last==']') return true;
     return false;
 }

 int main(){
     string fileName;

     cout << "Hello, please enter a filename: ";
     cin >> fileName;

     ifstream inFile;

     inFile.open(fileName.c_str());

     while (!inFile){
         cerr << "ERROR: Cannot open " << fileName << ". Please re-enter.\n";
         cin >> fileName;
         inFile.open(fileName.c_str());
     }

     string current;
     IntStack iStack;
     int par; //parenthesis

     while(inFile){
        inFile >> current;

        for(int i=0;i<current.length();i++){
             if(current[i] == '('||current[i] == '{'||current[i] == '['){
                 par = current[i];
                 iStack.push(par);
                }
             else if(current[i] == ')'||current[i] == '}'||current[i] == ']'){
                 if(!iStack.isEmpty() || !ArePair(par,current[i])){
                     cout << "debug\n";
                 }
                 else{
                    iStack.pop();
                 }
             }
         }
     }



     inFile.close();

 }

I think i may have an invalid pointer on the head of my pop function, but i am unsure. Or perhaps its my push function?

 void IntStack::push(int data){
     assert(!isFull());
         Node *temp = new Node;
         temp->data = data;
         temp->next = NULL;
         temp = head;
 }

1 answer

  • answered 2017-12-06 01:39 Jack Of Blades

    What are you trying to do in that code exactly? You could've just done int result = head->data; instead of the whole rigamarole with temp, which you just remove in the end. Also I reckon you never really remove head the way I suppose you want to.

    I'm guessing this is what you were asking about:

    int IntStack::pop(){ 
       int result = -1;             //Assume there is no element to pop
       if(!this->isEmpty()){        //if the stack is not empty,
            result = head->data;    //save the head's data in result
            Node *temp;             //make a temp node
            temp = head->next;      //point it to the node after the head
            delete head;            //remove the head
            head = temp;            //make the one after it the new head
        }
        return result;              //return the result
    }
    

    EDIT:

    You're missing the head node and the tail node in your code. When you have the tail node, the push should look like this:

     void IntStack::push(char data){
         if(!this->isFull()){
             Node *temp = new Node;
             temp->data = data;
             tail->next = temp;
             temp->next = NULL;
         }
     }
    

    ArePair has a loose return at the end you ought to regulate like this:

     bool ArePair(int first,char last){
         if(first == '(' && last == ')') return true;
         else if(first == '{' && last == '}') return true;
         else if(first == '[' && last==']') return true;
         else return false;
     }
    

    I think you'll find this more useful since you know the rest of your code yourself, and there are some basic constructs to cover here. https://www.codementor.io/codementorteam/a-comprehensive-guide-to-implementation-of-singly-linked-list-using-c_plus_plus-ondlm5azr