Is the .NET Stream class poorly designed? [closed]

Question

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);
    }
}

Menorrhagia

What are you actually going to do if CanRead returns false? Usually, this is a failure condition: you might as well just call Read and live with the exception... – Phrensy

Re your edit: No, because as you say, if you can't read from a stream which you expect to be able to read from, an exception is the right way to go, which will be thrown by s.Read. – Benoit

Answer 1

I think the classes are designed nicely. I would much rather check a property then attempt to do something and have to catch an exception. Interfaces fall short in the case of stream types that are of multiple "types". What type would be returned from a method that gets you a readable and writable stream? I agree the design isn't a true Object Oriented Design, but do you really want to treat streams in that manner? Some of the properties can change if the stream is closed or something else changes, what would happen in that case?

I think this question brings up a really interesting experiment though, why not try to design your own stream related classes. Publish your redesign on CodePlex or Google Code, it would be a great learning experience and would result in a potentially useful library for others to use.

Panicle

> Interfaces fall short in the case of stream types that are of multiple "types". I would say the opposite. You can derive from only one class but as many interfaces as you like. – Menorrhagia

Yet if you really need/want multiple classes you can build adapter classes to wrap several types and have implict casting operators. – Trenna

@I. J. Kennedy, My point is if you get the stream back from a method call, what type will it be? How will you know it is readable or writable? Type checking? – Panicle

The capabilities would be part of the type, where they should be. The method would not return a Stream, but an IReadableStream. – Oosphere

So if the stream was readable and writeable you'd get an IReadableWritableStream? – Panicle

No, you could get an object that implements both IReadable and IWritable. – Eisenstark

@Bob: That's how I'd do it. If IReadableWritableStream were a framework type which inherited IReadableStream and IWritableStream, an implementation of IReadableWritableStream would be no more work than an implementation of a maybe-does-everything IStream. For that matter, if one wanted all the "CanXX" methods, one could have IStream as a stream of unspecified capabilities, and have IRead, IWriteStream, IReadWriteStreamPair, IQueueStream, ISeekableReadStream, and ISeekableReadWriteStream, etc. all derive from it (and in some cases each other) without any new members. – Chirk

@Bob: Note that IQueueStream and IReadWriteStreamPair would both have the same set of 'working' methods, but different expected semantics; IQueueStream would be a stream where the reader and writer would be expected to behave as different ends of the same pipe, while IReadWriteStringPair would be expected to behave as a bidirectional pipe with something else at the other end (e.g. a TCP port). – Chirk

Answer 2

Using interfaces would mean that the value of "CanRead" couldn't be changed at runtime. The "FileStream" class changes the "CanRead" property based on the current state of the file.

Festoon

Could you elaborate on that? I don't get why CanRead couldn't be changed at runtime. – Parlous

If you close the stream CanRead = false – Panicle

devoured elysium: the OP is suggesting that readable streams would implement IReadable rather than having a CanRead property. With CanRead, if a stream becomes unreadable, it can return false from CanRead. But with IReadable, it wouldn't be able to remove IReadable from its type declaration. – Kusin

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? – Menorrhagia

The CanRead isn't the only reason you would need a lock around a stream. A Stream is a single resource that will cause plenty of problems if you pound on it from several Threads in an unorganized manner. – Trenna

Answer 3

They probably didn't use interfaces because, at the time, there were no extension methods. If you wanted every stream to have things like a default ReadByte method, you needed to use a class.

I wrote a blog post a couple months back about why I don't like IO.Stream and what I thought should be done. Essentially it boils down to streams not being very type-safe.

Oosphere

+1 I read your blog post, and yes, that is exactly what I'm talking about. – Menorrhagia

Would it make you happier if all the boilerplate properties were virtual and threw NotSupportedException by default? It would sure make implementing it a bit nicer. – Camelopardus

Additionally, Close is a legacy semantic when dealing with IO. Like a lot of things in c#, it is there to make transferring from WINAPI and C++ much easier. (goto, for example :P) – Camelopardus

+1 for the blog post. I was going to reply to it, but I think I'll just post my answer here. – Disjoint

Answer 4

