TThreadedQueue: interesting, but incomplete
I mentioned the new generic collection TThreadedQueue<T> in my First Look at Delphi XE. I decided to play around with it a little recently. It’s useful for passing data between one thread that produces output and another that consumes it, keeping the two in step by blocking if the consumer tries to pop from the queue while it’s empty.
The first thread goes through and pushes data into the queue however it wants to. The second has it easy; all it has to do is loop endlessly until the queue is shut down. And we all know how to do that:
[code lang="delphi"] for value in queue do process(value); [/code]
Except that if you try to do that, the compiler will complain at you. There’s no enumerator. For some strange reason, out of all the collections in Generics.Collections, TThreadedQueue<T> alone does not descend from TEnumerable<T>.
Oh well. It’s not all that hard to add an enumerator to a class that doesn’t have one. Just use a class helper.
[code lang="Delphi"] TThreadedQueueEnumerator= class private FQueue: TThreadedQueue ; FCurrent: T; function GetCurrent: T; public constructor Create(queue: TThreadedQueue ); property Current: T read GetCurrent; function MoveNext: Boolean; end; TThreadedQueueHelper = class helper for TThreadedQueue public function GetEnumerator: TThreadedQueueEnumerator ; end; implementation { TThreadedQueueEnumerator } constructor TThreadedQueueEnumerator .Create(queue: TThreadedQueue ); begin FQueue := queue; end; function TThreadedQueueEnumerator .GetCurrent: T; begin result:= FCurrent; end; function TThreadedQueueEnumerator .MoveNext: Boolean; begin result := FQueue.PopItem(FCurrent) = wrSignaled; end; { TThreadedQueueHelper } function TThreadedQueueHelper .GetEnumerator: TThreadedQueueEnumerator ; begin result := TThreadedQueueEnumerator.Create(self); end; [/code]
Well, that was easy. That’s probably the simplest enumerator I’ve ever written, because of the way the queue’s design makes it easy to implement MoveNext. Except… that doesn’t compile either. Apparently you can’t put generic type parameters on a class helper, which means that as far as I can tell, you can’t apply a class helper to a generic class at all.
I suppose I could subclass it and add the enumerator that way, but TEnumerableThreadedQueue<T> is a bit of a bulky name, don’t you think? I have to wonder why the enumerator was left off of this collection, though, especially since the standard enumerator pattern is basically the only reasonable way to use a class like this…
Why complicating the problem or dictating one way of process. Using for loop seem be misrepresent the logic. Use while loop is more clear and it works
while Queue.PopItem(Value) = wrSignaled do
Process(Value);
Cheers
Exactly – I’d go so far as to say that this is why ThreadedQueue doesn’t implement an enumerator… not an omission but a deliberate decision. Never mind it being simpler to use a while, it is quite simply not appropriate to “enumerate” a queue. You do not “MoveNext”, you stay where you are (on the 0th element) and the elements move underneath you.
You can of course fudge an enumerator to not be an enumerator and to do something else.
You can also use “with” and “goto” …. (not in this case, but making the point that being able to do something doesn’t mean that that “something” is the right or good thing to do).
And furthermore, re class helpers… I have said before, trying to use them as general purpose utilities in this way, particularly to extend classes in libraries that you did not originate, is always going to run into problems. It is NOT what they were designed for and not what they should be used for.
I recently found what I consider to be the first legitimate use for a class helper and will be blogging about it soon.
[…] I have long held the view that class helpers should not be viewed as a general purpose utility. They were designed for a very specific purpose and the authors of the technology themselves tell us how they are intended to be used. Using them in other ways is asking for trouble: they can break your – or others – code, they hide details of an implementation in A Bad Way™ and you are likely to run into limitations because what you want to use them for doesn’t tally with what they are in…. […]
This is a good example for wasting time in the wrong place.
U tried to transform the queue into a list instead of realizing the point of a queue :o)
My smilie was also transformed … sorry 4 that … 🙂
Pace the other commentators, I don’t see the harm in enabling the use of the for/in syntax here:
1. The statement ‘for Item in Queue do Process(Item)’, in itself, seems perfectly clear to me. The fact that the items are being removed as they are being enumerated doesn’t imply they’re not being enumerated!
2. This isn’t about ‘dictating one way of process’, just adding the option of using a more generic syntax.
3. The fact that internally, a method called ‘MoveNext’ rather than ‘GetFirstAndRemoveIt’ must be defined is an implementation detail – there’s nothing inherent in the pattern that says the elements being enumerated must be static.
4. It isn’t Mason who is conflating queues with simple ‘lists’, but his critics who conflate the sorts of things the for/in syntax *may* loop with what it *must*. In particular, there is no necessity for a for/in loop to have a direct for/to equivalent. Putting for/in loops with for/to ones on one side and while ones on the other is just silly, since a for/in is implemented using a while loop!
As AP says, the point with enabling “for in” being a waste of time here is that:
a) it obfuscates/confuses the workings of a queue – the differences being potentially significant to the understanding of code being read by a human as opposed to being compiled/executed by a machine
b) the aim was to enable a brevity of syntax which was already achievable!
Re your point #4 – the problem is that in 99% of cases, “for in” represents an enumeration of the content of a list. Someone reading “for in aQueue” may be forgiven for thinking that the code intends enumerating the *current* contents of the queue with no allowance for the fact that the queue may be added to in the meantime.
“while NOT queue.Empty” is far clearer in this regard.
Sure, the compiler can’t be confused, but a human being reading the code without full knowledge of the custom classes implemented behind the scenes to facilitate the syntax can *easily* make such a mistake.
“the problem is that in 99% of cases, “for in” represents an enumeration of the content of a list”
Well it’s not in the code I write. The fact that people initially only see for/in as a variant of for/to is simply because they haven’t used it much.
It seems to me that your general complaint is in danger of taking the form ‘I and many others are not used to syntax x, therefore x is inherently confusing’, which is plainly an invalid inference (I am inclined to interpret your criticism of anonymous methods in this light too) – the problem of such reasoning being that it would have been applied to things like exceptions in the past, things that presumably you would find quite natural now.
@Jolyon:
If you want to really get into semantics, then your example of “while NOT queue.Empty” is also wrong. The point of the threaded queue is synchronization. If the consumer reads from the queue faster than the producer produces new input for it, then it’s quite possible for the queue to be empty before its work is done, probably multiple times. Your ending condition is “queue.Empty AND queue.ShutDown”, which is getting a little bulky even before we start worrying about timeouts.
Having an enumerator here is important for consistency and simplicity, not brevity. If you can use a for-in loop on any other kind of collection, but this one requires special syntax, then that’s definitely going to confuse the human being writing the code.
And I don’t really understand the issue you raise about the user “easily” being confused and thinking he’s only enumerating the current contents of the queue. This is a thread-synchronization queue–it says so right in the name of the class–and if our hypothetical user doesn’t understand that that means there can be moving parts at both ends of it at once, then IMO he shouldn’t be playing around with multithreading yet.