bufio: add function to read a certain amount of bytes

[n10v]

In many programs, where I use bufio package, lacks the function, that can read certain amount of bytes.
So now I should create function like this:

func ReadSeveralBytes(r *bufio.Reader, n int) ([]byte, error) {
    if n == 0 || r == nil {
        return nil, nil
    }

    peeked, err := r.Peek(n)
    if err != nil {
        return nil, err
    }

    if _, err := r.Discard(n); err != nil {
        return nil, err
    }

    return peeked, nil
}

But it's inconvenient and I think, the way it works could be better.
I hope, what I'm not the only one, who wants it in bufio package.

[Thomasdezeeuw]

Take a look at https://golang.org/pkg/io/#ReadFull. I think that is what you want. Although I'm not sure why you would use Peek and then Discard.

[n10v]

Hm, I didn't see this function earlier, but it's closer to what I want, but not the perfect.
I should always allocate new buffer. It consumes more memory than just the way Peek function works.

[uluyol]

Doesn't

b := make([]byte, n)
got, err := r.Read(b)

do exactly this?

[bradfitz]

@uluyol, that allocates, and the io.Reader contract doesn't guarantee to read the requested number of bytes. Peek will load them in.

[n10v]

Look, I have this function in my project:

// ReadSeveralBytes reads n bytes.
func (r *Reader) ReadSeveralBytes(n int) ([]byte, error) {
    if n == 0 {
        return nil, nil
    }

    peeked, err := r.buf.Peek(n)
    if err != nil {
        return nil, err
    }

    if _, err := r.buf.Discard(n); err != nil {
        return nil, err
    }

    return peeked, nil
}

If it will allocate a slice on every call of function, it will consume a lot of memory. But Peek just returns the slice of already allocated array, so it consumes noticeably less memory, what is very important in my project.

[dsnet]

At least the naming of ReadSeveralBytes seems off to me since it makes it seem like I can pass in an integer like 1<<20 and it would return me a 1MiB byte slice. The documentation of Peek is clear that the method fails if the requested size is greater than the internal buffer. This is a subtle detail and I think the number of methods that bufio has with subtle conditions attached should be limited.

[bradfitz]

I agree. I'm going to close this issue. I'm happy we got bufio.Discard added because it enables helpers like this one, but I don't think bufio needs it itself. It's useful sometimes, but not enough to add to the docs and overwhelm most users with the cognitive overload of many too-similar functions.

[n10v]

@bradfitz So, when I want to read several bytes, what should I use? Peek + Discard?
And I don't think, what library will be overwhelmed with this simple function, whose purpose is very clear. I think, what this function will be a good and logic addition to bufio.
I am sure, that I'm not the only one, who needs similar method in standard library. And it's not so similar as other functions. Peek just peeks some chunk of memory, but doesn't advance the reader. Discard just advances the reader and doesn't return any memory slice. And this function will read several bytes and advance buffers. It's very easy to understand and I'm sure, what it won't overwhelm bufio library.
@dsnet Yes, I agree, what the naming isn't good. We should think about a good name.
I think, it should be noticed in documentation for this method, what the method fails if the requested size is greater than the internal buffer as you said and what user should always check the returned byte slice and error.

[bradfitz]

So, when I want to read several bytes, what should I use? Peek + Discard?

Just using Read is the most clear. But that requires a copy and you have to have a buffer to read into (or for you to allocate). If you need the performance and don't have a buffer handy, then use Peek + Discard.

And I don't think, what library will be overwhelmed with this simple function, whose purpose is very clear. I think, what this function will be a good and logic addition to bufio.

Users who need this already can do it, using the function you already wrote. I don't think it needs to be part of the standard library. I think it would do more harm (cognitive overload, documentation clutter) than good (benefiting a few users, reducing their need to write a small function).

[n10v]

Ok, I agree with you.
There are already many ways to do it:

1. Read
2. io.ReadFull
3. Peek + Discard

And it will overwhelm library. Now I understood this. Thanks for explanation and sorry for wasting your time!