code review 4969049: bytes: EqualString (issue 4969049)

[brad...@golang.org]

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@googlegroups.com (cc: golan...@googlegroups.com),

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

Description:
bytes: EqualString

Please review this at http://codereview.appspot.com/4969049/

Affected files:
M src/pkg/bytes/bytes.go
M src/pkg/bytes/bytes_test.go

Index: src/pkg/bytes/bytes.go
===================================================================
--- a/src/pkg/bytes/bytes.go
+++ b/src/pkg/bytes/bytes.go
@@ -49,6 +49,19 @@
return true
}

+// EqualString returns a boolean reporting whether string(a) == b.
+func EqualString(a []byte, b string) bool {
+ if len(a) != len(b) {
+ return false
+ }
+ for i, c := range a {
+ if c != b[i] {
+ return false
+ }
+ }
+ return true
+}
+
// explode splits s into an array of UTF-8 sequences, one per Unicode
character (still arrays of bytes),
// up to a maximum of n byte arrays. Invalid UTF-8 sequences are chopped
into individual bytes.
func explode(s []byte, n int) [][]byte {
Index: src/pkg/bytes/bytes_test.go
===================================================================
--- a/src/pkg/bytes/bytes_test.go
+++ b/src/pkg/bytes/bytes_test.go
@@ -65,12 +65,16 @@
b := []byte(tt.b)
cmp := Compare(a, b)
eql := Equal(a, b)
+ eqls := EqualString(a, tt.b)
if cmp != tt.i {
t.Errorf(`Compare(%q, %q) = %v`, tt.a, tt.b, cmp)
}
if eql != (tt.i == 0) {
t.Errorf(`Equal(%q, %q) = %v`, tt.a, tt.b, eql)
}
+ if eqls != (tt.i == 0) {
+ t.Errorf(`EqualString(%q, %q) = %v`, tt.a, tt.b, eqls)
+ }
}
}

[g...@golang.org]

LGTM

http://codereview.appspot.com/4969049/

[Robert Griesemer]

But wait for Russ' opinion.

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

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

[Brad Fitzpatrick]

On Mon, Aug 29, 2011 at 9:25 AM, Robert Griesemer <g...@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*

[Kyle Lemons]

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.

[Russ Cox]

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

[Brad Fitzpatrick]

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.

[Russ Cox]

On Mon, Aug 29, 2011 at 13:53, Brad Fitzpatrick <brad...@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

[Rob 'Commander' Pike]

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

[Brad Fitzpatrick]

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

*ducks*

[Gustavo Niemeyer]

> 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.

[Robert Griesemer]

On Mon, Aug 29, 2011 at 3:54 PM, Gustavo Niemeyer <gus...@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

[Brad Fitzpatrick]

Related:  (previously discussed)

http://code.google.com/p/go/issues/detail?id=2204

[Dave Cheney]

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

+1