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

Custom list to avoid out of memory exceptions

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

Problem

The following class is a helper to avoid OutOfMemoryException exceptions being thrown when the list is to large so that it is added onto the large objects heap. To do this it adds the items onto the heap as smaller arrays contained inside of a list.

Some quick notes:

  • I prefer not to use var.



  • The constant in all caps is a company standard.



```
using System;
using System.Collections;
using System.Collections.Generic;

namespace Helpers.Collections
{
public class ChunkedList : IEnumerable, IDisposable
{
private const int DEFAULT_CHUNK_SIZE = 4096;

private readonly int _nChunkSize;
private IList _alInternalCollection;
private int _nCount;
private bool _bTIsDisposable;

public int Count
{
get
{
return _nCount;
}
}

public ChunkedList() : this(DEFAULT_CHUNK_SIZE) { }

public ChunkedList(int chunkSize)
{
_alInternalCollection = new List();

_nChunkSize = chunkSize;

_nCount = 0;

_bTIsDisposable = typeof(IDisposable).IsAssignableFrom(typeof(T));
}

public void Dispose()
{
this.Clear();
}

public T this[int index]
{
get
{
if (index = _nCount)
{
throw new IndexOutOfRangeException();
}

int nChunk = index / _nChunkSize;
int nChunkIndex = index % _nChunkSize;

return _alInternalCollection[nChunk][nChunkIndex];
}
set
{
if (index = _nCount)
{
throw new IndexOutOfRangeException();
}

int nChunk = index / _nChunkSize;
int nChunkIndex = index % _nChunkSize;

T oldItem = _alInternalCollection[nChunk][nChunkIndex];
_alI

Solution

Following the existing conventions is more important than breaking them to follow the established, standard ones - so fine, have SCREAM_CASE constants and redundant type names everywhere.

But then, if you're going to not use var and make every type explicit, it seems to me that there's a worrying amount of care given to what type a variable might be, to the point of keeping Hungarian Notation around:

private readonly int _nChunkSize;
    private IList _alInternalCollection;
    private int _nCount;
    private bool _bTIsDisposable;


These prefixes are useless at best, utterly annoying at worst. I'd much rather read this:

private readonly int _chunkSize;
    private readonly List _items;
    private readonly bool _isDisposable;


Anything that's only ever meant to be assigned in the constructor should be readonly.

_alInternalCollection is a terrible name IMO. First, it's not a collection, second, what's the a prefix for anyway; third, I suppose l stands for "List", and that makes the Collection part redundant, if not confusing. I'd just call it _items and be done with it.

Name things after what they're used for, not after their type.

The positioning of members makes the code much harder to follow than it should be: I expect a type's constructor to be the first public member of a class. Your code has a Count property instead.

Hence:

private const int DEFAULT_CHUNK_SIZE = 4096;

private readonly List _items;
private readonly int _chunkSize;
private readonly bool _isDisposable;

public ChunkedList()
    : this(DEFAULT_CHUNK_SIZE)
{
}

public ChunkedList(int chunkSize)
{
    _chunkSize = chunkSize;
    _items = new List();
    _isDisposable = typeof(IDisposable).IsAssignableFrom(typeof(T));
}


Followed by public properties/indexers, then public methods, then the IEnumerable implementation, and then the IDisposable implementation; you have Dispose near the top, Count above the constructors, and IEnumerable members in the middle of the class. Feels messy.

Another thing that strikes me: you've got Add, AddRange, Clear, Contains, IndexOf, Count ...but you're not implementing IList? Not even ICollection? This means this ChunkedList, when used in code that's written against abstractions, can only ever be exposed as an IEnumerable... and that doesn't feel right. You have a custom generic collection class, make it implement a generic collection interface and let the client code work off an IList and be blissfully unaware of the implementation detail that the "chunking" is.

Code Snippets

private readonly int _nChunkSize;
    private IList<T[]> _alInternalCollection;
    private int _nCount;
    private bool _bTIsDisposable;
private readonly int _chunkSize;
    private readonly List<T[]> _items;
    private readonly bool _isDisposable;
private const int DEFAULT_CHUNK_SIZE = 4096;

private readonly List<T[]> _items;
private readonly int _chunkSize;
private readonly bool _isDisposable;

public ChunkedList()
    : this(DEFAULT_CHUNK_SIZE)
{
}

public ChunkedList(int chunkSize)
{
    _chunkSize = chunkSize;
    _items = new List<T[]>();
    _isDisposable = typeof(IDisposable).IsAssignableFrom(typeof(T));
}

Context

StackExchange Code Review Q#134279, answer score: 6

Revisions (0)

No revisions yet.