Generic cached IEnumerable<T>

Question

I've an IEnumerable<T> source which takes time to generate each item. Once an item is generated, I want to cache it to avoid recomputing the same thing. I want to separate out caching logic from original source.

A few things that bother me are:

1. Where should I have _cache? In CachedEnumerable or CacheEnumerator?
2. Thread safety in adding into cache.
3. Reinventing? Any existing .NET or other library which does this.
4. Why Dispose in IEnumerator? Should I be doing something there? _cache nuke?

class CachedEnumerable<T> : IEnumerable<T>
{
    private IList<T> _cache = new List<T>();
    IEnumerator<T> _cacheEnumerator = null;
    IEnumerable<T> _source = null;

    public CachedEnumerable(IEnumerable<T> source)
    {
        _source = source;
        _cacheEnumerator = _source.GetEnumerator();
    }

    public IEnumerator<T> GetEnumerator()
    {
        return new CachedEnumerator<T>(_cache, _cacheEnumerator);
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

class CachedEnumerator<T> : IEnumerator<T>
{
    private IList<T> _cache;
    private IEnumerator<T> _source;
    private T _current;
    private int _index;
    private bool _moveNextStatus;

    public CachedEnumerator(IList<T> cache, IEnumerator<T> source)
    {
        _cache = cache;
        _source = source;
        Reset();
    }

    public T Current
    {
        get
        {
            if (_moveNextStatus)
            {
                return _current;
            }
            else
            {
                throw new InvalidOperationException();
            }
        }
    }

    public void Dispose()
    {
        _source.Dispose();
    }

    object System.Collections.IEnumerator.Current
    {
        get { return Current; }
    }

    public bool MoveNext()
    {
        _moveNextStatus = _MoveNext();
        return _moveNextStatus;
    }

    private bool _MoveNext()
    {
        _index++;

        if (_index < _cache.Count)
        {
            _current = _cache[_index];
            return true;
        }
        else
        {
            if (_source.MoveNext())
            {
                _current = _source.Current;
                _cache.Add(_current);
                return true;
            }
            else
            {
                return false;
            }
        }
    }

    public void Reset()
    {
        _index = -1;
        _current = default(T);
        _moveNextStatus = false;
    }
}

Answer 1

1. You need to access _cache in both classes, so it makes sense to have it as field in both of them too.
2. Your code is not thread safe. If you wanted to make it thread safe, I think the best solution would be to have a lock that you use whenever you access _cache or _source.
3. I don't think anything like this is in .Net.
4. What you're currently doing in Dispose() is wrong. You can't call Dispose() on _source and expect to use it later in another enumerator. In your case, I think you shouldn't do anything in Dispose(). You certainly shouldn't “nuke” _cache, because you're going to need it later!

Answer 2

1 and 2 I agree with svick:

1. You need to access _cache in both classes, so it makes sense to have it as field in both of them too.
2. Your code is not thread safe. If you wanted to make it thread safe, I think the best solution would be to have a lock that you use whenever you access _cache or _source.

Regarding 3 I found the following 2 solutions besides this page:

• Option 2: MemoizeAll()
• A threadsafe cached IEnumerable

Answer 3

You do not have to go to all this trouble to implement a caching wrapper. It can be done much more simply:

class CachedEnumerable<T> : IEnumerable<T> 
{
    private readonly IEnumerable<T> producer;
    private IEnumerable<T> cache;

    public CachedEnumerable(IEnumerable<T> producer)
    {
        this.producer = producer;
    }

    public IEnumerator<T> GetEnumerator()
    {
        if (!this.IsCacheValid()) {
            this.cache = this.producer.ToList();
        }

        return this.cache.GetEnumerator();
    }

    protected virtual bool IsCacheValid()
    {
        return this.cache != null;
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return this.GetEnumerator();
    }
}

This implementation has a cache that never expires, but you can modify (or override in a derived class) IsCacheValid to tweak this behavior to taste. It also has no thread safety, but that's as easy to add as

// need to make this always have a non-null value so it can be 
// used as a parameter to Monitor.Enter
private IEnumerable<T> cache = new T[0];

public IEnumerator<T> GetEnumerator()
{
    lock (this.cache) {
        if (!this.IsCacheValid()) {
            this.cache = this.producer.ToList();
        }

        return this.cache.GetEnumerator();
    }
}

If IsCacheValid were private then the lock could be reduced to protecting just the this.producer.ToList() statement, but as it stands it must protect everything inside GetEnumerator.