Can someone please review this code for me. I have not implemented all the methods for simplicity.
Original code:
/**
* Implements a blocking bounded queue from a given non-blocking unbounded queue implementation
*/
abstract class DerivedQueue<E> implements Queue<E>
{
DerivedQueue(Queue<E> queue, int size)
{
if(queue==null | size <0)
{
throw new RuntimeException("Bad Input");
}
fQueue = queue;
fSize = size;
}
Queue<E> fQueue;
int fSize;
int fCnt;
@Override
public boolean addAll(Collection<? extends E> arg0)
{
return false;
}
@Override
public boolean isEmpty()
{
return (fCnt==0);
}
public E remove()
{
if(fCnt==0)
{
try
{
wait();
}
catch (InterruptedException e)
{
throw new RuntimeException("Waiting thread was interrupted during remove with msg:",e);
}
}
E elem = fQueue.remove();
fCnt--;
notifyAll();
return elem;
}
@Override
public boolean add(E elem)
{
if(fCnt == fSize)
{
try
{
wait();
}
catch (InterruptedException e)
{
throw new RuntimeException("Waiting thread was interrupted during remove with msg:",e);
}
}
return fQueue.add(elem);
}
}
Updated code:
/**
* Implements a blocking bounded queue from a given non-blocking unbounded queue implementation
*/
abstract class DerivedQueue<E> implements Queue<E>
{
DerivedQueue(Queue<E> queue, int size)
{
if(queue==null | size <0)
{
throw new RuntimeException("Bad Input");
}
fQueue = queue;
fMaxSize = size;
}
Queue<E> fQueue;
int fMaxSize ;
int fCurrrentCnt;
@Override
public boolean isEmpty()
{
return (fCurrrentCnt==0);
}
public synchronized E remove()
{
while(fCurrrentCnt==0)
{
try
{
wait();
}
catch (InterruptedException e)
{
throw new RuntimeException("Waiting thread was interrupted during remove with msg:",e);
}
}
E elem = fQueue.remove();
fCurrrentCnt--;
notifyAll();
return elem;
}
@Override
public synchronized boolean add(E elem)
{
while(fCurrrentCnt== fMaxSize )
{
try
{
wait();
}
catch (InterruptedException e)
{
throw new RuntimeException("Waiting thread was interrupted during remove with msg:",e);
}
}
boolean bool = fQueue.add(elem);
notifyAll();
return bool;
}
}
Open questions apart from CR:
Would calling
notifyAll()
lead to multiple waiting thread checking thewhile()
condition at the same time .. and hence there is a possibility that before the while gets saitsfied, 2 threads are already out of it causing outOfBound exception?
OR does the methods need to be markedsynchronized
at all, instead I can use asynchronized
block for reading or writing part of the array while the checking of constrained and waiting can remain unsynchronized?Is their a way to notify precisely only the consumer threads[waiting in take] or only the producer[waiting input] threads in this example?
notifyAll
notifies all of them
||
. A single pipe|
is for bitwise OR. – David Harkness Jul 3 '11 at 6:41