53

If I have a method that requires a parameter that,

  • Has a Count property
  • Has an integer indexer (get-only)

What should the type of this parameter be? I would choose IList<T> before .NET 4.5 since there was no other indexable collection interface for this and arrays implement it, which is a big plus.

But .NET 4.5 introduces the new IReadOnlyList<T> interface and I want my method to support that, too. How can I write this method to support both IList<T> and IReadOnlyList<T> without violating the basic principles like DRY?

Edit: Daniel's answer gave me some ideas:

public void Foo<T>(IList<T> list)
    => Foo(list, list.Count, (c, i) => c[i]);

public void Foo<T>(IReadOnlyList<T> list)
    => Foo(list, list.Count, (c, i) => c[i]);

private void Foo<TList, TItem>(
    TList list, int count, Func<TList, int, TItem> indexer)
    where TList : IEnumerable<TItem>
{
    // Stuff
}

Edit 2: Or I could just accept an IReadOnlyList<T> and provide a helper like this:

public static class CollectionEx
{
    public static IReadOnlyList<T> AsReadOnly<T>(this IList<T> list)
    {
        if (list == null)
            throw new ArgumentNullException(nameof(list));

        return list as IReadOnlyList<T> ?? new ReadOnlyWrapper<T>(list);
    }

    private sealed class ReadOnlyWrapper<T> : IReadOnlyList<T>
    {
        private readonly IList<T> _list;

        public ReadOnlyWrapper(IList<T> list) => _list = list;

        public int Count => _list.Count;

        public T this[int index] => _list[index];

        public IEnumerator<T> GetEnumerator() => _list.GetEnumerator();

        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
    }
}

Then I could call it like Foo(list.AsReadOnly())


Edit 3: Arrays implement both IList<T> and IReadOnlyList<T>, so does the List<T> class. This makes it pretty rare to find a class that implements IList<T> but not IReadOnlyList<T>.

Şafak Gür
  • 7,045
  • 5
  • 59
  • 96
  • Related question: http://stackoverflow.com/questions/12622539/why-doesnt-generic-icollection-implement-ireadonlycollection-in-net-4-5 – Zaid Masud Aug 19 '13 at 13:57
  • 1
    Update from January 2021: It was looking like it would be done in time for .NET 5 but it's still an open issue: https://github.com/dotnet/runtime/issues/31001 – Dai Jan 12 '21 at 06:03

4 Answers4

36

You are out of luck here. IList<T> doesn't implement IReadOnlyList<T>. List<T> does implement both interfaces, but I think that's not what you want.

However, you can use LINQ:

  • The Count() extension method internally checks whether the instance in fact is a collection and then uses the Count property.
  • The ElementAt() extension method internally checks whether the instance in fact is a list and than uses the indexer.
