HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Generic cached IEnumerable<T>

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericcachedienumerable

Problem

I've an IEnumerable 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:

  • Where should I have _cache? In CachedEnumerable or CacheEnumerator?



  • Thread safety in adding into cache.



  • Reinventing? Any existing .NET or other library which does this.



  • Why Dispose in IEnumerator? Should I be doing something there? _cache nuke?



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

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

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

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

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

public CachedEnumerator(IList cache, IEnumerator 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)

Solution


  • You need to access _cache in both classes, so it makes sense to have it as field in both of them too.



  • 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.



  • I don't think anything like this is in .Net.



  • 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!

Context

StackExchange Code Review Q#11505, answer score: 5

Revisions (0)

No revisions yet.