code review 4969049: bytes: EqualString

bytes: EqualString

[bradfitz, 2011-08-29 16:19:51]

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

I'd like you to review this change to
https://go.googlecode.com/hg/

[gri, 2011-08-29 16:23:51]

LGTM

[gri, 2011-08-29 16:25:07]

But wait for Russ' opinion.

One question: Why shouldn't this be in strings?
- Robert

On Mon, Aug 29, 2011 at 9:23 AM,  <gri@golang.org> wrote:
> LGTM
>
> http://codereview.appspot.com/4969049/
>

[bradfitz, 2011-08-29 16:27:34]

On Mon, Aug 29, 2011 at 9:25 AM, Robert Griesemer <gri@golang.org> wrote:

> But wait for Russ' opinion.
>
> One question: Why shouldn't this be in strings?
>

Arbitrary, I suppose.

bytes is about []bytes
strings is about strings
this is one of both

*shrug*

[kevlar, 2011-08-29 17:11:35]

>
> One question: Why shouldn't this be in strings?
>

I'd paint my bikeshed bytes, because in most of the cases where I am messing
with a []byte and want to compare it to a string, I've already imported
bytes for one of its other functions; but I'm usually manipulating something
from an io.Reader/io.Writer, so someone who has a more string-driven
pipeline would probably see the opposite.

[rsc, 2011-08-29 17:25:08]

I am a little worried about this being the
thin edge of a (string, bytes) cross-product wedge.
What's the situation where this is important
enough to have?

If you're doing many comparisons then
existing code typically does a single conversion
outside the loop and then uses == or bytes.Equal.

Russ

[bradfitz, 2011-08-29 17:53:06]

I'm not going to fight for this to go in, but there's a bunch of other
trivial functions in package bytes and strings.  If there's a line, I don't
know where it is.

I can just include this in my own code when I want to avoid copies.


On Mon, Aug 29, 2011 at 10:25 AM, Russ Cox <rsc@golang.org> wrote:

> I am a little worried about this being the
> thin edge of a (string, bytes) cross-product wedge.
> What's the situation where this is important
> enough to have?
>
> If you're doing many comparisons then
> existing code typically does a single conversion
> outside the loop and then uses == or bytes.Equal.
>
> Russ
>

[rsc, 2011-08-29 17:59:34]

On Mon, Aug 29, 2011 at 13:53, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> I'm not going to fight for this to go in, but there's a bunch of other
> trivial functions in package bytes and strings.  If there's a line, I don't
> know where it is.
> I can just include this in my own code when I want to avoid copies.

This might be below the line, as long as it's
not the first of many.  Let's wait and see what Rob thinks.

Russ

[r2, 2011-08-29 21:24:47]

On 30/08/2011, at 3:59 AM, Russ Cox wrote:

> On Mon, Aug 29, 2011 at 13:53, Brad Fitzpatrick <bradfitz@golang.org> wrote:
>> I'm not going to fight for this to go in, but there's a bunch of other
>> trivial functions in package bytes and strings.  If there's a line, I don't
>> know where it is.
>> I can just include this in my own code when I want to avoid copies.
> 
> This might be below the line, as long as it's
> not the first of many.  Let's wait and see what Rob thinks.

Rob is afraid of the strings and bytes packages filling up with hard-to-remember
helpers.  Although he does have some sympathy for this function, the fact that
it's uncertain which package it belongs in pushes him towards the "no" camp.

It would help if these packages had a known structure.

-Rob, for rob

[bradfitz, 2011-08-29 21:27:56]

On Mon, Aug 29, 2011 at 2:24 PM, Rob 'Commander' Pike <r@google.com> wrote:

>
> On 30/08/2011, at 3:59 AM, Russ Cox wrote:
>
> > On Mon, Aug 29, 2011 at 13:53, Brad Fitzpatrick <bradfitz@golang.org>
> wrote:
> >> I'm not going to fight for this to go in, but there's a bunch of other
> >> trivial functions in package bytes and strings.  If there's a line, I
> don't
> >> know where it is.
> >> I can just include this in my own code when I want to avoid copies.
> >
> > This might be below the line, as long as it's
> > not the first of many.  Let's wait and see what Rob thinks.
>
> Rob is afraid of the strings and bytes packages filling up with
> hard-to-remember helpers.  Although he does have some sympathy for this
> function, the fact that it's uncertain which package it belongs in pushes
> him towards the "no" camp.
>
> It would help if these packages had a known structure.
>

I could add strings.EqualBytes(s string, b []byte) for symmetry.  :-)

*ducks*

[gustavo_niemeyer.net, 2011-08-29 22:54:53]

> I could add strings.EqualBytes(s string, b []byte) for symmetry.  :-)

More seriously, why doing

    if strings.EqualsBytes(s, b) { ... }

rather than

    if string(b) == s { ... }

If this is just about saving the conversion, should this be done by
the compiler?

-- 
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I never filed a patent.

[gri, 2011-08-29 23:03:29]

On Mon, Aug 29, 2011 at 3:54 PM, Gustavo Niemeyer <gustavo@niemeyer.net> wrote:
>> I could add strings.EqualBytes(s string, b []byte) for symmetry.  :-)
>
> More seriously, why doing
>
>    if strings.EqualsBytes(s, b) { ... }
>
> rather than
>
>    if string(b) == s { ... }
>
> If this is just about saving the conversion, should this be done by
> the compiler?

+1
- gri

[bradfitz, 2011-08-29 23:04:47]

On Mon, Aug 29, 2011 at 4:03 PM, Robert Griesemer <gri@golang.org> wrote:

> On Mon, Aug 29, 2011 at 3:54 PM, Gustavo Niemeyer <gustavo@niemeyer.net>
> wrote:
> >> I could add strings.EqualBytes(s string, b []byte) for symmetry.  :-)
> >
> > More seriously, why doing
> >
> >    if strings.EqualsBytes(s, b) { ... }
> >
> > rather than
> >
> >    if string(b) == s { ... }
> >
> > If this is just about saving the conversion, should this be done by
> > the compiler?
>
> +1
> - gri
>

Related:  (previously discussed)
http://code.google.com/p/go/issues/detail?id=2204

[dave_cheney.net, 2011-08-29 23:38:56]

>> If this is just about saving the conversion, should this be done by
>> the compiler?
>
> +1
> - gri

+1