Daniel Hilgarth
  • 171,043
  • 40
  • 335
  • 443
  • 2
    So the answer would be changing the method in a way that it accepts an IEnumerable, which is supported by any object that implements wheter IList or IReadOnlyList. – Yiğit Yener Oct 11 '12 at 11:25
  • 2
    If it is important to only allow `IList` and `IReadOnlyList` as input, I would create two public overloads, one for `IList` and one for `IReadOnlyList`, and create a private overload with `IEnumerable`. Otherwise I would just create one public version with `IEnumerable`. – Daniel Hilgarth Oct 11 '12 at 11:29
  • Thank you, I decided not to use LINQ but it gave me some ideas. I updated the question with the solution I'm going to use. – Şafak Gür Oct 11 '12 at 12:00
  • @ŞafakGür If you want your method to take in a "list" (allows indexing), and only want to read from it, surely use the parameter type `IReadOnlyList` because it's **covariant** in `T`. It means that if people have a "list" of a more specialized item type `T2`, where `T2` derives from `T`, they don't have to iterate through their "list" to convert it to a new "list"; they can pass it directly because of covariance. Any **reasonable** list class that implements `IList` will **also** implement `IReadOnlyList`. This goes for BCL types like `T[]` (array) and `List` and so on. – Jeppe Stig Nielsen Nov 29 '12 at 23:52
  • 5
    @JeppeStigNielsen: While any reasonable list type which is implemented *in the future* should implement both `IList` and `IReadOnlyList`, many good classes which implement `IList` were written before `IReadOnlyList` existed. I have long wished for a means by which `IList` could be made to inherit from a covariant read-only interface with an indexed getter, with the run-time--if necessary--automatically creating a read-only indexer which would wrap the read-write one, but because no such means exists it will for now be necessary to accept either `IList` or `IReadOnlyList`. – supercat Nov 30 '12 at 16:26
  • @supercat It's a good point. If you use other peoples' list classes from before .NET 4.5, you will have to accept that they are "reasonable" but don't know about `IReadOnlyList<>`. However, it's easy to update your own list classes, if you have some home-made ones. – Jeppe Stig Nielsen Nov 30 '12 at 16:31
  • 1
    LINQ extension methods only check if your instance implements IList. So if you have an instance of a class which implements only IReadOnlyList and not IList, you're out of luck and LINQ will dumbly iterate through the whole collection (tested on .net 4.7, and checked sources of .net core). Luckily it's quite uncommon case but can happen with custom implementations. – Elephantik Feb 28 '18 at 13:31
  • If only it were that simple, that implementing a method for both `IList` and `IReadOnlyList` did the trick, but the thing is it doesn't work nicely for `List`, because the compiler doesn't know which one to choose... – Tom Oct 21 '18 at 18:49
  • @Tom I "solved" that problem by just adopting a brave face and making all of my methods accept `IEnumerable` instead - and using my own extension-method to get a `Count` which checking for `IReadOnlyCollection.Count` _and_ `ICollection.Count` (and even falling-back to Reflection). Still, could be worse... we could be writing in Java ;) - another option is to define a _succint_ extension-method on `ICollection` that adapts it to `IReadOnlyCollection` and then having an overload only for `IReadOnlyCollection`. – Dai Jun 10 '21 at 02:46
  • @Dai What I did when I was still working in C# was that I did everything that only required read access only for the "read-only" interfaces, and that whenever I found myself needing such functionality for an object whose compile-time type did not implement a "read-only" interface, I converted it to a read-only view of itself. This conversion was done by an extension method which checked if the actual run-time type was a "read-only" one and just used a cast in that case, and used a wrapping implementation (which I had to write myself) otherwise. – Tom Jun 11 '21 at 06:33
  • @supercat exactly. I've always considered `IReadOnlyList` to be a design mistake. It should have been something like `IReadableList` as a covariant read-only interface with an indexed getter, and have `IList` implement it. – Mr Anderson Mar 28 '22 at 19:18
  • @MrAnderson: A bigger design mistake was failing to have `IEnumerable` and `IEnumerator` include methods/properties to do things like return an immutable enumerable or indexable enumerable with the same items as the original, indicate whether the count is "known" (along wiht a property to indicate what it is), either move ahead by N items and return 0, or if only M items remained, return N-M, indicate whether rewinding is supported, rewind the enumeration (if rewinding is supported), etc. Any of those things could be done in type-agnostic fashion with any `IEnumerable`, but ... – supercat Mar 28 '22 at 22:57
  • ...most implementations could support a better means of doing them would be possible with the existing implementations. For example, if an implementation *is* an immutable enumerable, its "return an immutable enumerable with the same contents" method could simply return a reference to itself, and a `List` could process "skip ahead by 1,000,000 elements" simply by adding 1,000,000 to the index and checking whether it landed within the list, and returning the amount by which it passed the end if it didn't, without having to execute any kind of loop 1,000,000 times. – supercat Mar 28 '22 at 23:00
5

Since IList<T> and IReadOnlyList<T> do not share any useful "ancestor", and if you don't want your method to accept any other type of parameter, the only thing you can do is provide two overloads.

If you decide that reusing codes is a top priority then you could have these overloads forward the call to a private method that accepts IEnumerable<T> and uses LINQ in the manner Daniel suggests, in effect letting LINQ do the normalization at runtime.

However IMHO it would probably be better to just copy/paste the code once and just keep two independent overloads that differ on just the type of argument; I don't believe that micro-architecture of this scale offers anything tangible, and on the other hand it requires non-obvious maneuvers and is slower.

