golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.66k stars 17.49k forks source link

tour: rot13Reader hint #12880

Open jonburgin opened 8 years ago

jonburgin commented 8 years ago

Context: http://tour.golang.org/methods/12

Great tutorial! If I may humbly suggest, it would have helped me to be told that all characters in a reader should be processed even if there is an error. I put a guard statement to not bother to do the rot13 if the enclosed reader returned an error, for efficiency reasons. Although obvious after the fact, I didn't discover the source of my problem until I read the docs on Reader.

ianlancetaylor commented 8 years ago

CC @adg @campoy

rakyll commented 8 years ago

it would have helped me to be told that all characters in a reader should be processed even if there is an error.

Is it? func (r rot13Reader) Read can return an error anytime and you can don't have to consume all the characters from s if an error happens -- even though I don't see where an error can happen in this particular exercise during Read. Could you provide a snippet from your solution to make it a bit clear?

jonburgin commented 8 years ago

There isn't a problem with the exercise, just my ignorance, but basically I did a guard pattern, to prevent doing extra work. Sorry about the syntax, I'm doing this off the top of my head and I'm still a 'go' novice I did something like:

if count, err := otherReader.read(myByteBuffer); err{ return 0, err

} //do rot13 code here

But when I did that, none of the data was returned. I should have processed the data as well, since the original reader returned both all the data AND an EOF

The reference docs, that clued me in can be found at:

https://golang.org/pkg/io/#Reader

Specifically: "Callers should always process the n > 0 bytes returned before considering the error err. "

The bold text above would have prevented me from my original error. I don't think guard conditions are an unusual pattern, but including one here caused the program to silently fail. Anyhow, no big deal, thought it would improve the exercise, but I understand the desired for brevity, so I wouldn't argue if you think the inclusion is too verbose.

Have a good one, and thanks for looking into it! Jon

On Tue, Oct 13, 2015 at 4:34 PM, rakyll notifications@github.com wrote:

it would have helped me to be told that all characters in a reader should be processed even if there is an error.

Is it? func (r rot13Reader) Read can return an error anytime and you can don't have to consume all the characters from s if an error happens -- even though I don't see where an error can happen in this particular exercise during Read. Could you provide a snippet from your solution to make it a bit clear?

— Reply to this email directly or view it on GitHub https://github.com/golang/go/issues/12880#issuecomment-147860680.

kytrinyx commented 8 years ago

There's an open issue about making the reader example less confusing: #11373

@jonburgin I'm having trouble figuring out what the code looked like that led you to getting a silent failure. Without a clear idea of what went wrong it's difficult to come up with a good explanation that would have helped you avoid it.

I'm pretty sure that having if count, err := otherReader.read(myByteBuffer); err { would have been a compile error. Do you recall if you said err != nil?

jonburgin commented 8 years ago

Yes, err != nil

On Wed, Oct 14, 2015 at 2:00 PM, Katrina Owen notifications@github.com wrote:

There's an open issue about making the reader example less confusing:

11373 https://github.com/golang/go/issues/11373

@jonburgin https://github.com/jonburgin I'm having trouble figuring out what the code looked like that led you to getting a silent failure. Without a clear idea of what went wrong it's difficult to come up with a good explanation that would have helped you avoid it.

I'm pretty sure that having if count, err := otherReader.read(myByteBuffer); err { would have been a compile error. Do you recall if you said err != nil?

— Reply to this email directly or view it on GitHub https://github.com/golang/go/issues/12880#issuecomment-148156866.