code review 7042045: bytes: Examples recommending bytes.Compare(a, b) rel_op...

bytes: Examples recommending bytes.Compare(a, b) rel_op 0 to test a rel_op b.

[mdempsky, 2013-01-03 22:45:13]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[minux1, 2013-01-04 20:32:07]

Go doesn't support relational operations for slices, so
i don't know why we mention them here.

https://codereview.appspot.com/7042045/diff/5001/src/pkg/bytes/bytes.go
File src/pkg/bytes/bytes.go (right):

https://codereview.appspot.com/7042045/diff/5001/src/pkg/bytes/bytes.go#newco...
src/pkg/bytes/bytes.go:14: // Compare returns an integer comparing two byte
arrays lexicographically.
while you're at here, please s/arrays/slices/

[mdempsky, 2013-01-04 21:33:07]

On 2013/01/04 20:32:07, minux wrote:
> Go doesn't support relational operations for slices, so
> i don't know why we mention them here.

The Go language doesn't support rel_ops on slices, but I think it's still
reasonable to logically discuss orderings on them.  E.g., Compare's raison
d'être is to define a byte-wise lexicographical ordering on byte slices, and its
godoc already uses the notations "a < b" and "a > b" in reference to this
ordering even though they're not syntactically valid Go expressions.

The intent of this CL is just to document that if you want to test "x <= y",
then since the Go language doesn't allow that expression when x and y are byte
slices, an idiomatic alternative would be to use "Compare(x, y) <= 0".  Further,
this pattern holds for any rel_op.

This is an idiom I'm accustomed to from C programming for interpreting
strcmp/memcmp/etc results, but it took me a moment too long of carefully
considering the Compare() return values to make sure the idiom still worked
here.  I couldn't find any other document (e.g., Effective Go) that suggested
this idiom, so I thought a concise comment might help others and establish a
consistent and easy-to-read convention.

Of course, opinions may differ on whether "Compare(a, b) == -1" or "Compare(a,
b) < 0" is preferable.  IMO the latter is easier to read, but I'm not a
committer so I defer judgement. ;)

https://codereview.appspot.com/7042045/diff/5001/src/pkg/bytes/bytes.go
File src/pkg/bytes/bytes.go (right):

https://codereview.appspot.com/7042045/diff/5001/src/pkg/bytes/bytes.go#newco...
src/pkg/bytes/bytes.go:14: // Compare returns an integer comparing two byte
arrays lexicographically.
On 2013/01/04 20:32:07, minux wrote:
> while you're at here, please s/arrays/slices/

I believe nigeltao intends to fix this in
https://codereview.appspot.com/7028051/ (and also included my "the" removal), so
I've reverted this part of the CL.

[rsc, 2013-01-04 23:31:28]

Instead of writing text, how about writing an ExampleCompare function
showing uses with < 0, == 0, and >0.

[mdempsky, 2013-01-04 23:59:20]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[adg, 2013-01-06 22:03:35]

https://codereview.appspot.com/7042045/diff/17001/src/pkg/bytes/example_test.go
File src/pkg/bytes/example_test.go (right):

https://codereview.appspot.com/7042045/diff/17001/src/pkg/bytes/example_test....
src/pkg/bytes/example_test.go:33: // It's idiomatic to interpret Compare's
result by comparing it to 0.
// Interpret Compare's result by comparing it to zero.

[mdempsky, 2013-01-06 22:48:10]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org,
adg@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[adg, 2013-01-06 22:58:57]

LGTM

[adg, 2013-01-06 22:59:56]

*** Submitted as https://code.google.com/p/go/source/detail?r=9ef7b839accd ***

bytes: Examples recommending bytes.Compare(a, b) rel_op 0 to test a rel_op b.

R=golang-dev, minux.ma, rsc, adg
CC=golang-dev
https://codereview.appspot.com/7042045

Committer: Andrew Gerrand <adg@golang.org>

[rsc, 2013-01-07 00:26:18]

https://codereview.appspot.com/7042045/diff/15002/src/pkg/bytes/example_test.go
File src/pkg/bytes/example_test.go (right):

https://codereview.appspot.com/7042045/diff/15002/src/pkg/bytes/example_test....
src/pkg/bytes/example_test.go:62: // Return needle <= haystack[i].
Please use haystack[i] >= needle, to match the sort.Search documentation.
(And fix the next line and the Equal call.)

That is, sort.Search's docs say:

    To complete the example above, the following code tries to find the
    value x in an integer slice data sorted in ascending order:

	x := 23
	i := sort.Search(len(data), func(i int) bool { return data[i] >= x })
	if i < len(data) && data[i] == x {
		// x is present at data[i]
	} else {
		// x is not present in data,
		// but i is the index where it would be inserted.
	}

This should use the same comparisons and operand orders.