patternjavaMinor
Circular queue implementation
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:
Implicit this
Contrary to the above having explicit
Naming
What is
size:
currentSize:
Iterator
If you want your class to be able to be used in a
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:
You'll notice that the last call to
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 namessize:
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.