Jon
  • 428,835
  • 81
  • 738
  • 806
  • 4
    I strongly disagree with copy/paste, even on this level. – Daniel Hilgarth Oct 11 '12 at 11:27
  • 4
    @DanielHilgarth: I 'm a believer in "refactoring on seeing a third copy". Dismissing copy/paste *unconditionally* does not really sound like good engineering. – Jon Oct 11 '12 at 11:30
  • 4
    The only difference is the input type, everything else is completly the same. Using copy and paste you open up yourself to subtle bugs. You might fix one version but forget the other. I don't see why it is not good engineering to prevent something like this from the beginning. What is the advantage of your approach to allow two copies of the same code? What is the disadvantage of my approach to not do it? – Daniel Hilgarth Oct 11 '12 at 11:33
  • 2
    @DanielHilgarth: Sure, but the requirements mentioned in the answer hint that it isn't going to be that bad (how complicated can you get with only `Count` and an indexer?). If we 're talking about 5 lines of code I would copy/paste and document. Otherwise some other solution would probably be preferable. I 'm just saying that outright banning copy/paste is not a very good idea. – Jon Oct 11 '12 at 11:38
  • 1
    First: I never talked about "outright banning copy/paste", that is your false interpretation of my strong disagreement with using it. For me it is a "tool" on the same level as goto: In rare cases, it has its use. Second: You still didn't bring forward any *reasons* for why you would copy and paste with all the problems that come with it. You even write you would document it. What reason for the copy would this documentation contain when there is an easy fix available? – Daniel Hilgarth Oct 11 '12 at 11:42
  • 1
    @DanielHilgarth: First, let's all chill out. :) Then: you said you disagree, and I clarified that I 'm visualizing a pretty short and simple method here. IMO it would be fair to say that disagreeing in the hypothetical case of such a method is outright banning. C/P has the drawbacks that you mention, but any kind of other refactoring also has obvious drawbacks (performance, what LINQ does is "type switching", etc). Hiding these drawbacks behind a binary barrier and comparing this sanitized view with the naked truth of C/P is neither the best way to evaluate the situation nor fair. – Jon Oct 11 '12 at 11:50
  • 1
    For me, using C/P has nothing to do with the complexity of the code you are copying but of the negative side effects of the alternative solutions. This means that your interpretation of a "outright ban" is still wrong and nothing I ever said. I asked for clarification why you would propose C/P in this case when it is generally accepted that it should be avoided because of the problems I mentioned. You didn't offer them and instead kept repeating that you didn't agree with what you thought I meant. This is irritating. I actually agree with you that there are cases where it is better to use C/P. – Daniel Hilgarth Oct 11 '12 at 12:01
  • However, IMHO these are rare cases, for example when the tiny performance overhead of the LINQ methods indeed is a problem or if LINQ doesn't support the particular interface. But that doesn't seem to be the case here, so I still don't understand why you would use C/P in such a scenario :-) – Daniel Hilgarth Oct 11 '12 at 12:04
  • 1
    @DanielHilgarth: Complexity of C/P code has everything to do with this because it determines the magnitude of the drawbacks that you yourself mention -- it's exactly this that would be weighed against an alternative's drawbacks. I did offer what you asked for: LINQ also does "horrible" things (e.g. obviously here it does runtime type switching). Does hiding that inside another DLL make it any better? If not, isn't that a tangible reason to consider which is really the lesser of the evils? But anyway, I think it's pointless to continue arguing on this. Let's drop it. – Jon Oct 11 '12 at 12:30
  • Agreed. We obviously disagree here :) – Daniel Hilgarth Oct 11 '12 at 12:31
  • 1
    My advice: Support only `IReadOnly`. Even if `IList` does not derive from it, any class that is an `IList` ought to be an `IReadOnlyList` as well. And like I said in comments to the other answers, all BCL classes that implement `IList` implement `IReadOnlyList` as well. – Jeppe Stig Nielsen Nov 30 '12 at 00:04
  • @JeppeStigNielsen, but even then you still basically end up with this same problem when a method wants to return `IList` to hedge against returning something other than `List` in the future… – binki Apr 17 '15 at 01:39
4

If you're more concerned with maintaining the principal of DRY over performance, you could use dynamic, like so:

public void Do<T>(IList<T> collection)
{
    DoInternal(collection, collection.Count, i => collection[i]);
}
public void Do<T>(IReadOnlyList<T> collection)
{
    DoInternal(collection, collection.Count, i => collection[i]);
}

private void DoInternal(dynamic collection, int count, Func<int, T> indexer)
{
    // Get the count.
    int count = collection.Count;
}

However, I can't say in good faith that I'd recommend this as the pitfalls are too great:

  • Every call on collection in DoInternal will be resolved at run time. You lose type safety, compile-time checks, etc.
  • Performance degradation (while not severe, for the singular case, but can be when aggregated) will occur

Your helper suggestion is the most useful, but I think you should flip it around; given that the IReadOnlyList<T> interface was introduced in .NET 4.5, many API's don't have support for it, but have support for the IList<T> interface.

That said, you should create an AsList wrapper, which takes an IReadOnlyList<T> and returns a wrapper in an IList<T> implementation.

