5
\$\begingroup\$

Please review the code:

    package com.gmail.practice;

import java.util.Arrays;

public class Queues {

    int head;
    int tail;
    int size;
    int[] queue;

    public Queues(int arraysize)
    {
        if(arraysize<0)
        {
            throw new IllegalArgumentException("size cannote be less than equals zero");
        }
        head = -1;
        tail = -1;
        size = arraysize;
        queue = new int[size];
    }

    public boolean isEmpty()
    {
        return(tail == -1 && head == -1);
    }

    public void enqueue(int value)
    {
        if((tail+1)%size == head)
        {
            System.out.println("the queue is full");
        }else if(isEmpty())
        {
            head++;
            tail++;
            queue[tail] = value;
        }else{
            tail = (tail+1)%size;
            queue[tail] = value;
        }
    }

    public int dequeue()
    {
        int value = 0;
        if(isEmpty())
        {
            System.out.println("the queue is empty cant dequeue");
        }else if(tail == head){
            value = queue[head];
            head = -1;
            tail = -1;
        }else{
            value = queue[head];
            head = (head+1)%size;
        }

        return value;
    }

    public void display()
    {
        System.out.println(Arrays.toString(queue) + " " + "tail is " + " "+ tail +" the head is" + " " + head);
    }

    public static void main(String[] args)
    {
        Queues q = new Queues(5);
        q.enqueue(3);
        q.enqueue(4);
        q.enqueue(2);
        q.enqueue(1);
        q.enqueue(5);
        q.display();
        q.dequeue();
        q.dequeue();
        q.display();
        q.enqueue(1);
        q.enqueue(5);
        q.dequeue();
        q.dequeue();

    }
}
\$\endgroup\$
4
\$\begingroup\$
  • I think the value variable in the dequeue method is potentially destructive. rather than printing "the queue is empty cant dequeue", and returning the default value of the value variable, I think it's better to throw an exception and catch it. For example on why it is destructive:
    if ( q.dequeue() == 5) // this will return 0 if the array is empty.
    This is not good. The user will think the value that is being returned is 0 though it's already empty.
  • You initialized the size variable, but you didn't really do anything to it in your queue operations (E.G. size-1 when you dequeue an item). It doesn't make sense for the size variable to not change if you enqueue/dequeue items from the queue. But I'm not sure why it's there yet you don't have a size() method. If you're not going to implement a size method() (you should), I think the size variable is useless.
  • I think it would be more flexible if the array is resizable (E.G. size*2 if full, size/4 if too large).
  • Make it generic so you can add other types.
\$\endgroup\$
3
\$\begingroup\$

So adding to what morbidCode said. If I where you I would leave the tail exclusive making your code much more simple. You don't have to check if its empty on the enqueue operation among other things. (The tails would be used as a dummy). This way you can start with the tail and the head equal to zero.

Enqueue

You should throw an exception when is full instead, also if the queue can not be resized you should implement a public method isFull (It would be better if the size is not static thought).

Your code would end up like this:

public boolean isFull() 
{ 
    return (tail + 1) % size == head;
}
public void enqueue(int value)
{
    if(isFull())
        //throw some overflow exception 

    ++tail;
    if(isEmpty())
        ++head; 
    else
        tail %= size;

    queue[tail] = value;
}

If you make the tail exclusive you end up with this:

public void enqueue(int value)
{
    if(isFull()) 
        //throw some overflow exception 

    queue[tail] = value;
    (++tail) %= size;                
}

Dequeue

You can change it in a similar way as what I did with enqueue.

public int dequeue()
{

    if(isEmpty())
        //throw some underflow exception

    int value = queue[head];
    if(tail == head)
        head = tail = -1;
    else
        (++head) %= size;

    return value;
}

If you leave the tail exclusive you get:

public boolean isEmpty() 
{
    return head == tail;
}

public int dequeue()
{
    if (isEmpty()) 
        //throw some underflow exception

    int element = queue[head];
    (++head) %= size;
    return element;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.