21

I know that in .NET all arrays derive from System.Array and that the System.Array class implements IList, ICollection and IEnumerable. Actual array types also implement IList<T>, ICollection<T> and IEnumerable<T>.

This means that if you have, for example, a String[], then that String[] object is also a System.Collections.IList and a System.Collections.Generic.IList<String>;.

It's not hard to see why those IList's would be considered "ReadOnly", but surprisingly...

String[] array = new String[0];
Console.WriteLine(((IList<String>)array).IsReadOnly); // True
Console.WriteLine(((IList)array).IsReadOnly); // False!

In both cases, attempts to remove items via the Remove() and RemoveAt() methods results in a NotSupportedException. This would suggest that both expressions correspond to ReadOnly lists, but IList's ReadOnly property does not return the expected value.

How come?

Selman Genç
  • 100,147
  • 13
  • 119
  • 184
J Smith
  • 2,375
  • 3
  • 18
  • 36
  • 5
    I suspect Remove() and RemoveAt() throw exceptions because those operations imply the resizing of the underlying list, which isn't supported by arrays outside the Resize method - but this doesn't answer your main question. – Erik Forbes Oct 20 '14 at 21:44
  • @ErikForbes Think you are correct. `Add` on `IList` even though apparently is not read-only, throws a Not Supported exception of `Collection was of a fixed size` – TyCobb Oct 20 '14 at 21:47
  • Yeah I think any operation that implicitly would change the size of the underlying array will fail with that exception. – Erik Forbes Oct 20 '14 at 21:48
  • 1
    [Related reading](http://stackoverflow.com/questions/11163297/how-do-arrays-in-c-sharp-partially-implement-ilistt) – Rawling Oct 20 '14 at 21:49
  • My money is on "historical accident", but I'd love for an MSFT insider to pop up with an explanation. In particular, `IList.ReadOnly` is arguably *correct* in saying the array is *not* read only, because you can modify its elements (which, according to the documentation on `ReadOnly`, you are not supposed to be able to do) but of course this runs counter to how people *actually* use `ReadOnly` (to see if you can add or remove elements), so when `List` rolled around they "fixed" it. – Jeroen Mostert Oct 20 '14 at 21:51
  • Wish the answer that pointed out `Array`'s implementation didn't get downvoted to hell. Seems like it was on the right track. – TyCobb Oct 20 '14 at 21:54
  • 1
    @TyCobb It claimed that the cast to `IList` was instantiating a `ReadOnlyCollection`, which was wrong and misleading. – Timothy Shields Oct 20 '14 at 21:55
  • Oh. Missed that. It was gone before I could fully read it. Hate when I auto refresh and stuff disappears. Thanks for clarifying. – TyCobb Oct 20 '14 at 21:57

2 Answers2

12

This looks like a plain bug to me:

  • It clearly isn't read-only, as the indexer allows it to be modified
  • It is not performing a conversion to any other kind of object

Note that you don't need to cast - there's an implicit conversion:

using System;
using System.Collections.Generic;

class Test
{
    static void Main()
    {
        string[] array = new string[1];
        IList<string> list = array;
        Console.WriteLine(object.ReferenceEquals(array, list));
        Console.WriteLine(list.IsReadOnly);
        list[0] = "foo";
        Console.WriteLine(list[0]);
    }
}

ICollection<T>.IsReadOnly (which IList<T> inherits the property from) is documented as:

A collection that is read-only does not allow the addition, removal, or modification of elements after the collection is created.

While an array doesn't allow the addition or removal of elements, it clearly does allow modification.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    `IsReadOnly` isn't granular enough to indicate the level of read-only-ness here. Maybe they should have had methods such as `CanAddElements`, `CanRemoveElements`, `CanModifyElements`. Alas. – Erik Forbes Oct 20 '14 at 21:53
  • 1
    The MSDN link Marius provided shows this isn't a bug - although it doesn't make any sense why `IsReadOnly` should have a different value depending on whether the `T[]` is cast to `IList` or `IList`. – Timothy Shields Oct 20 '14 at 22:00
  • I think the difference in implementation between the two casts is the bug Jon's referring to. – Erik Forbes Oct 20 '14 at 22:05
  • @TimothyShields: I don't think it actually shows that at all. The quote doesn't make any sense to me. – Jon Skeet Oct 20 '14 at 22:26
  • The specification for `IList` would suggest that if `someList.IsReadOnly` returns `False`, then for any `x` of type `T` and any `n` of type `Int32` less than `someList.Count`, one may safely perform `someList[n] = x`. Even though `Cat[]` implements `IList`, one cannot safely store elements of type `Animal` to it. – supercat Oct 20 '14 at 22:32
  • @supercat: No, I don't think it would suggest that. The documentation doesn't say "If this returns false, then *every* modification is allowed" - it says "If this returns true, then *no* modification is allowed" which is clearly not the case. – Jon Skeet Oct 20 '14 at 22:34
  • It might have been helpful to say that if `arr` is an instance of `Cat[]` then `((IList)arr).IsReadOnly` should return `true` even though `((IList)arr).IsReadOnly` would be required to return false, but the I don't think the latter should return true. – supercat Oct 20 '14 at 22:34
  • @supercat: Why would the `IList` version return true? Did you mean those to be the other way round? Either way round, I disagree - there are ways you can modify the collection, therefore it *must* return `false` IMO. – Jon Skeet Oct 20 '14 at 22:35
  • @JonSkeet: The documentation for the `Item` property indicates that it throws NotSupportedException if the list is read-only, but doesn't clarify exactly what that means. It does not mention the possibility that the property setter might throw `ArrayTypeMismatchException`. – supercat Oct 20 '14 at 22:39
  • @supercat: Yup. Am I right in saying you're agreeing with me now? The `IList` implementation behaviour seems to clearly violate the contract. – Jon Skeet Oct 20 '14 at 22:42
  • @JonSkeet: I got those backward, but I still don't think we're in agreement: if an `IList` identifies an instance of `Cat[]`, it cannot promise to be modifiable; since `IsReadOnly` doesn't promise that a collection is immutable, I'm not sure what useful information one could glean from the fact that it returns `true`. – supercat Oct 20 '14 at 22:46
  • @JonSkeet: Of course, the fundamental problem is that a single `IsReadOnly` property is trying to do way too much; there should have been separate properties indicating whether a collection allows `Add`, write-by-index, and item-moving methods like `Insert` and `Remove`. – supercat Oct 20 '14 at 22:47
  • @supercat: `IsReadOnly` promises that no new items can be added or removed, or that elements can be modified. If you mean "deep" immutability, then sure, it can't promise that - but even the weaker promise that it's making here is still being clearly violated. If `list[0] = "foo"` isn't "modifying an element" then I don't know what *would* count. I agree that it's a problematic property in general, but I don't think that excuses returning `true` when elements can be modified. – Jon Skeet Oct 21 '14 at 05:56
  • @JonSkeet: What useful information is gleaned by knowing that one can't do something with an object? Immutability is useful not because of what one can't do, but because of what one *can*--safely regard a reference to an object encapsulating its contents. If `IsReadOnly` really means `!CanAddItems` that would be a slightly useful (albeit very poorly named) property. How would one use an `IsReadOnly` property that guaranteed either that `Add` will work, or write-by-index will work, but didn't specify which? – supercat Oct 21 '14 at 12:30
  • @supercat: There are levels of nuance to it all over the place - for example, just because *I* can't modify it doesn't mean no-one else can (see `ReadOnlyCollection` for example). I'm not arguing it's a well-designed property - I'm saying that the documented behaviour does not match the behaviour for arrays. – Jon Skeet Oct 21 '14 at 12:32
  • @JonSkeet: The behavior is confusingly documented, but I can't imagine any *useful* meaning for `ICollection.IsReadOnly` other than "If false, indicates that items may be added to or removed from the collection". If `IList.IsReadOnly` were independent of `ICollection.IsReadOnly`, then it might have made sense for arrays to have it implement different behavior, but since it isn't, they can't do so without rendering the property completely meaningless. – supercat Oct 21 '14 at 15:12
  • @supercat: So you're saying it's the "modification of elements" part that's the bug? That seems odd to me - because I *certainly* don't regard arrays as being "read only" in any meaningful sense. While `ICollection` doesn't expose any way of modifying elements, it is reasonable to assume that some concrete implementations may allow for this, and to reflect that potential ability in the API. This whole thing is just a huge mess, IMO... – Jon Skeet Oct 21 '14 at 15:25
  • @JonSkeet: The "modification of elements" language may be intended to suggest that interfaces which inherit `ICollection` should only return `false` from `IsReadOnly` if it can *promise* that *all* mutating methods will work. I think the point of confusion here stems from the fact that in general Boolean properties should have their "logic level" chosen so that in cases where they can't *promise* anything they return `false`. By such logic, the information which `IsReadOnly` provides should have been given by `CanUseAllWriteMethods`; likewise that of `WeakReference.IsAlive`, by `IsDead`. – supercat Oct 21 '14 at 15:57
  • @supercat: If that was the intention, it's been incredibly poorly written. Instead, it sounds to me like it's promising that it will return `true` if *none* of the mutating methods work. (And if your understanding is correct, then it doesn't explain why the `IList` version returns false... unless you're going to understand `IList.IsReadOnly` to mean something different.) – Jon Skeet Oct 21 '14 at 16:00
  • @JonSkeet: There are a few places in .NET where the same name is given to totally unrelated things (e.g. `Delegate.BeginInvoke` vs. `Control.BeginInvoke`). This is one of them. `IList` has an `IsFixedSize` property separate from `IsReadOnly`, and thus `IsReadOnly` can at best promise to allow modifications which would not contradict `IsFixedSize` (though because of array covariance it can't actually promise to allow the addition of anything that wasn't read from the array). Perhaps the biggest question should be why `ICollection` doesn't have `IsFixedSize`, which could be useful... – supercat Oct 21 '14 at 16:07
  • ...even for collections where `IsReadOnly` is true if the semantics were that code which read `Count` on a collection where `IsFixedSize` was true could safely cache the value and computations derived from it; if `IsReadOnly` were true but `IsFixedSize` were false, such caching would not be safe. – supercat Oct 21 '14 at 16:09
  • @supercat: I would have great problems with something which claimed to be read-only but not a fixed size. I view `IsReadOnly` being true to be making a promise, rather than `IsReadOnly` being false. I just can't read the documentation the same way you're doing. I would say that `ICollection` should have `IsFixedSize` *instead of* `IsReadOnly`. That would make perfect sense - the current situation makes no sense to me at all. Also note that the `IList.IsReadOnly` documentation (in the "remarks" section) is *word for word* the same as `IList.IsReadOnly`. – Jon Skeet Oct 21 '14 at 16:19
  • @JonSkeet: Given `var list = new List(); var wrapper = new ReadOnlyCollection(list);`, I would suggest that `wrapper` should return `false` for `IsFixedSize` (since a call to `list.Add(4);` would cause `wrapper.Count` to increase) but `true` for `IsReadOnly` (even though the collection is mutable, the wrapper itself only provides read-only access). What would you suggest those properties should return? – supercat Oct 21 '14 at 16:34
  • 2
    @supercat: That's precisely the problem I mentioned earlier on. Basically `IsReadOnly` doesn't really give you anything useful, even if you take it at its word. Anyway, I think we've rather overextended the comments here. I still maintain it's badly designed and the implementation for arrays is broken according to the poor documentation. – Jon Skeet Oct 21 '14 at 16:41
10

From MSDN:

Array implements the IsReadOnly property because it is required by the System.Collections.IList interface. An array that is read-only does not allow the addition, removal, or modification of elements after the array is created.

If you require a read-only collection, use a System.Collections class that implements the System.Collections.IList interface.

If you cast or convert an array to an IList interface object, the IList.IsReadOnly property returns false. However, if you cast or convert an array to a IList<T> interface, the IsReadOnly property returns true.

Read-only here means that the items in the array cannot be modified and that's why it returns false.

Also have a look at Array.IsReadOnly inconsistent depending on interface implementation.

Community
  • 1
  • 1
Marius Bancila
  • 16,053
  • 9
  • 49
  • 91
  • 2
    While I appreciate you're only quoting, the very phrase "an array that is read-only" makes no sense to me... the only immutable array is an empty one. *All* arrays allow the modification of elements after the array is created. My answer gives an example of modifying a supposedly read-only list. The `Array.IsReadOnly` doesn't do more than state the inconsistency between the `IList` and `IList` implementations. Still looks like a bug to me. – Jon Skeet Oct 20 '14 at 22:28