HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Circular queue implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationcircularqueue

Problem

I've implemented a simple thread-safe circular queue. I'm looking for code review, optimizations and best practices.

public class CircularQueue {
    int[] queue; 
    int front;
    int rear; 
    int currentSize;
    int size;

    public CircularQueue(int size) { 
        this.queue = new int[size];
        this.front = 0;
        this.rear = 0;
        this.size = size;
        this.currentSize = 0;
    }

    public synchronized boolean add(int x) {

        if (currentSize == size) {
            throw new IllegalStateException("The queue is full: front size: " + rear);
        }

        queue[rear++] = x;
        rear = (rear + 1) % size;
        currentSize++;

        return true;
    }

    public synchronized int peek() {
        if (front == rear) {
            throw new IllegalStateException("The queue is empty");
        }
        return queue[front];
    }

    public synchronized int poll() {
        if (front > rear) {
            throw new IllegalStateException("The queue is empty");
        }
        currentSize--;
        return queue[front++];
    }
}

Solution

Explicit visibility

I don't like relying on standard visibility; explicit declaring takes away all doubts. Instead have this:

private int[] queue; 
private int front;
private int rear; 
private int currentSize;
private int size;


Implicit this

Contrary to the above having explicit this is unneeded and takes away from the readability. You should only use it when there is ambiguity with a parameter name. This results in this:

queue = new int[size];
front = 0;
rear = 0;
this.size = size;
currentSize = 0;


Naming

What is currentSize and how does it differ from size? At first glance I would think they're the same. Some alternative names

size:

  • size



  • maxSize



currentSize:

  • index



  • offset



Iterator

If you want your class to be able to be used in a foreach loop, you could implement Iterator.

Generics

Your datastructure is not generic so you're losing out on a lot of possibilities.

Kaput

Your code isn't working as it is. I'm not entirely sure why (it's 3:30 AM so I'm not going to debug it deeply anymore) but I followed a hunch and this small example demonstrates it:

public static void main(String[] args) {
    CircularQueue cq = new CircularQueue(3);
    cq.add(5);
    cq.add(10);
    System.out.println(cq.poll());
    cq.add(15);
    System.out.println(cq.poll());
}


You'll notice that the last call to poll() will throw you your self defined exception (aka front > rear evaluates to true).

Code Snippets

private int[] queue; 
private int front;
private int rear; 
private int currentSize;
private int size;
queue = new int[size];
front = 0;
rear = 0;
this.size = size;
currentSize = 0;
public static void main(String[] args) {
    CircularQueue cq = new CircularQueue(3);
    cq.add(5);
    cq.add(10);
    System.out.println(cq.poll());
    cq.add(15);
    System.out.println(cq.poll());
}

Context

StackExchange Code Review Q#52057, answer score: 9

Revisions (0)

No revisions yet.