During code review, I ran into the following code (simplified):

	public class Wrapper
	{
	private IEnumerable _inner;
	public Wrapper(IEnumerable inner)
	{
	_inner = inner;
	}
	public dynamic this[int i]
	{
	get => Enumerable.ElementAt(i);
	}
	}

view raw first.cs hosted with ❤ by GitHub

We have some wrapper object for IEnumerable and allow to access it by index.

I don’t like this code, and suggested this version, instead:

	public class Wrapper
	{
	private IEnumerable _inner;
	public Wrapper(IEnumerable inner)
	{
	_inner = inner;
	}
	public dynamic this[int i]
	{
	get
	{
	if(_inner is not ICollection c)
	{
	_inner = c = _inner.ToList();
	}
	return c[i];
	}
	}
	}

view raw second.cs hosted with ❤ by GitHub

The problem is that if you’ll look at the code of ElementAt(), it is already doing that, so why the duplicate work? It is specialized to make things fast for similar scenarios, why do I write the same code again?

Because I’m familiar with the usage scenario. A really common usage pattern for the wrapper object is something like this:

var wrapper = new Wrapper( item.SelectMany(x=>x.Tags) );

view raw usage.cs hosted with ❤ by GitHub

The difference between what ElementAt() does and what I do in the second version of the code is that my version will materialize the values. If you are calling that in a for loop, you’ll pay the cost of iterating over all the items once.

On the other hand, since the instance we pass to ElementAt() isn’t one that has an indexer, we’ll have to scan through the enumerator multiple times. A for loop with this implementation is a quadratic accident waiting to happen.