Interfaces can be overused, and this would be one of those cases. I think the current design is great. The fact that streams can change capabilities at runtime means that IReadable/IWritable/ISeekable would not obviate the need for CanRead, CanWrite and CanSeek, so you would just be increasing the complexity for no real gain other than eliminating a handful of stub methods and properties in your derived classes.

Personally I prefer that a stream class be easier to use than easier to write, because you'll be writing it once and using it many times.

Substance

@Gerald: Good point, but it's more than just the annoying stubs. Take a look at the updated part of the question and tell me if I'm off base. Seems like even using this Stream design correctly is difficult too. – Menorrhagia

@IJ, to touch on your update, it could be either an exceptional condition, or an expected condition. You may have a design where a stream is temporarily unreadable, and since you know that will be the case it's not really exceptional. In that case you'll want to use a lock. On the other hand, you can also rely on exceptions if you want to, where it is would truly be an exceptional case. – Substance

Answer 5

Class Stream uses Optional feature pattern, you can read more here.

Tanka

That was a very interesting read, highly recommended for anyone who stumbles upon this SO thread. – Restaurateur

Interesting document. Conceptually, though, I think it should be possible to use something like enum-based approach to offer some kinds of extensibility which would otherwise be awkward. For example, if after the original design is released it turns out that it would be useful to know whether some method is known to be computable fast, or is known to be computable only slowly, having a function that returns information about an object as an enum that has spare bits available would allow existing implementations to support that function (indicating that the method isn't known to be fast... – Chirk

...but isn't known to be slow either). Also, if there exist methods which accept an enum indicating what they are supposed to do, and a static system method which can be provide a sensible default behavior, then new capabilities can be defined and used, without breaking existing implementations of the interface. – Chirk

Answer 6

To answer the question: Probably.

I pretty much agree with @Strilanc's answer that Yes it is poorly implemented, but I thought I'd go ahead and post my thoughts as well.

While it certainly would be cleaner to implement this stuff with composable interfaces and extension methods (now that they're available), .NET 1 didn't have those features, so I can understand why they chose to design Stream that way.

However, now that we have really useful constructs like Generics and extension methods, I think it's time to revisit a lot of the original classes and mark them with ObsoleteAttribute. Of course, you'd want a replacement API first.

ObsoleteAttribute would allow those classes to remain as part of the framework while simultaneously discouraging their use. These classes could then be removed in a future version of the framework or maybe even kept in but not available in a different profile of the framework.

This would apply to ALL non-generic collections (not just those in the System.Collections namespace) and "weird abstractions". The System.Text.RegularExpressions *Collection classes are good examples, but there's so many more of those throughout the framework.

I know that my "weird abstractions" comment is extremely vague, but I guess the litmus test would be that if you have to throw NotImplementedExceptions all over your derived classes, there's probably a better way to do things.

So in summary, I don't like the design of Stream either, but until there's a better API, you probably just have to deal with it. Writing Adapters/Wrappers for the existing Stream is probably your best bet for now, and it's what I'm currently doing.

Disjoint

Answer 7

You could take the (Java*) approach of writing a mostly-empty MyStreamclass that inherits the base Streamclass, but provides most of the member methods (e.g., CanSeek()) and embuing them with reasonable default behavior (e.g., throwing NotImplemented). Then your real class simply extends your MyStreamclass, implementing the two or three remaining methods that you really need.

As you reuse your MyStreamclass, you'll save a lot of reinventing of the wheel.

* This is called an abstract adapter class in the Java libraries.

Froma

Nice idea... totally obvious but never occurred to me :) – Turbinal

Answer 8

My issue with the stream class is the Length property - it doesn't specify how to implement a stream of unknown, unspecified, or infinite length!

Hereinto

See msdn.microsoft.com/en-us/library/system.io.stream.length.aspx If the stream doesn't support seeking (which would be the case for something with unknown length) then throw a NotSupportedException. – Augean

@Richard: Since a stream might have a known length and yet not support seeking, does that mean that the only way for client code to determine whether a stream has known length is to ask for it and catch the exception? – Chirk

Answer 9

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.

Crosseyed