io: make MultiReader documentation more explicit

[gopherbot]

by ryguillian:

I'm running Go 1.2.

My issues are about io.MultiReader: I went to go use MultiReader and its behavior
confused me.

MultiReader's documentation sez: 

“MultiReader returns a Reader that's the logical concatenation of the provided input
readers. They're read sequentially. Once all inputs are drained, Read will return EOF.”

This documentation is pretty bad. It only says what happens once “all inputs are
drained”—whatever the hell that means. 

I wish this were just a bug report about documentation, but I'm sad to say I think
MultiReader's behavior is pretty dumb.

Like, uh, is MultiReader.Read supposed to stop after the first Reader returns EOF?
Because that's what it does.

In other words, if I run `cat file1 file2 file3` I get the concatenation of file1,
file2, and file3. Even if file2 is empty! I only use this example because Go is replete
with analogies to Unix commandlineisms (io.MultiWriter mentions 'tee(1)' explicitly).
And in case differs drastically from the expected behavior. Although in a talk I saw
online Rob Pike said something about breaking analogies, so I guess it's OK. Right?

Go is obviously very opinionated software, and that's good. But if you want the user to
use io.ReadAtLeast to get the expected behavior then document it. Although I'd argue
that using io.ReadAtLeast to invoke a continuation (think about it) is a little too
subtle for me.

I'd argue for changing the behavior of this, though. It's weird because you're providing
an abstraction over Readers and then still preserving reader boundaries. I guess this
all has to work with network code where things can get unpredictable. If that's what led
to this wacky design, then maybe offer a non-network-oriented MultiReader. This seems
more like a 'ReaderPool' or something.

It just seems hard to imagine a case where you'd have intermittently available resources
somewhere and want to still read them sequentially. I suppose I could contrive an
example, but this can't be the #1 use case. I can imagine a lot more situations where
you'd want to do something like a queue and use MultiReader to read from the queue if
it's not empty and then read from an underlying Reader otherwise. "And then"
is implicit when you use a word like "logical concatenation".

So yeah, I'd call for the redesign of
io.MultiReader.readLessThanSliceLengthOrIfEOFReturnNilCallAgainIfYouWantMore.

Thanks,

Ryan

P.S. The only code in the Go 'pkg' directory that uses MultiReader is
'io/multireader_test.go'.

[gopherbot]

Comment 1 by ryguillian:

“And in case differs drastically” should be “And in this case differs
drastically”.
Sorry about that.

[minux]

Comment 2:

I don't understand what you've said. Could you wirte a small program what do you think
MultiReader is buggy?
> is MultiReader.Read supposed to stop after the first Reader returns EOF?
Yes, (*multireader).Read will return after the first Reader returns EOF, however, it
won't return EOF
itself, so I don't think you can call that "stop" as that signals the caller that it can
always
call Read again.

Status changed to WaitingForReply.

[bradfitz]

Comment 3:

The bug tracker is not for ranting.
There are no implementation issues here, and your P.S. is wrong. A simple grep finds
other uses. And many uses in outside projects (e.g. Camlistore has 12)
If you have specific documentation suggestions, discuss first on golang-nuts and see if
others agree.  If so, then file a bug.  We've had many documentation issues in the past,
but I don't agree this is a major problem.  The docs seem accurate and clear to me, at
least.

Status changed to WorkingAsIntended.

[gopherbot]

Comment 4 by ryguillian:

You're right Brad. I was wrong. It appears in 'net/http' (once) and 'mime/multipart'
(once).
Thank you for your insight on this difficult and subtle issue.

[adg]

Comment 5:

@ryguillian I can't tell if you're being sarcastic. Are you actually going to give us a
clear description of the buggy behavior? Or an example program?
I've used MultiReader a handful of times and it's always behaved the way I expect, but
I'm willing to believe there's an edge case lurking somewhere.

[minux]

Comment 6:

I think this issue originate from misunderstanding of how io.Reader works.
(e.g. Read is allowed to return 0, nil, and it's treated as a no-op, and it's also
allowed to return less than len(buf) and non-nil error, etc.)
I also believe the MultiReader implementation is as correct as it can be.

[bradfitz]

Comment 7:

