patterncsharpMinor
Generic cached IEnumerable<T>
Viewed 0 times
genericcachedienumerable
Problem
I've an
A few things that bother me are:
```
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)
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? InCachedEnumerableorCacheEnumerator?
- Thread safety in adding into cache.
- Reinventing? Any existing .NET or other library which does this.
- Why
DisposeinIEnumerator? Should I be doing something there?_cachenuke?
```
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
_cachein 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
_cacheor_source.
- I don't think anything like this is in .Net.
- What you're currently doing in
Dispose()is wrong. You can't callDispose()on_sourceand expect to use it later in another enumerator. In your case, I think you shouldn't do anything inDispose(). 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.