Make readonly collections truly readonly #394
Conversation
These methods actually returned mutable collections that happen to implement the readonly interfaces. User could cast them to the actual type and add things. I'd rather avoid even that possibility by making these truly return readonly stuff.
|
|
||
| var punchCard = new PunchCard(points); | ||
|
|
||
| Assert.Null(punchCard.PunchPoints as ICollection<PunchCardPoint>); |
There was a problem hiding this comment.
Assert.IsNotType<ICollection<PunchCardPoint>>(punchCard.PunchPoints) ?
There was a problem hiding this comment.
Nope. That asserts that it's exactly that type. I want to test the is a relationship. Unfortunately there's not a built in assertion for exactly this.
There was a problem hiding this comment.
Of course it does, silly me.
What about an custom assert that does the opposite of Assert.IsAssignableFrom to make this a bit more descriptive?
There was a problem hiding this comment.
Well I think it reads just fine, but I could add AssertEx.IsNotAssignableFrom
There was a problem hiding this comment.
Or...
public static void IsReadOnlyCollection<T>(object instance) where T : class
{
var collection = instance as ICollection<T>;
Assert.True(collection == null || collection.IsReadOnly);
}There was a problem hiding this comment.
Even better.
public static void IsReadOnlyCollection<T>(object instance) where T : class
{
var collection = instance as ICollection<T>;
// The collection == null case is for .NET 4.0
Assert.True(instance is IReadOnlyCollection<T> && (collection == null || collection.IsReadOnly));
}
|
Looks good. Perhaps making the test a bit more readable by using IsNotType is the only improvement I can think of... |
|
|
Make readonly collections truly readonly
These methods actually returned mutable collections that happen
to implement the readonly interfaces. User could cast them to
the actual type and add things. I'd rather avoid even that
possibility by making these truly return readonly stuff.