Message Queue for inter-thread communication

Question

I wrote this little piece of code, a while back. The intention behind it was to create a system to send messages between consumer and producer threads. I have no idea for what i wanted to use it. I just found it in an old snippet folder of mine. The type of the queue is FIFO.

The usage
Create a queue with new MessageQueue<MessageType>().

let's assume we will store an instance of the message queue in an private field inside the classes that will be using it.
This field is called messageQueue

1. Use messageQueue.push(new MessageType([...])) in a producer thread to add a Message.
2. Take it out of the queue from a consumer thread with messageQueue.pull().
3. Check if there is a message in the queue with messageQueue.hasMessage().

I know that I thought about implementing a messageQueue.peek() but I never implementet it.

The code

/**
 * Holds a queue of messages for inter-thread communication
 * Producer and Consumer should <b>not</b> be the same thread!
 * 
 * @param <T> type of the messages
 */
public class MessageQueue<T> {
private static final int MAX_SIZE = 255;
 private T[] messages;
 private int lastReadIndex;
 private int lastInsertIndex;
 private int adjustIndex(int index) {
 if (index >= MAX_SIZE) {
 return 0;
 }
 return index;
 }
 /**
 * Constructor of the {@link MessageQueue}
 */
 @SuppressWarnings("unchecked") // Java can't create an Array of a generic type...
 public MessageQueue() {
 messages = (T[]) new Object[MAX_SIZE];
 }
 /**
 * Pushes a new entry into the queue.
 * This function will block when the queue is full.
 * 
 * @param msg The entry to push on the stack
 * @throws InterruptedException 
 */
 public synchronized void push(T msg) throws InterruptedException {
 int newIndex = adjustIndex(lastInsertIndex + 1);
 // Check if we can push the message
 while (messages[newIndex] != null) {
 wait();
 }
 // Push the message
 messages[newIndex] = msg;
 lastInsertIndex = newIndex;
 notifyAll();
 }
 /**
 * Pulls an entry from the queue.
 * The entry will be returned and removed from the queue.
 * 
 * @return The entry from the queue
 */
 public synchronized T pull() {
 int newIndex = adjustIndex(lastReadIndex + 1);
 T shouldReturn = messages[newIndex];
 messages[newIndex] = null;
 lastReadIndex = newIndex;
 notifyAll();
 return shouldReturn;
 }
 /**
 * Checks if there is an entry in the queue.
 * 
 * @return <code>true</code>, when there is an entry in the queue
 */
 public synchronized Boolean hasMessage() {
 if (lastInsertIndex > -1) {
 int newIndex = adjustIndex(lastReadIndex + 1);
 if (messages[newIndex] != null) {
 return true;
 }
 }
 return false;
 }
}

• mcgyver5: it would be easier to test and understand if you could post some of the code that would call the code under review.
• Mischa: Right above the code, there is a little part called"The Usage". Isn't that enough? I can expand it, if needed.

Answer 1

Nothing in-depth, just a few things that threw me off as a developer that would be using your class:

1. If you design a queue, use proper interfaces.
   There are a few interfaces in Java, designed to offer standardized access to a Queue

   public class MessageQueue<T> { // A queue should implement the Queue<E> interface, preferably BlockingQueue<E>
   

2. Mention constants in your documentation, together with their reason.
   I have a hard time understanding your limit of 255 messages. As a developer that cannot look at private variables, I would be thrown off with errors once I go over that limit. Document it. Tell others about this limitation!

   private static final int MAX_SIZE = 255; // Mention this constant in your documentation
   

3. Is there a reason to use the Boolean object type in favor of the boolean primitive? Using Boolean involves unnecessary boxing and might, if used excessively and in time-sensitive applications, introduce lower performance.

   public synchronized boolean hasMessage() { // Use primitives when objects are not necessary
   

This isn't meant as an exhaustive list, but only my subjective opinion.

• Mischa: 1. Yes, I could (and also should) have done that. I ll do it as soon as I have access to my projects again. - 2. As far as I remember, the limit of 255 was far more than enough for my usecase. But i really should have included it into the documentation comments. - 3. No reason for using Boolean instead of boolean. Just a typo. - All in All: Thanks for your input. :)