JAVA Queue Implementation using Arrays

Question

I'm trying to implement Queues in JAVA. I'm a beginner. I dont understand why this isn't working. Push() works fine but pop() isn't working. Can someone please point out where im going wrong?

pop():

 public void pop()
    {
    for(int i=0;i<length;i++)
    {
         while(i<(length-1))
         {
        arr[i]=arr[i+1];

        }

    }

    }

push():

public void push(int x)
    {
    push:for(int i=0;i<length;i++)
    {
        if(arr[i]==null) 
        {
        arr[i]=x;
        break push;
        }
    }
    }

show():

public void show()
    {
    int c=0;
    for(int i=0;i<length;i++)
        //if(arr[i]!=null) 
        {
            System.out.println(arr[i]);
            c++;

        }
    System.out.println("Current Capacity "+c+"/"+length);       
    }

main()

public static void main(String...i)
    {
        System.out.println("Stack Implementation");
        Queue stack = new Queue();
        System.out.println("Push");
        stack.push(1);
        stack.push(2);
        stack.push(3);
        stack.push(4);
        stack.push(5);
        stack.show();
        System.out.println("Pop");
        stack.pop();
        stack.show();
    }

The output doesn't show any data after pop() is run.

Answer 1

You don't increment i in pop() so the while loop will run endlessly.

In push you are using a for loop which increments i: :for(int i=0;i<length;i++ /*here*/)

You also don't initialize i in pop() so it will probably have the value of the last increment in push(). That value will be the index of the next empty element (if there's one left) and thus that's wrong anyways. However, you want to pop from the front, so you'd need to start at i = 0 - in that case another for loop would work as well, i.e. you just copy the value of element at i+1 to the index i and set the last element to null (for more efficiency you could stop once i+1 has a null element).

Edit: now that you've posted more code for pop() the situation is a little different. You are already using a for loop in pop() but another loop inside that. I assume you want to do if(i<(length-1)) instead of while(i<(length-1)) - but in that case you'd still have to handle the last element, i.e. once the queue was full you'd need to set the last element to null when you pop one and move the rest.

Answer 2

When you pushed the element you need to return from the method:

 public void push(int x) {
   for (int i = 0; i < length; i++) {
     if (arr[i] == null) {
       arr[i] = x;
       return;  // Exit from push when you added the element in the right position
     }
   }
 }

Note that this code is not optimized. Push an element requires O(n), so can waste a lot of time for big queues, but this is the closest solution to your code. Anyway a simple optimization can be done introducing a variable holding the last used index. So you can use that variable to push and pop an element in O(1).