code review 245041: Add ReadRune and WriteRune to bytes.Buffer

Add ReadRune and WriteRune to bytes.Buffer

The comments mention bufio's WriteRune, which should come next.

[r, 2010-03-05 04:06:28]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[rsc, 2010-03-05 04:17:30]

LGTM

http://codereview.appspot.com/245041/diff/3001/4001
File src/pkg/bytes/buffer.go (right):

http://codereview.appspot.com/245041/diff/3001/4001#newcode186
src/pkg/bytes/buffer.go:186: // code point r to the buffer, returning its length
and
Drop the length?  WriteByte doesn't bother with it.
Don't care much either way.

http://codereview.appspot.com/245041/diff/3001/4001#newcode191
src/pkg/bytes/buffer.go:191: b.runeBytes[0] = uint8(r)
b.WriteByte(byte(r))

http://codereview.appspot.com/245041/diff/3001/4001#newcode236
src/pkg/bytes/buffer.go:236: // ReadRune reads and returns the next
UTF-8-encoded
Needs an err os.Error I believe.
bufio.Reader's ReadRune has it.

[r2, 2010-03-05 07:03:07]

On Mar 4, 2010, at 8:17 PM, rsc@golang.org wrote:

> LGTM
> 
> 
> 
> 
> http://codereview.appspot.com/245041/diff/3001/4001
> File src/pkg/bytes/buffer.go (right):
> 
> http://codereview.appspot.com/245041/diff/3001/4001#newcode186
> src/pkg/bytes/buffer.go:186: // code point r to the buffer, returning
> its length and
> Drop the length?  WriteByte doesn't bother with it.
> Don't care much either way.

i agonized over this.  WriteByte always has a count of 1.  WriteRune does not.
opinions welcome.


> http://codereview.appspot.com/245041/diff/3001/4001#newcode191
> src/pkg/bytes/buffer.go:191: b.runeBytes[0] = uint8(r)
> b.WriteByte(byte(r))

i was avoiding a trivial function call.  do you care?

> http://codereview.appspot.com/245041/diff/3001/4001#newcode236
> src/pkg/bytes/buffer.go:236: // ReadRune reads and returns the next
> UTF-8-encoded
> Needs an err os.Error I believe.
> bufio.Reader's ReadRune has it.

yeah, you're right.  a threesome to read a character is pretty heavyweight. 
sigh.

> http://codereview.appspot.com/245041/show

[rsc, 2010-03-05 08:38:05]

> i agonized over this.  WriteByte always has a count of 1.  WriteRune does not.
opinions welcome.

either way.
returning length means that the caller
can get invalid runes right
if it does care about length.
i don't think it matters much here,
because no one will check the error return anyway.

>> http://codereview.appspot.com/245041/diff/3001/4001#newcode191
>> src/pkg/bytes/buffer.go:191: b.runeBytes[0] = uint8(r)
>> b.WriteByte(byte(r))
>
> i was avoiding a trivial function call.  do you care?

no, but b.WriteByte(byte(r)) should be more
efficient than the combination of
b.runeBytes[0] = uint8(r) and b.Write(b.runeBytes[0:1]).

russ

[r, 2010-03-05 18:47:09]

Hello rsc (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2010-03-05 19:02:15]

LGTM

[r, 2010-03-05 19:35:07]

*** Submitted as http://code.google.com/p/go/source/detail?r=eac42b5e34d8 ***

Add ReadRune and WriteRune to bytes.Buffer

The comments mention bufio's WriteRune, which should come next.

R=rsc
CC=golang-dev
http://codereview.appspot.com/245041