Should an IEnumerable iterator on a Queue dequeue an item?

Iterators should always be idempotent, that is, don't modify the queue as you iterate over it There are no guarantees that there won't be two concurrent iterations Edit to address your new comments: When another programmer (such as your future self ;) ) comes along to add features to the code, they may not assume that the iterators are single-use. They might add a log statement that lists what's in the queue just before using it (oops) Another thing I just thought of is that the visual studio debugger will often enumerate your classes for display. That would cause some extremely confusing bugs :) If you're implementing a sub-interface of IEnumerable, and don't want to support IEnumerable, you should throw a NotSupportedException.

Although this will not give you any compile time warnings, the run time error will be very clear, while a strange IEnumerable implementation could waste future you hours.

Iterators should always be idempotent, that is, don't modify the queue as you iterate over it. There are no guarantees that there won't be two concurrent iterations... Edit to address your new comments: When another programmer (such as your future self ;) ) comes along to add features to the code, they may not assume that the iterators are single-use. They might add a log statement that lists what's in the queue just before using it (oops).

Another thing I just thought of is that the visual studio debugger will often enumerate your classes for display. That would cause some extremely confusing bugs :) If you're implementing a sub-interface of IEnumerable, and don't want to support IEnumerable, you should throw a NotSupportedException. Although this will not give you any compile time warnings, the run time error will be very clear, while a strange IEnumerable implementation could waste future you hours.

I agree that Iterators should be idempotent, I am just wondering if a queue is where you can break the mould. In my case (see edit) it wouldn't matter if there were multiple process iterating over the queue and in fact there probably will be. – Bronumski Nov 15 '10 at 23:00 1 It's even simpler than a concurrency problem.

IEnumerables are forward-only, read-only. Modifying the underlying concrete collection can cause IEnumerable implementations to get "lost", and so most of them don't allow it. Take a List, for example.

Plug it into a foreach that removes each element. On the first go-round, index 0 disappears and index 1 becomes index 0. When the enumerator calls MoveNext, it will attempt to move from index 0 to index 1, which is now actually index 2.

This will result in undesired behavior. – KeithS Nov 15 '10 at 23:18 3 @Keith in general that's an excellent point, but in the OP's code, the enumerator implementation doesn't exhibit that particular problem. – Rob Fonseca-Ensor Nov 15 '10 at 23:28 Additionally, one thing that I had not thought of till now is that I don't have to implement the IEnumerable inerface, all I have to do is provide a IEnumerator GetEnumerator() method as the .

Net language supports duck typing, foreach being one or the places. Now the question is this still an abuse? – Bronumski Nov 15 '10 at 23:45 yes it's still abuse dammit :D – Rob Fonseca-Ensor Nov 15 '107 at 0:14.

Absolutely positively there is no way you should be mutating a collection as you iterate it. The whole point of iterators is that they provide a read-only non-destructive view of a collection. It would be deeply surprising to anyone using your code that looking at it changes it.In particular: you do not want examining the state of your queue in the debugger to change it.

The debugger calls IEnumerable just like any other consumer, and if that has side effects, they are executed.

2 Fair point, one that I had not considered. – Bronumski Nov 15 '10 at 23:23 You might be interested to see some of the responses to my other question stackoverflow. Com/questions/4194900/… – Bronumski Nov 17 '10 at 9:28.

I would suggest that you might want to have a method called something like DequeueAll which returns an item of a class that features a GetEnumerator method that represents everything in the queue, and clears the queue (if a queue item is added around the time the iEnumerable is created, the new item should either appear in the present all to AllItemsDequeued and not in the queue, or in the queue but not the present call). If this class implements iEnumerable, it should be constructed in such a way that the returned object will remain valid even after the an enumerator has been created and disposed (allowing it to be enumerated multiple times). If that would be impractical, it may be useful to give the class a name that suggests objects of the class should not be persisted.

One could still do foreach(QueueItem theItem in theQueue.DequeueAll()) {}but would be less likely to (wrongfully) save the result of theQueue. DequeueAll to an iEnumerable. If one wanted maximum performance while allowing the result of theQueue.

DequeueAll to be used as an iEnumerable, one could define a widening cast that would take a snapshot of the DequeueAll result (thus allowing the old items to be discarded).

I like that, thinking outside the box. – Bronumski Nov 15 '10 at 23:36.

I would say no, do it the second way. Not only is this more consistent with the built-in Queue class, but it is also more consistent with the fact that the IEnumerable interface is meant to be read-only. Also, does this really seem intuitive to you?

: //queue is full foreach(var item in queue) Console. WriteLine(item.ToString()); //queue is empty!?

I agree if that is how I was going to use it, use it is a bit of abuse and I think the question I really should be asking myself is should my queue really implement IEnumerable just because the core queue does. As there is no requirement for it I would have to say no. – Bronumski Nov 15 '10 at 23:02 @Bronumski: Since it is a matter of only one line of code, I think the real question is, why not?

I could certainly imagine the ability to enumerate (or especially use Linq on) a queue's elements to be useful, so I really don't see why you wouldn't. – BlueRaja - Danny Pflughoeft Nov 15 '10 at 23:25 Sure for a core API I could see it being useful but when developing from an Agile approach and accepting I am not going to abuse the language I am going to have to call YAGNI. – Bronumski Nov 15 '10 at 23:35 1 @Bronumski: I think you are applying the agile slogan well out of context; for the enormous benefits you gain (including, as so many others have mentioned, debugging benefits), it is definitely worth considering.

If this were an hour's worth of work, I could perhaps see the YAGNI argument as valid... but it's one very clean and repercussion-free line of code, which you already have written! – BlueRaja - Danny Pflughoeft Nov 15 '10 at 23:47 Depends on who you ask :) – Bronumski Nov 15 '10 at 23:52.

I am going to leave the bandwagon behind and say yes. This seems like a reasonable approach. Though I have to make one suggestion.

That is, do not implement this consuming iterator in the GetEnumerator method. Instead call it GetConsumingEnumerator or something similiar. That way it is plainly obvious what is going to happen and the foreach mechanism will not be using it by default.

You will not be the first person to do this. In fact, Microsoft already does this in the BCL via the BlockingCollection class and they even used GetConsumingEnumerator as the method1. I think you know what I am going to suggest next right?2 1How do you think I came up with the name suggestion?2Why not just use BlockingCollection?

It does everything you have asked for and more.

I had noticed the IProducerConsumerCollection interface before but I never noticed the BlockingCollection, thanks for the heads up. – Bronumski Nov 16 '10 at 14:23.

Strictly speaking, queue provides only push-back, pop-front, is-empty and maybe get-size operations. Everything that you add beside that is not a part of a queue, so if you decide to provide additional operations, you don't need to keep to the queue's semantics. In particular, iterators are not a part of the standard queue interface, so you don't need to have them remove the item current being iterated.(As others have pointed out, this would contradict the expectations on iterators as well.).

I think that was part of my problem, I was looking at it from a purest point of view where a queue should only push and pop. Good explanation. – Bronumski Nov 15 '10 at 23:39.

I would just iterate over the contents of the queue - enumerating them (e.g. For display) rather than modifying the collection.

Iterating through the queue should NOT dequeue. This is designed to see examine the content of the Queue not to dequeue. This is also how MSMQ or MQ Series works.

I don't think that it should. It would be very implicit, and it does not communicate that intent to anyone that is used to using the . Net framework.

I'd say the second one. Your enumerator definitely should not change the state of the collection.

I had code where a propperty get was not idempotent... It was such a pain to decode. Please stick with the manual Dequeue. Also you might not be the only one working on the queue, so it could get messy with more then one consumer.

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.