patterncsharpMinor
Simple wrapper to use IEnumerable as a producer in a producer-consumer pattern
Viewed 0 times
simplewrapperconsumerproducerpatternuseienumerable
Problem
I have a relatively simple problem: I have an
As I understand it, this is a classic producer-consumer scenario, so I'm choosing to use a
To support this, I have a simple wrapper:
Any code having an
Interested in a general review, but in particular:
IEnumerable, which takes considerable time to yield each term. This is then used by another piece of code, which takes considerable (but not identical) time to process each term.As I understand it, this is a classic producer-consumer scenario, so I'm choosing to use a
BlockingCollection so that I can produce and consume concurrently.To support this, I have a simple wrapper:
class EnumerableProducer
{
public readonly BlockingCollection ProducerCollection;
public EnumerableProducer(IEnumerable source)
{
ProducerCollection = new BlockingCollection();
AddItemsToBlockingCollection(ProducerCollection, source).Start();
}
private static Task AddItemsToBlockingCollection(BlockingCollection collection, IEnumerable source)
{
return new Task(() =>
{
foreach (var item in source)
{
collection.Add(item);
}
collection.CompleteAdding();
});
}
}Any code having an
IEnumerable and wanting to use it as a producer passes it to the constructor, then accesses it through the ProducerCollection.Interested in a general review, but in particular:
- Is this the simplest/most idiomatic way to do achieve the overall goal?
- Any comments/suggestions on naming.
- Extensibility (e.g. adding cancellation or parallelizing the enumeration of the source) probably fall under YAGNI for me at the moment, but if there are alternative designs which would make this kind of extension in general easier to implement, I would be interested.
Solution
-
I think the implementation is too leaky - it should not expose how the blocking internals are achieved since if if decide do change this you might have to change a whole lot of application code which violates the open-closed-principle (the O part of SOLID). This can be easily fixed by making the class
-
I think the more idiomatic way to start a new task is to use
-
I would rename it into
-
Since you're talking about long running operations you may want to add a method of canceling the task.
-
It might be useful to add a capacity for the intermediate blocking collection as well to provide better control of how much data is cached.
Slightly refactored code:
I think the implementation is too leaky - it should not expose how the blocking internals are achieved since if if decide do change this you might have to change a whole lot of application code which violates the open-closed-principle (the O part of SOLID). This can be easily fixed by making the class
IEnumerable and using GetConsumingEnumerable of the BlockingCollection.-
I think the more idiomatic way to start a new task is to use
Task.Factory.StartNew.-
I would rename it into
BlockingEnumerable since that what it is.-
Since you're talking about long running operations you may want to add a method of canceling the task.
-
It might be useful to add a capacity for the intermediate blocking collection as well to provide better control of how much data is cached.
Slightly refactored code:
class BlockingEnumerable : IEnumerable
{
private readonly BlockingCollection _ProducerCollection;
public BlockingEnumerable(IEnumerable source)
{
_ProducerCollection = new BlockingCollection();
AddItemsToBlockingCollection(source);
}
public IEnumerator GetEnumerator()
{
return _ProducerCollection.GetConsumingEnumerable().GetEnumerator();
}
IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
private void AddItemsToBlockingCollection(IEnumerable source)
{
Task.Factory.StartNew(() =>
{
foreach (var item in source)
{
_ProducerCollection.Add(item);
}
_ProducerCollection.CompleteAdding();
});
}
}Code Snippets
class BlockingEnumerable<T> : IEnumerable<T>
{
private readonly BlockingCollection<T> _ProducerCollection;
public BlockingEnumerable(IEnumerable<T> source)
{
_ProducerCollection = new BlockingCollection<T>();
AddItemsToBlockingCollection(source);
}
public IEnumerator<T> GetEnumerator()
{
return _ProducerCollection.GetConsumingEnumerable().GetEnumerator();
}
IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
private void AddItemsToBlockingCollection(IEnumerable<T> source)
{
Task.Factory.StartNew(() =>
{
foreach (var item in source)
{
_ProducerCollection.Add(item);
}
_ProducerCollection.CompleteAdding();
});
}
}Context
StackExchange Code Review Q#93129, answer score: 2
Revisions (0)
No revisions yet.