Improve reply helper functions.

- Improve documentation.
- Add Scan function.
- Remove int64 to []byte and string type conversions. It's probably an
  if the application received an integer value when expected []byte or
  string.
- Bonus: fix test in experiment package.

README.markdown

@@ -11,6 +11,7 @@ Features
 * [Publish/Subscribe](http://go.pkgdoc.org/github.com/garyburd/redigo/redis#Publish_and_Subscribe).
 * [Connection pooling](http://go.pkgdoc.org/github.com/garyburd/redigo/redis#Pool).
 * [Script helper object](http://go.pkgdoc.org/github.com/garyburd/redigo/redis#Script) with optimistic use of EVALSHA.
+* [Helper functions](http://go.pkgdoc.org/pkg/github.com/garyburd/redigo/redis#Reply_Helpers) for working with command replies.
 
 Documentation
 -------------

exp/scan/struct_test.go

@@ -83,7 +83,7 @@ var formatStructTests = []struct {
 
 func TestFormatStruct(t *testing.T) {
 	for _, tt := range formatStructTests {
-		args := scan.FormatStruct(tt.value)
+		args := scan.AppendStruct(nil, tt.value)
 		if !reflect.DeepEqual(args, tt.args) {
 			t.Fatalf("FormatStruct(%s) returned %v, want %v", tt.title, args, tt.args)
 		}

redis/doc.go

@@ -135,4 +135,16 @@
 //  if err != nil {
 //      // handle error return from c.Do or type conversion error.
 //  }
+//
+// The Scan function converts elements of a multi-bulk reply to Go types:
+//
+//  var value1 int
+//  var value2 string
+//  mb, err := redis.MultiBulk(c.Do("MGET", "key1", "key2"))
+//  if err != nil {
+//      // handle error
+//  }
+//   if _, err := redis.Scan(mb, &value1, &value2); err != nil {
+//      // handle error
+//  }
 package redis

redis/pubsub.go

@@ -100,7 +100,7 @@ func (c PubSubConn) Receive() interface{} {
 	}
 
 	var kind string
-	multiBulk, err = Values(multiBulk, &kind)
+	multiBulk, err = Scan(multiBulk, &kind)
 	if err != nil {
 		return err
 	}
@@ -108,19 +108,19 @@ func (c PubSubConn) Receive() interface{} {
 	switch kind {
 	case "message":
 		var m Message
-		if _, err := Values(multiBulk, &m.Channel, &m.Data); err != nil {
+		if _, err := Scan(multiBulk, &m.Channel, &m.Data); err != nil {
 			return err
 		}
 		return m
 	case "pmessage":
 		var pm PMessage
-		if _, err := Values(multiBulk, &pm.Pattern, &pm.Channel, &pm.Data); err != nil {
+		if _, err := Scan(multiBulk, &pm.Pattern, &pm.Channel, &pm.Data); err != nil {
 			return err
 		}
 		return pm
 	case "subscribe", "psubscribe", "unsubscribe", "punsubscribe":
 		s := Subscription{Kind: kind}
-		if _, err := Values(multiBulk, &s.Channel, &s.Count); err != nil {
+		if _, err := Scan(multiBulk, &s.Channel, &s.Count); err != nil {
 			return err
 		}
 		return s

redis/reply.go

@@ -22,6 +22,7 @@ import (
 
 var ErrNil = errors.New("redigo: nil returned")
 
+// Values is deprecated. Use Scan instead.
 func Values(multiBulk []interface{}, values ...interface{}) ([]interface{}, error) {
 	if len(multiBulk) < len(values) {
 		return nil, errors.New("redigo Values: short multibulk")
@@ -50,130 +51,134 @@ func Values(multiBulk []interface{}, values ...interface{}) ([]interface{}, erro
 	return multiBulk[len(values):], err
 }
 
-// Int is a helper that converts a Redis reply to an int. 
+// Int is a helper that converts a command reply to an integer. If err is not
+// equal to nil, then Int returns 0, err. Otherwise, Int converts the
+// reply to an int as follows:
 //
 //  Reply type    Result
-//  integer       return reply as int
-//  bulk          parse decimal integer from reply
-//  nil           return error ErrNil
-//  other         return error
-//  
-func Int(v interface{}, err error) (int, error) {
+//  integer       int(reply), nil
+//  bulk          strconv.ParseInt(reply, 10, 0)
+//  nil           0, ErrNil
+//  other         0, error
+func Int(reply interface{}, err error) (int, error) {
 	if err != nil {
 		return 0, err
 	}
-	switch v := v.(type) {
+	switch reply := reply.(type) {
 	case int64:
-		return int(v), nil
+		x := int(reply)
+		if int64(x) != reply {
+			return 0, strconv.ErrRange
+		}
+		return x, nil
 	case []byte:
-		n, err := strconv.ParseInt(string(v), 10, 0)
+		n, err := strconv.ParseInt(string(reply), 10, 0)
 		return int(n), err
 	case nil:
 		return 0, ErrNil
 	case Error:
-		return 0, v
+		return 0, reply
 	}
-	return 0, fmt.Errorf("redigo: unexpected type for Int, got type %T", v)
+	return 0, fmt.Errorf("redigo: unexpected type for Int, got type %T", reply)
 }
 
-// String is a helper that converts a Redis reply to a string. 
+// String is a helper that converts a command reply to a string. If err is not
+// equal to nil, then String returns "", err. Otherwise String converts the
+// reply to a string as follows:
 //
 //  Reply type      Result
-//  integer         format as decimal string
-//  bulk            return reply as string
-//  string          return as is
-//  nil             return error ErrNil
-//  other           return error
-func String(v interface{}, err error) (string, error) {
+//  bulk            string(reply), nil
+//  string          reply, nil
+//  nil             "",  ErrNil
+//  other           "",  error
+func String(reply interface{}, err error) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	switch v := v.(type) {
+	switch reply := reply.(type) {
 	case []byte:
-		return string(v), nil
+		return string(reply), nil
 	case string:
-		return v, nil
-	case int64:
-		return strconv.FormatInt(v, 10), nil
+		return reply, nil
 	case nil:
 		return "", ErrNil
 	case Error:
-		return "", v
+		return "", reply
 	}
-	return "", fmt.Errorf("redigo: unexpected type for String, got type %T", v)
+	return "", fmt.Errorf("redigo: unexpected type for String, got type %T", reply)
 }
 
-// Bytes is a helper that converts a Redis reply to slice of bytes. 
+// Bytes is a helper that converts a command reply to a slice of bytes. If err
+// is not equal to nil, then Bytes returns nil, err. Otherwise Bytes converts
+// the reply to a slice of bytes as follows:
 //
 //  Reply type      Result
-//  integer         format as decimal string
-//  bulk            return reply as slice of bytes
-//  string          return reply as slice of bytes
-//  nil             return error ErrNil
-//  other           return error
-func Bytes(v interface{}, err error) ([]byte, error) {
+//  bulk            reply, nil
+//  string          []byte(reply), nil
+//  nil             nil, ErrNil
+//  other           nil, error
+func Bytes(reply interface{}, err error) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	switch v := v.(type) {
+	switch reply := reply.(type) {
 	case []byte:
-		return v, nil
+		return reply, nil
 	case string:
-		return []byte(v), nil
-	case int64:
-		return strconv.AppendInt(nil, v, 10), nil
+		return []byte(reply), nil
 	case nil:
 		return nil, ErrNil
 	case Error:
-		return nil, v
+		return nil, reply
 	}
-	return nil, fmt.Errorf("redigo: unexpected type for Bytes, got type %T", v)
+	return nil, fmt.Errorf("redigo: unexpected type for Bytes, got type %T", reply)
 }
 
-// Bool is a helper that converts a Redis reply to a bool. 
+// Bool is a helper that converts a command reply to a boolean. If err is not
+// equal to nil, then Bool returns false, err. Otherwise Bool converts the
+// reply to boolean as follows:
 //
 //  Reply type      Result
-//  integer         return value != 0
-//  bulk            return false if reply is "" or "0", otherwise return true.
-//  nil             return error ErrNil
-//  other           return error
-func Bool(v interface{}, err error) (bool, error) {
+//  integer         value != 0, nil
+//  bulk            strconv.ParseBool(reply) 
+//  nil             false, ErrNil
+//  other           false, error
+func Bool(reply interface{}, err error) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	switch v := v.(type) {
+	switch reply := reply.(type) {
 	case int64:
-		return v != 0, nil
+		return reply != 0, nil
 	case []byte:
-		if len(v) == 0 || (len(v) == 1 && v[0] == '0') {
-			return false, nil
-		}
-		return true, nil
+		return strconv.ParseBool(string(reply))
 	case nil:
 		return false, ErrNil
 	case Error:
-		return false, v
+		return false, reply
 	}
-	return false, fmt.Errorf("redigo: unexpected type for Bool, got type %T", v)
+	return false, fmt.Errorf("redigo: unexpected type for Bool, got type %T", reply)
 }
 
-// MultiBulk is a helper that converts a Redis reply to a []interface{}. 
+// MultiBulk is a helper that converts a command reply to a []interface{}. If
+// err is not equal to nil, then MultiBulk returns nil, err. Otherwise,
+// MultiBulk converts the reply as follows:
 //
 //  Reply type      Result
-//  multi-bulk      return []interface{}
-//  nil             return error ErrNil
-//  other           return error
-func MultiBulk(v interface{}, err error) ([]interface{}, error) {
+//  multi-bulk      reply, nil
+//  nil             nil, ErrNil
+//  other           nil, error
+func MultiBulk(reply interface{}, err error) ([]interface{}, error) {
 	if err != nil {
 		return nil, err
 	}
-	switch v := v.(type) {
+	switch reply := reply.(type) {
 	case []interface{}:
-		return v, nil
+		return reply, nil
 	case nil:
 		return nil, ErrNil
 	case Error:
-		return nil, v
+		return nil, reply
 	}
-	return nil, fmt.Errorf("redigo: unexpected type for MultiBulk, got type %T", v)
+	return nil, fmt.Errorf("redigo: unexpected type for MultiBulk, got type %T", reply)
 }

redis/scan.go

@@ -0,0 +1,156 @@
+// Copyright 2012 Gary Burd
+//
+// Licensed under the Apache License, Version 2.0 (the "License"): you may
+// not use this file except in compliance with the License. You may obtain
+// a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations
+// under the License.
+
+package redis
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+	"strconv"
+)
+
+func cannotConvert(d reflect.Value, s interface{}) error {
+	return fmt.Errorf("redigo: Scan cannot convert from %s to %s",
+		reflect.TypeOf(s), d.Type())
+}
+
+func convertAssignBulk(d reflect.Value, s []byte) (err error) {
+	switch d.Type().Kind() {
+	case reflect.Float32, reflect.Float64:
+		var x float64
+		x, err = strconv.ParseFloat(string(s), d.Type().Bits())
+		d.SetFloat(x)
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		var x int64
+		x, err = strconv.ParseInt(string(s), 10, d.Type().Bits())
+		d.SetInt(x)
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
+		var x uint64
+		x, err = strconv.ParseUint(string(s), 10, d.Type().Bits())
+		d.SetUint(x)
+	case reflect.Bool:
+		var x bool
+		x, err = strconv.ParseBool(string(s))
+		d.SetBool(x)
+	case reflect.String:
+		d.SetString(string(s))
+	case reflect.Slice:
+		if d.Type().Elem().Kind() != reflect.Uint8 {
+			err = cannotConvert(d, s)
+		} else {
+			d.SetBytes(s)
+		}
+	default:
+		err = cannotConvert(d, s)
+	}
+	return
+}
+
+func convertAssignInt(d reflect.Value, s int64) (err error) {
+	switch d.Type().Kind() {
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		d.SetInt(s)
+		if d.Int() != s {
+			err = strconv.ErrRange
+			d.SetInt(0)
+		}
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
+		if s < 0 {
+			err = strconv.ErrRange
+		} else {
+			x := uint64(s)
+			d.SetUint(x)
+			if d.Uint() != x {
+				err = strconv.ErrRange
+				d.SetUint(0)
+			}
+		}
+	case reflect.Bool:
+		d.SetBool(s != 0)
+	default:
+		err = cannotConvert(d, s)
+	}
+	return
+}
+
+// Scan copies from a multi-bulk command reply to the values pointed at by
+// dest. 
+// 
+// The values pointed at by test must be a numeric type, boolean, string or a
+// byte slice. Scan uses the standard strconv package to convert bulk values to
+// numeric and boolean types. 
+//
+// If the multi-bulk value is nil, then the corresponding dest value is not
+// modified. 
+//
+// To enable easy use of Scan in a loop, Scan returns the slice following the
+// copied values.
+func Scan(multiBulk []interface{}, dest ...interface{}) ([]interface{}, error) {
+
+	// We handle the most common dest types using type switches and fallback to
+	// reflection for all other types.
+
+	if len(multiBulk) < len(dest) {
+		return nil, errors.New("redigo: Scan multibulk short")
+	}
+	var err error
+	for i, d := range dest {
+		switch s := multiBulk[i].(type) {
+		case nil:
+			// ingore
+		case []byte:
+			switch d := d.(type) {
+			case *string:
+				*d = string(s)
+			case *int:
+				*d, err = strconv.Atoi(string(s))
+			case *bool:
+				*d, err = strconv.ParseBool(string(s))
+			case *[]byte:
+				*d = s
+			default:
+				if d := reflect.ValueOf(d); d.Type().Kind() != reflect.Ptr {
+					err = cannotConvert(d, s)
+				} else {
+					err = convertAssignBulk(d.Elem(), s)
+				}
+			}
+		case int64:
+			switch d := d.(type) {
+			case *int:
+				x := int(s)
+				if int64(x) != s {
+					err = strconv.ErrRange
+					x = 0
+				}
+				*d = x
+			case *bool:
+				*d = s != 0
+			default:
+				if d := reflect.ValueOf(d); d.Type().Kind() != reflect.Ptr {
+					err = cannotConvert(d, s)
+				} else {
+					err = convertAssignInt(d.Elem(), s)
+				}
+			}
+		default:
+			err = cannotConvert(reflect.ValueOf(d), s)
+		}
+		if err != nil {
+			break
+		}
+	}
+	return multiBulk[len(dest):], err
+}

redis/scan_test.go

@@ -0,0 +1,128 @@
+// Copyright 2012 Gary Burd
+//
+// Licensed under the Apache License, Version 2.0 (the "License"): you may
+// not use this file except in compliance with the License. You may obtain
+// a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations
+// under the License.
+
+package redis_test
+
+import (
+	"fmt"
+	"github.com/garyburd/redigo/redis"
+	"math"
+	"reflect"
+	"testing"
+)
+
+var scanConversionTests = []struct {
+	src  interface{}
+	dest interface{}
+}{
+	{[]byte("-inf"), math.Inf(-1)},
+	{[]byte("+inf"), math.Inf(1)},
+	{[]byte("0"), float64(0)},
+	{[]byte("3.14159"), float64(3.14159)},
+	{[]byte("3.14"), float32(3.14)},
+	{[]byte("-100"), int(-100)},
+	{[]byte("101"), int(101)},
+	{int64(102), int(102)},
+	{[]byte("103"), uint(103)},
+	{int64(104), uint(104)},
+	{[]byte("105"), int8(105)},
+	{int64(106), int8(106)},
+	{[]byte("107"), uint8(107)},
+	{int64(108), uint8(108)},
+	{[]byte("0"), false},
+	{int64(0), false},
+	{[]byte("f"), false},
+	{[]byte("1"), true},
+	{int64(1), true},
+	{[]byte("t"), true},
+	{[]byte("hello"), "hello"},
+	{[]byte("world"), []byte("world")},
+}
+
+var scanConversionErrorTests = []struct {
+	src  interface{}
+	dest interface{}
+}{
+	{[]byte("1234"), byte(0)},
+	{int64(1234), byte(0)},
+	{[]byte("-1"), byte(0)},
+	{int64(-1), byte(0)},
+	{[]byte("junk"), false},
+}
+
+func TestScanConversion(t *testing.T) {
+	for _, tt := range scanConversionTests {
+		multiBulk := []interface{}{tt.src}
+		dest := reflect.New(reflect.TypeOf(tt.dest))
+		multiBulk, err := redis.Scan(multiBulk, dest.Interface())
+		if err != nil {
+			t.Errorf("Scan(%v) returned error %v", tt, err)
+			continue
+		}
+		if !reflect.DeepEqual(tt.dest, dest.Elem().Interface()) {
+			t.Errorf("Scan(%v) returned %v, want %v", tt, dest.Elem().Interface(), tt.dest)
+		}
+	}
+}
+
+func TestScanConversionError(t *testing.T) {
+	for _, tt := range scanConversionErrorTests {
+		multiBulk := []interface{}{tt.src}
+		dest := reflect.New(reflect.TypeOf(tt.dest))
+		multiBulk, err := redis.Scan(multiBulk, dest.Interface())
+		if err == nil {
+			t.Errorf("Scan(%v) did not return error", tt)
+		}
+	}
+}
+
+func ExampleScan() {
+	c, err := dial()
+	if err != nil {
+		panic(err)
+	}
+	defer c.Close()
+
+	c.Send("HMSET", "album:1", "title", "Red", "rating", 5)
+	c.Send("HMSET", "album:2", "title", "Earthbound", "rating", 1)
+	c.Send("HMSET", "album:3", "title", "Beat")
+	c.Send("LPUSH", "albums", "1")
+	c.Send("LPUSH", "albums", "2")
+	c.Send("LPUSH", "albums", "3")
+	multiBulk, err := redis.MultiBulk(c.Do("SORT", "albums",
+		"BY", "album:*->rating",
+		"GET", "album:*->title",
+		"GET", "album:*->rating"))
+	if err != nil {
+		panic(err)
+	}
+
+	for len(multiBulk) > 0 {
+		var title string
+		rating := -1 // initialize to illegal value to detect nil.
+		multiBulk, err = redis.Scan(multiBulk, &title, &rating)
+		if err != nil {
+			panic(err)
+		}
+		if rating == -1 {
+			fmt.Println(title, "not-rated")
+		} else {
+			fmt.Println(title, rating)
+		}
+	}
+	// Output:
+	// Beat not-rated
+	// Earthbound 1
+	// Red 5
+}