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
.ToArray() at the end of every method simply to satisfy the
ICollection. The caller might only be interested in
.Take(some) or doing further filtering. The creation and allocation of the intermediate collection is then wasted time.
ICollection has methods like
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
.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
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 (
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.