Is the .NET Stream class poorly designed?

Viewed 4174

I've spent quite a bit of time getting familiar with the .NET Stream classes. Usually I learn a lot by studying the class design of professional, commercial-grade frameworks, but I have to say that something doesn't quite smell right here.

System.IO.Stream is an abstract class representing a sequence of bytes. It has 10 abstract method/properties: Read, Write, Flush, Length, SetLength, Seek, Position, CanRead, CanWrite, CanSeek. So many abstract members makes it cumbersome to derive from, because you have to override all those methods, even if most end up just throwing NotImplemented.

Users of Stream classes are expected to call CanRead, CanWrite, or CanSeek to find out the capabilities of the Stream, or I suppose just go ahead and call Read, Write, or Seek and see if it throws NotImplemented. Is it just me, or is this crummy design?

Though there are many nits I'd like to pick with the Stream class design, the main one I'd like to ask about is this: Why didn't they use interfaces, like IReadable, IWriteable, ISeekable, instead? Then a new Stream class could gracefully derive from the interfaces it supports. Isn't this the object-oriented way of doing things? Or am I missing something?

Update: It was pointed out that the value CanRead et al can change at runtime—for example if a FileStream is closed—and the point is taken. However, I remain unconvinced that this is a good design. From where I'm from, trying to read from a file that's already been closed is a bug, or at least an exceptional condition. (And thus throwing an exception is a natural way to handle this situation.)

Does this mean that every time I'm about to Read from a Stream, I should check CanRead? And would that mean I should put a lock in place to avoid a race condition, if it's possible for the value to change sometime in between the CanRead call and the Read call?

Update Aug 7 2010: The consensus here seems to be the Stream design is pretty good as it stands. But let me ask one more time just to be 100% sure: People are writing something like this every time they read from a Stream?

//  s is a Stream

lock(s)
{
    if (s.CanRead)
    {
        s.Read(buf, 0, buf.Length);
    }
}
9 Answers

I think the Stream class is poorly designed because in fact it violates the Single Responsibility Principle. In my opinion reading is something completely different than writing. Yet the Stream class allows both reading from & writing to the same stream. Moreover if we really think of a true stream, then we could hardly imagine how to seek a stream. In fact you can skip some bytes in a stream, but not move back (especially when writing). Imagine a stream of water with a bloody red liquid pouring in. The red liquid inside the stream flows in or out - it gives or consumes depending on the direction of the flow. There is no way to look on the stream in both directions at once. Trying to look in both directions at once leads to a Stream class known from .NET. To be more clear - encapsulating a class that breaks the SRP makes a much more code smell. Just look into the BufferedStream class source code into the Flush() method.

I come from the Java world, and for me the Input & Output stream separation is a natural thing. You may however like to work with a file, and to seek it forth and back - there is nothing wrong with it, unless you realize that a file is like a buffer that is stored somewhere, not a bidirectional stream. You may use a stream to fill that buffer, and then use another one to read it back. Still these two streaming operations are clearly separated logically.

Related