From e4314bce1b1fd284a68b53bebd66b6e36a555b1f Mon Sep 17 00:00:00 2001
From: Jordan Orelli <jorelli@etsy.com>
Date: Thu, 7 Aug 2014 18:01:21 +0000
Subject: [PATCH] reduce type waffling in progress

---
 values.go      | 19 ++++++++++++-------
 values_test.go | 46 +++++++++++++++++++++++-----------------------
 2 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/values.go b/values.go
index 828199c..fd964e3 100644
--- a/values.go
+++ b/values.go
@@ -102,17 +102,20 @@ func readValue(r io.Reader) (value, error) {
 	if line[len(line)-2] != '\r' {
 		return nil, fmt.Errorf("unable to read redis protocol value: bad line terminator: %q", line)
 	}
-	line = line[:len(line)-2]
 	switch line[0] {
 	case start_string:
-		return StringVal(line[1:]), nil
+		return StringVal(line), nil
 	case start_error:
+		line = line[:len(line)-2]
 		return Error(line[1:]), nil
 	case start_integer:
+		line = line[:len(line)-2]
 		return Integer(line[1:]), nil
 	case start_bulkstring:
+		line = line[:len(line)-2]
 		return readBulkString(line[1:], br)
 	case start_array:
+		line = line[:len(line)-2]
 		return readArray(line[1:], br)
 	default:
 		return nil, fmt.Errorf("unable to read redis protocol value: illegal start character: %c", line[0])
@@ -124,14 +127,16 @@ func readValue(r io.Reader) (value, error) {
 type StringVal []byte
 
 func (s StringVal) Write(w io.Writer) (int, error) {
-	w.Write([]byte{'+'})
-	w.Write(s)
-	w.Write([]byte{'\r', '\n'})
-	return 0, nil
+	return w.Write(s)
 }
 
 func String(s string) value {
-	return StringVal(s)
+	b := make(StringVal, len(s)+3)
+	b[0] = '+'
+	copy(b[1:], []byte(s))
+	b[len(b)-2] = '\r'
+	b[len(b)-1] = '\n'
+	return b
 }
 
 // ------------------------------------------------------------------------------
diff --git a/values_test.go b/values_test.go
index 87b563e..37f1ad3 100644
--- a/values_test.go
+++ b/values_test.go
@@ -30,31 +30,31 @@ func eq(v1, v2 value) bool {
 }
 
 func (test valueTest) run(t *testing.T) {
-	v, err := readValue(strings.NewReader(test.in + "\r\n"))
+	v, err := readValue(strings.NewReader(test.in))
 	if err != nil {
 		t.Errorf("valueTest error: %v", err)
 	}
 	if !eq(v, test.out) {
-		t.Errorf("expected %v, got %v", test.out, v)
+		t.Errorf("expected %q, got %q", test.out, v)
 	}
 }
 
 var valueTests = []valueTest{
-	{"+hello", String("hello")},
-	{"+one two", String("one two")},   // intermediate space
-	{"+one two ", String("one two ")}, // trailing space
-	{"+ one two", String(" one two")}, // leading space
+	{"+hello\r\n", String("hello")},
+	{"+one two\r\n", String("one two")},   // intermediate space
+	{"+one two \r\n", String("one two ")}, // trailing space
+	{"+ one two\r\n", String(" one two")}, // leading space
 
-	{"-hello", Error("hello")},
-	{"-one two", Error("one two")},   // intermediate space
-	{"-one two ", Error("one two ")}, // trailing space
-	{"- one two", Error(" one two")}, // leading space
+	{"-hello\r\n", Error("hello")},
+	{"-one two\r\n", Error("one two")},   // intermediate space
+	{"-one two \r\n", Error("one two ")}, // trailing space
+	{"- one two\r\n", Error(" one two")}, // leading space
 
 	{"$-1\r\n", nil},
-	// {"$0\r\n\r\n", BulkString("")}, // is this even a thing?
-	// {"$1\r\nx\r\n", BulkString("x")},
-	// {"$4\r\netsy\r\n", BulkString("etsy")},
-	// {"$12\r\nSaskatchewan\r\n", BulkString("Saskatchewan")},
+	{"$0\r\n\r\n", BulkString("")}, // is this even a thing?
+	{"$1\r\nx\r\n", BulkString("x")},
+	{"$4\r\netsy\r\n", BulkString("etsy")},
+	{"$12\r\nSaskatchewan\r\n", BulkString("Saskatchewan")},
 
 	// {":0", Integer(0)},
 	// {":1", Integer(1)},
@@ -64,15 +64,15 @@ var valueTests = []valueTest{
 	// {":9223372036854775807", Integer(9223372036854775807)},   // int64 max
 	// {":-9223372036854775808", Integer(-9223372036854775808)}, // int64 min
 
-	// {"+hello\r\n+extra\r\n", String("hello")},
-	// {"+one two\r\n+extra\r\n", String("one two")},   // intermediate space
-	// {"+one two \r\n+extra\r\n", String("one two ")}, // trailing space
-	// {"+ one two\r\n+extra\r\n", String(" one two")}, // leading space
+	{"+hello\r\n+extra\r\n", String("hello")},
+	{"+one two\r\n+extra\r\n", String("one two")},   // intermediate space
+	{"+one two \r\n+extra\r\n", String("one two ")}, // trailing space
+	{"+ one two\r\n+extra\r\n", String(" one two")}, // leading space
 
-	// {"-hello\r\n+extra\r\n", Error("hello")},
-	// {"-one two\r\n+extra\r\n", Error("one two")},   // intermediate space
-	// {"-one two \r\n+extra\r\n", Error("one two ")}, // trailing space
-	// {"- one two\r\n+extra\r\n", Error(" one two")}, // leading space
+	{"-hello\r\n+extra\r\n", Error("hello")},
+	{"-one two\r\n+extra\r\n", Error("one two")},   // intermediate space
+	{"-one two \r\n+extra\r\n", Error("one two ")}, // trailing space
+	{"- one two\r\n+extra\r\n", Error(" one two")}, // leading space
 
 	// {":0\r\n+extra\r\n", Integer(0)},
 	// {":1\r\n+extra\r\n", Integer(1)},
@@ -82,7 +82,7 @@ var valueTests = []valueTest{
 	// {":9223372036854775807\r\n+extra\r\n", Integer(9223372036854775807)},   // int64 max
 	// {":-9223372036854775808\r\n+extra\r\n", Integer(-9223372036854775808)}, // int64 min
 
-	// {"*-1\r\n", nil},    // nil array
+	{"*-1\r\n", nil}, // nil array
 	// {"*0\r\n", Array{}}, // is this a thing?  I have no idea.
 	// {"*1\r\n+hello\r\n", Array{String("hello")}},
 	// {"*2\r\n+one\r\n+two", Array{String("one"), String("two")}},