Implementing push/pop in a linked list (C++)

I'm trying to incorporate push/pop into a linked list and I can't seem to get it to work. When I run my test function, I set my linked list to zero and I try to push on values but the list keeps getting returned with no values in it. Could anyone possibly tell me what I'm开发者_StackOverflow中文版 doing wrong?

if (top == NULL){
      current = top;
      current->next = NULL; //NULL->next : will cause segfault
  }

if top is NULL, you set current = top [which is NULL], and then you access current->next, which will cause a segfault, you are trying to access NULL..

EDIT: follow up to comments:
your if statement seems redundant, you should probably only need to set: current->next = head; and head = current; [in addition to the current allocation]

Instead of

 if (top == NULL){
      current = top;
      current->next = NULL;
 }

you want

 if (top == NULL){
      top = current;
      current->next = NULL;
 }

And of course, after this, you have to make sure that you actually set head to top again.

Now that you've made this change, it should be clear that both cases do the same thing -- so no case distinction is actually necessary. So the function can be simplified to

void push(Data * newPushData){
    LinkNode * current = new LinkNode(newPushData);
    current->next = head;
    head = current;
}

The top variable is local variable for push(...) function. You can use head instead, and I'd rather modify the if statement.

I think that function should look like this:

void push(Data * newPushData){
    LinkNode * current = new LinkNode(newPushData);
    if (head != NULL){
        current->next = head;
        head = current;
    }
    else{
        head = current;
        current->next = NULL; // if you haven't done it in LinkNode constructor
    }
}

can you please specify the attributes of the linked list class ? [ is there slightly chance you are doing something wrong]

Instead of you , I'd do :

void push(Data * newPushData){
if (head   == NULL) 
    head->data = newPushData
    tail = head ; 

else // regular situation 
     { 
     Node  * node  = new Node() ; 
     tail->next = node; 
     node->data  = newPushData; 
     node->next  = NULL ;
     tail  = node ; 
   } 

}

In a linked list you have got to maintain the head pointer point on the head of the list , maintain that the tail pointer is point on the tail of the list , You must take care of the 2 cases of enlarging the list. the best way for learning is to illustrate an insertion on a blank linked list.

Take care S

void push(Data * newPushData)
{
    if( head != NULL )
    {
        LinkNode current = new LinkNode(newPushData);
        current->next = head;
        head = current;
    }
    else
    {
        head = new LinkNode(newPushData);
    }
}

Try this code...

void push(data * newpushdata){
    if(head !=null){
        linkednode current = new linkednode(newpushdata);
        current->next = head;
        head = current;
    }
    else {
       head = new linkednode(newpushdata);
    }
}

that is my working solution for a Stack containing int elements, but maybe it's better to create a void pushStack using Stack **S instead of Stack *S.

in pop(Stack **S) i created a sentinel, so if the stack is empty -1 is returned.:

typedef struct StackT {

    int val;
    struct StackT *next;

} Stack;

int isStackEmpty (Stack *S) {

    if (S == NULL)
        return 1;
    else
        return 0;
}


int *pop(Stack **S) {
    Stack *tmp = *S;
    int i = -1;
    if (isStackEmpty(tmp) == 0) {
        i = tmp->val;
        *S = tmp->next;
    }
    return i;
}

Stack *pushStack (Stack *S, int x) {

    Stack *node = (Stack *) malloc (sizeof (Stack));
    node->val = x;
    node->next = S;

    return node;
}

you can call pop and stack easly:

    Stack *S = NULL;
    int x = somevalue;
    int y;
    S = pushStack(S, x);
    y = pop(&S);