.NET stream capabilities - is the CanXXX test safe?
.net, design-patterns, reliability, stream
Solution
Without knowing the internals of an object, you must assume that a "flag" property is too volatile to rely on when the object is being modified in multiple threads.
I have seen this question more commonly asked about read-only collections than streams, but I feel it's another example of the same design patter, and the same arguments apply.
To clarify, the ICollection interface in .NET has the property IsReadOnly, which is intended to be used as an indicator of whether the collection supports methods to modify its contents. Just like streams, this property can change at any time and will cause InvalidOperationException or NotSupportedException to be thrown.
The discussions around this usually boil down to:
- Why isn't there an IReadOnlyCollection interface instead?
- Whether NotSupportedException is a good idea.
- The pros and cons of having "modes" versus distinct concrete functionality.
Modes are rarely a good thing, as you are forced to deal with more than one "set" of behaviour; having something which can switch modes at any time is considerably worse, as your application now has to deal with more than one "set" of behaviour too. However, just because it's possible to break something down into more discreet functionality does not necessarily mean you always should, particularly when breaking it apart does nothing to reduce the complexity of the task at hand.
My personal opinion is that you have to pick the pattern which is closest to the mental model you perceive the consumers of your class will understand. If you are the only consumer, pick whichever model you like most. In the case of Stream and ICollection, I think that having a single definition of these is much closer to the mental model built up by years of development in similar systems. When you talk about streams, you talk about file streams and memory streams, not whether they're readable or writeable. Similarly, when you talk about collections, you rarely refer to them in terms of "writeability".
My rule of thumb on this one: Always look for a way to break down the behaviours into more specific interfaces, rather than having "modes" of operation, as long as it compliments a simple mental model. If it's hard to think of the separate behaviours as separate things, use a mode-based pattern and document it very clearly.
Problem
There is a fairly common pattern used in .NET to test for the capabilities of a class. Here I'll use the Stream class as an example, but the issue applies to all classes that use this pattern. The pattern is to supply a boolean property called CanXXX to indicate that capability XXX is available on the class. For example, the Stream class has CanRead, CanWrite and CanSeek properties to indicate that the Read, Write and Seek methods can be called. If the properties value is false, then calling the respective method will result in a NotSupportedException being thrown. From the MSDN documentation on the stream class: Depending on the underlying data source or repository, streams might support only some of these capabilities. An application can query a stream for its capabilities by using the CanRead, CanWrite, and CanSeek properties. And documentation for the CanRead property: When overridden in a derived class, gets a value indicating whether the current stream supports reading. If a class derived from Stream does not support reading, calls to the Read, ReadByte, and BeginRead methods throw a NotSupportedException. I see a lot of code written along the lines of the following: ``` if (stream.CanRead) { stream.Read(…) } ``` Note that there is no synchronisation code, say, to lock the stream object in any manner — other threads may be accessing it or objects that it references. There is also no code to catch a NotSupportedException. The MSDN documentation does not state that the property value can not change over time. In fact, the CanSeek property changes to false when the stream is closed, demonstrating the dynamic nature of these properties. As such, there is no contractual guarantee that call to Read() in the above code snippet will not throw a NotSupportedException. I expect that there is a lot of code out there that suffers from this potential problem. I wonder how those who have identified this issue have addressed it. What design patterns are appropriate here? I'd also appreciate comments on the validity of this pattern (the CanXXX, XXX() pairs). To me, at least in the case of the Stream class, this represents a class/interface that is trying to do too much and should be split into more fundamental pieces. The lack of a tight, documented contract makes testing impossible and implementation even harder!