Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

I have a loop structure, that I really don't like. It's the initialisation I find appalingly ugly. Is there a nicer way to do the following?

Queue<Integer> queue = getMyQueue();
Integer element = queue.poll();
while (element != null){
  calculate_smart_stuff(element);
  element = queue.poll();
}

A for loop is slightly better, but still has the ugly repetition of the assignment of element in the initialiser and the 'incrementer' I also prefer my for loops to be really easy, and only to be reserved for counting and adding (or substracting in corner cases) 1, as this is how I expect a for loop to behave.

Queue<Integer> queue = getMyQueue();
for (Integer element = queue.poll(); element != null; element = queue.poll()){
  calculate_smart_stuff(element);
}

Or should I just keep checking the size before assignment like so?

Queue<Integer> queue = getMyQueue();
while (queue.size() > 0){
  Integer element = queue.poll();
  calculate_smart_stuff(element);
}

What I don't like about this is that I have to check the size every time before getting an element, which itself has a clear indication that the queue was empty.

Are there any better constructs I can use here, or am I just being picky with the last two options?

share|improve this question

3 Answers

up vote 8 down vote accepted

It would be better to do 

for (Integer element; (element = queue.poll) != null;){

}

Because you don't want element to be in the scope after the loop has ended

share|improve this answer
Definitely better than my for attempt for the for loop. Thanks – Martijn May 31 '11 at 21:38
up vote 6 down vote

Can you not do:

while ((element = queue.poll) != null) {
..
}

?

It requires more knowledge of the language but that might be a trade-off you like.

share|improve this answer
yes, that works, and was about what I was looking for – Martijn May 31 '11 at 20:26
Just a shame that in the while statement, I can't do the declaration like in the for example, but it helps – Martijn May 31 '11 at 20:47
up vote 0 down vote

Since java.util.Queue is iterable...

for (int i : queue)
{
    System.out.println ("" + i);
}

Simplified for-Loop, introduced about 2005.

In the comments, the question is raised, what to do with PriorityQueue, which doesn't give guarantees about its iterator. Well - unfortunately, we can't just write:

for (int i : Arrays.sort (pq.toArray ()))

because Arrays.sort sorts an Array in place, and doesn't return it. So we have to construct the Array beforehand, 

public void test () 
{
    Queue <Integer> queue = new PriorityQueue <Integer> ();
    queue.offer (4);
    queue.offer (41);
    queue.offer (44);
    queue.offer (14);

    for (int i : queue)
    {
        System.out.println ("" + i);
    }

    out.println ("-------------");

    Integer [] arr = queue.toArray (new Integer [queue.size ()]);
    Arrays.sort (arr);
    for (int i : arr)
    {
        System.out.println ("" + i);
    }
}

That is, of course, more boilerplate than in the question. While I ask myself, why an iterator in a PriorityQueue doesn't guarantee an order, and would suggest to do otherwise, if you write your own Queues where you can overwrite 'iterator', where you're interested in the order of iteration, I don't have a suggestion for the trivial case.

share|improve this answer
1  
-1 if I could: This is OK sometimes, but: in a PriorityQueue this won't give you the right ordering, in a BlockingQueue it won't block correctly, and if the queue changes inside the loop (which is actually a pretty common thing to do if the queue is used for BFS), it will throw an exception. – trutheality Jun 1 '11 at 4:49
And above examples are better in that regard? – user unknown Jun 1 '11 at 14:17
poll() removes the head of the queue, the for constuct uses an iterator. The iterator gives no guarantees for order: Returns an iterator over the elements in this collection. There are no guarantees concerning the order in which the elements are returned (unless this collection is an instance of some class that provides a guarantee). (from download.oracle.com/javase/1,5.0/docs/api/java/util/… ) – Martijn Jun 1 '11 at 18:04
Yes, using poll() functions properly in all cases. – trutheality Jun 1 '11 at 18:57
I agree with you that it is counter-intuitive, and quite a cotcha, that the Iterator on the Queue interface doesn't guarantee any specific order. The reason is probably to allow more flexibility on the backend. The PriorityQueue in java.util is backed by a priorityheap, and I'm not sure how easy (or possible) it is to implement an in-order iterator on it. – Martijn Jun 2 '11 at 11:18

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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