However, if you want to emphasize on your API that you are taking an IReadOnlyList<T> (to emphasize the fact that you aren't mutating the data), then the AsReadOnlyList extension that you have now would be more appropriate, but I'd make the following optimization to AsReadOnly:

public static IReadOnlyList<T> AsReadOnly<T>(this IList<T> collection)
{
    if (collection == null)
        throw new ArgumentNullException("collection");

    // Type-sniff, no need to create a wrapper when collection
    // is an IReadOnlyList<T> *already*.
    IReadOnlyList<T> list = collection as IReadOnlyList<T>;

    // If not null, return that.
    if (list != null) return list;

    // Wrap.
    return new ReadOnlyWrapper<T>(collection);
}
casperOne
  • 73,706
  • 19
  • 184
  • 253
  • +1 for insight and thanks for suggestion, I went with `IList` to `IReadOnlyList` because it felt wrong to throw `NotSupportedException`s from explicitly implemented interfaces and to my suprise, it accepts arrays too. (It's odd since `Array` doesn't implement it, `IReadOnlyList` doesn't even have a non-generic version like `IList`) I wonder what do you think about my first sample though. I was thinking about using `dynamic` then but as you have said, it could hurt the performance (I'll call this method a lot in a short time)... – Şafak Gür Oct 12 '12 at 06:40
  • ...So I assumed the `Count` would never change (thread-safety is not intended) and calling indexer's get accessor using a delegate would be nearly as fast as calling it directly. So I guess the performence difference in those approaches is between creating an instance of `ReadOnlyWrapper` and creating an instance of `Func` everytime it is called (And it sounds like micro-optimization). – Şafak Gür Oct 12 '12 at 06:41
  • @ŞafakGür Arrays implement `IList`, so that's probably what you're seeing. I chose to go the other way because of the interoperability issues with previously existing code that I can't change. To me, throwing the exception is alright, as the first line of the documentation for that exception states (emphasis mine): "**The exception that is thrown when an invoked method is not supported**". That sounds like a perfect fit. Yeah, it's a run-time exception, but I think we're talking about limited use cases where you're interacting with an API that was poorly designed in the first place... – casperOne Oct 12 '12 at 11:36
  • @ŞafakGür ... if it was designed with `IList`. I will concede that there are cases where `IList` is appropriate, not saying it's wrong in all cases, but people tend to blatantly use that when say, `IEnumerable` will do. The first method isn't bad, but the more distinct operations on `IReadOnlyList` that you'll have to perform, the more `Func` or `Action` parameters you'll have to pass, and that's going to get really messy. Better to define what you mean and pass a wrapper for `IList` (as I mentioned in the last paragraph, if it works for you)... – casperOne Oct 12 '12 at 11:43
  • 1
    @ŞafakGür ... I've also updated the answer with an optimization on your `AsReadOnlyList` implementation. It's slight, but doesn't hurt. No need to wrap something with `IReadOnlyList` when it already *is* a `IReadOnlyList`. This kind of type-sniffing is done all the time in LINQ. – casperOne Oct 12 '12 at 11:44
  • 1
    Thanks for the trick, I'll use that. And yes arrays implement `IList` but oddly, `new int[0] is IReadOnlyList` returns true. I can use arrays to call methods that accept `IReadOnlyList`. – Şafak Gür Oct 12 '12 at 12:20
  • 3
    Yes, there's no need for all this trouble. Just make the method take an `IReadOnlyList`. Any class that implements `IList` will also implement `IReadOnlyList`. This includes BCL types like `T[]` and `List`, and should include your own list types (if you have written any). – Jeppe Stig Nielsen Nov 29 '12 at 23:59
-1

What you need is the IReadOnlyCollection<T> available in .Net 4.5 which is essentially an IEnumerable<T> which has Count as the property but if you need indexing as well then you need IReadOnlyList<T> which would also give an indexer.

I don't know about you but I think this interface is a must have that had been missing for a very long time.

torrential coding
  • 1,755
  • 2
  • 24
  • 34
MaYaN
  • 6,683
  • 12
  • 57
  • 109
  • `I think this interface is a must have`- It is a class, not an interface. It implements both `IList` and `IReadOnlyList`. – Şafak Gür Jul 01 '14 at 07:17
  • 1
    What I meant was the IReadOnlyCollection I somehow missed the "I" – MaYaN Jul 01 '14 at 11:37
  • 3
    `IReadOnlyCollection` has the exact same problem as `IReadonlyList`: `ICollection` does not implement it. – binki Apr 17 '15 at 01:46