3
\$\begingroup\$

I have a relatively simple problem: I have an IEnumerable<T>, 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<T> so that I can produce and consume concurrently.

To support this, I have a simple wrapper:

class EnumerableProducer<T>
{
    public readonly BlockingCollection<T> ProducerCollection;

    public EnumerableProducer(IEnumerable<T> source)
    {
        ProducerCollection = new BlockingCollection<T>();
        AddItemsToBlockingCollection(ProducerCollection, source).Start();
    }

    private static Task AddItemsToBlockingCollection(BlockingCollection<T> collection, IEnumerable<T> source)
    {
        return new Task(() =>
        {
            foreach (var item in source)
            {
                collection.Add(item);
            }
            collection.CompleteAdding();
        });
    }
}

Any code having an IEnumerable<T> 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.
\$\endgroup\$
  • 1
    \$\begingroup\$ You may want to take a look at Reactive Extensions, as it's designed to do precisely what you are trying to do. \$\endgroup\$ – Dan Lyons Jun 9 '15 at 18:16
  • \$\begingroup\$ @DanLyons Thanks! Ideally I don't want to add any new dependencies to the project for what's at the moment a very simple requirement, but I'll keep it in mind for the future. \$\endgroup\$ – Ben Aaronson Jun 9 '15 at 22:08
  • 1
    \$\begingroup\$ I feel that more context/code would be helpful for giving a review. Is the IEnumerable CPU- or I/O-bound? Can you give an example of an IEnumerable that you would use with this, and maybe also code for a consumer? Is this meant to be a general solution for any slow IEnumerable? \$\endgroup\$ – mjolka Jun 10 '15 at 6:21
2
\$\begingroup\$
  1. 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<T> and using GetConsumingEnumerable of the BlockingCollection.

  2. I think the more idiomatic way to start a new task is to use Task.Factory.StartNew.

  3. I would rename it into BlockingEnumerable since that what it is.

  4. Since you're talking about long running operations you may want to add a method of canceling the task.

  5. 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<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();
        });
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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