CODEWITHSUNDEEP
cqueue
PandaBearSoup
PandaBearSoup

Reputation: 699

Queue in C acting strangely after poping all elements

I define my queue like so:

struct Node
 {
  int Data;
  struct Node* next;
 }*rear, *front;

int pop()
{
      struct Node *temp, *var=rear;
      int data = var->Data;
      if(var==rear)
      {
             rear = rear->next;
             free(var);
      }
      else{
      printf("\nQueue Empty");
      }
      return data;
}

void push(int value)
{
     struct Node *temp;
     temp=(struct Node *)malloc(sizeof(struct Node));
     temp->Data=value;
     if (front == NULL)
     {
           front=temp;
           front->next=NULL;
           rear=front;
     }
     else
     {
           front->next=temp;
           front=temp;
           front->next=NULL;
     }
}

When I pop the last element in the queue, I am unable to push more elements.

Results:

1. Push to Queue
2. Pop from Queue
3. Display Data of Queue
4. Exit

5. Empty Choose Option: 1

Enter a valueber to push into Queue: 10
Calling push with 10, temp at 8061420. push done: front = 8061420, rear = 8061420
Elements are as:        10

Choose Option: 1

Enter a valueber to push into Queue: 20
Calling push with 20, temp at 8061430. push done: front = 8061420, rear = 8061430
Elements are as:        20

Choose Option: 2
Elements are as:        20

Choose Option:

Upvotes: 0

Views: 102

Answers (1)

woolstar
woolstar

Reputation: 5083

Starting at the beginning, if you do not set front and rear somewhere to NULL, then none of this works right.

Then lets look at push():

When the list is empty, you set front and rear equal to temp. That's good. When there is a front you set front->next= temp ; This is not good if you already added something else in the list. The idea should be to use rear to always point to the last thing added to the list, and always add to the end of rear.

So you should do:

void push(int value)
{
  struct Node * temp= (struct Node *) malloc( sizeof( struct Node)) ;
  temp-> Data= value ;
  temp-> next= NULL ;
  fprintf(stderr, "Calling push with %d, temp at %lx.\n", value, temp) ;

  if ( rear) { rear-> next= temp ;  rear= temp ; }
    else { front= rear= temp ; }

  fprintf(stderr, "push done: front = %lx, rear = %lx\n", (long) front, (long) rear) ;
}

Similarly, you have things a bit backwards on pop(). You look at rear and also check rear->next. rear->next should always be NULL. Instead, just take the first thing off of front. Worse than that though is that you look at the value of rear before even checking that its empty. This is going to do bad things. So test the pointer for being valid first, then read its value:

int pop()
{
  int retval= -1 ;
  if ( front )
  {
    struct Node * temp= front ;
    front= front-> next ;
    if ( ! front ) { rear= front ; }
    retval= temp-> Data ;
    free( temp) ;
  }
  return retval ;
}

This should get you operating. }

Upvotes: 1

Related Questions