Enumerable types and interfaces, which type do you prefer to return?
Tags: csharp
Published: 2019-11-19 23:39:05

A discussion I've been involved with recently has been an updated guideline around the return types of interfaces. It was decided that `IEnumerable` was to not be used unless it was required that results be yielded rather than available immediately. There was to be a new rule that whenever you saw an `IEnumerable` you change it to `ICollection` to make it clear the results are immediately available.

I disagree with this change for many reasons.

First, on principle. It seems like a change to protect against an implementation where the developer doesn't realise an enumerable might be lazy and the generator might be in an invalid state when the consumer requests the results (perhaps it has an `IDisposable` dependency). This has never seemed like a convincing argument to me. We shouldn't limit design choices so that developers don't have to learn the language, they should understand the language they're writing in.

Also, it has the effect that developers tend to call `.ToList()` or `.ToArray()` at the end of every method simply to satisfy the `ICollection`. The caller might only be interested in `.Any()` or `.Take(some)` or doing further filtering. The creation and allocation of the intermediate collection is then wasted time.

`ICollection` has methods like `Add` and `Remove`, meaning what was previously a beautiful, simple, immutable return type is now a bloated mutable mess of an interface. Worse is if someone actually does start using the `.Add` and `.Remove` methods but the implementation behind the interface returns an array (which for some reason that I've never quite understood implements the `ICollection` interface) the calling code will start having runtime exceptions. 

I wouldn't be so averse to using `IReadOnlyCollection` but unfortunately this idea was met with strong resistance as that type wasn't something other devs were familiar with and `ICollection` was already used in some places in the code, where as `IReadOnlyCollection` was not. The other issue brought up with `IReadOnlyCollection` was that it wasn't a sub-type of `ICollection`, it was on the same level. 

I'd always assumed it was a subtype but apparently not, it was added much later and so would have been a breaking change to make `ICollection` implement `IReadOnlyCollection`. Regardless, it is sort of a subtype because all the members declared on `IReadOnlyCollection` are declared on `ICollection` and more so that argument again I didn't find convincing. I also don't really understand why it had to be a subtype of `ICollection` to be used instead, surely the added immutability would be reason enough.

I've always been of the opinion that these sorts of decisions should be made on a case by case basis. There isn't always a clear cut answer for the correct return type for a collection. I would usually stick to simpler interfaces (`IEnumerable` or `IReadOnlyCollection`) but I'd be hesitant to make a blanket statement around which to use in general. What do you think? How do you decide on return types? Perhaps you agree with my colleagues and can put their arguments to me so that I can be on the same page and feel more comfortable with the new guidelines.