What does this part mean:
> Although I'd argue that using io.ReadAtLeast to invoke a continuation (think about it)
is a
> little too subtle for me.
I thought about it, but I'm still confused.

[gopherbot]

Comment 8 by ryguillian:

When you call Read on MultiReader with a slice larger than what's left in the rest of
the file MultiReader yields rather than trying to keep reading. As pointed out, this is
more of a pathological design in Go. It's like a tacit "yield" using n < len(slice)
&& err==nil as a "more to read" signal. If the Go team thinks this is okay, fine. I
would be interested in the rationale for this pervasive design.

[adg]

Comment 9:

There is always more to read until you get io.EOF or another error.
http://golang.org/pkg/io/#Reader

[adg]

Comment 10:

Also there is no "yield" in Go. It would be clearer if you would use the same
terminology as the language spec where possible.
It is common for a Read to return n < len(slice). This is by design, so that
many wrapped readers can share the same buffer and avoid copying. It has worked very
well so far. I think you are just misunderstanding the io.Reader design.

[gopherbot]

Comment 11 by ryguillian:

Alright this is getting obnoxious. I wasn't trying to be sarcastic or a pain in the
neck, I was honestly confused. I'm not an idiot. I know Go doesn't have a 'yield'
statement. io.MultiReader.Read returns in the middle of a for loop once it receives
io.EOF from an underlying reader. Fine. I get it. I know how it works. _As I said_ this
is _obviously_ a _design decision_ that's _pervasive_ in Go. Okay?
Here's what io.Reader's documentation that a...@golang.org linked to:
'If some data is available but not len(p) bytes, Read conventionally returns what is
available instead of waiting for more.'
What does 'available' mean? You have to be joking if you tell me this is clear. Also, it
says 'conventionally'. MultiReader is already inside of a for loop. Also, why is the
data from the next reader in mr.readers 'not available'? Can anybody actually explain
the actually design philosophy behind this or is it just made-up and to be intrepidly
defended because that's what we say and that's what goes? If that's the attitude at Go,
fine, but then count you user base one less.

[adg]

Comment 12:

I did not intend to be obnoxious. I was just asking you to be precise (just as you are
asking of us), so that everyone is clear on what is being discussed.
As for io.Reade, what "available" means depends on the context. io.Reader is a general
interface implemented by many different things. For instance, a Read from a network
connection might just return whatever data has appeared on the wire (as handed to the Go
program from the kernel by a read syscall).
But then another io.Reader implementation might decide to wait until the Read buffer is
full before returning (this is unusual, and as such should be documented by the
implementor, hence the "conventionally" in the io.Reader docs).
Both are valid implementations of io.Reader.
But as a user of io.Reader, you must always repeatedly call Read until you get io.EOF.
I'm not seeing how this is "pathalogical" though.
Take a look at the various io.Reader implementations in the tree to get an idea of how
this works in practice.

[minux]

Comment 13:

As andrew explained, we first made the decision in the design of io.Reader that
it is allowed to not fill the buffer (btw, it's also the design of the read(2) syscall
after which io.Reader is modeled).
Given that, MultiReader's behavior is perfectly fine.
If you want to read the buffer full, use io.ReadFull, which saves a lot of trouble.
And by looking at the source of io.ReadAtLeast you will find that actually filling
the buffer is difficult that what you think. Duplicate all that logic in MultiReader
doesn't make sense given ReadFull already implements it.
Honestly, do you want to duplicate that logic in every io.Reader implementation
you write? Try take a look at the compress/* Readers and see why it's not
feasible to always fill the buffer passed in.

[gopherbot]

Comment 14 by ryguillian:

Thank you. I'll write up something more comprehensive on golang-nuts. I'm crossing my
fingers that 'the code is the documentation' isn't Go's actual attitude as expressed
above. Worse is better?
Anyway, please close the issue.

[adg]

Comment 15:

The documentation describes the "what." If you want to implement or use an io.Reader,
the docs explain how it works. You're asking *why* the io.Reader is so designed. We're
suggesting that you take a look at the various implementations of io.Reader to get a
feel for the various possibilities that the interface's specification provides.
FWIW, you have received a somewhat chilly response in this bug because you launched an
all out "I think you guys should change this," before actually understanding the system.
The issue is already closed.