Repository: elliotchance/pie
Pull request #134 (2019-05-12 16:21:42)

Issues/133 join more efficient impl
See #133 

In the end I didn't call `strings.Join` directly (would make it harder to compile `join.go` for `type StringSliceType []StringElementType`). Also, I didn't copy-paste the stdlib impl.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/elliotchance/pie/134)
<!-- Reviewable:end -->


codecov-io: # [Codecov](https://codecov.io/gh/elliotchance/pie/pull/134?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@dc0f90e`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/elliotchance/pie/pull/134/graphs/tree.svg?width=650&token=OvugwUQGM3&height=150&src=pr)](https://codecov.io/gh/elliotchance/pie/pull/134?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##             master   #134   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?      8           
  Lines             ?   1277           
  Branches          ?      0           
=======================================
  Hits              ?   1277           
  Misses            ?      0           
  Partials          ?      0
```


| [Impacted Files](https://codecov.io/gh/elliotchance/pie/pull/134?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pie/strings\_pie.go](https://codecov.io/gh/elliotchance/pie/pull/134/diff?src=pr&el=tree#diff-cGllL3N0cmluZ3NfcGllLmdv) | `100% <100%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/elliotchance/pie/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/elliotchance/pie/pull/134?src=pr&el=footer). Last update [dc0f90e...8a34029](https://codecov.io/gh/elliotchance/pie/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Deleplace: IIt seems that `mightBeString.String()` is _always_ replaced with `fmt.Sprintf("%v", mightBeString)`

Patch:
diff --git a/README.md b/README.md
index 99b96cb..aade08c 100644
--- a/README.md
+++ b/README.md
@@ -196,7 +196,7 @@ The letters in brackets indicate:
 | [`Float64s`](#float64s) (S)             | ✓      | ✓      | ✓      |      | n        | Float64s transforms each element to a float64.  |
 | [`Intersect`](#intersect)               | ✓      | ✓      |        |      | n        | Intersect returns items that exist in all lists.  |
 | [`Ints`](#ints) (S)                     | ✓      | ✓      | ✓      |      | n        | Ints transforms each element to an integer.  |
-| [`Join`](#join)                         | ✓      |        |        |      | n        | Join returns a string from joining each of the elements.  |
+| [`Join`](#join) (S)                     | ✓      | ✓      | ✓      |      | n        | Join returns a string from joining each of the elements.  |
 | [`JSONBytes`](#jsonbytes)               | ✓      | ✓      | ✓      |      | n        | JSONBytes returns the JSON encoded array as bytes.  |
 | [`JSONBytesIndent`](#jsonbytesindent)   | ✓      | ✓      | ✓      |      | n        | JSONBytesIndent returns the JSON encoded array as bytes with indent applied.  |
 | [`JSONString`](#jsonstring)             | ✓      | ✓      | ✓      |      | n        | JSONString returns the JSON encoded array as a string.  |
diff --git a/functions/join.go b/functions/join.go
index 9dc1963..e6be743 100644
--- a/functions/join.go
+++ b/functions/join.go
@@ -1,14 +1,21 @@
 package functions
 
+import "strings"
+
 // Join returns a string from joining each of the elements.
-func (ss StringSliceType) Join(glue string) (s string) {
-	for i, element := range ss {
-		if i > 0 {
-			s += glue
-		}
+func (ss SliceType) Join(glue string) (s string) {
+	var slice interface{} = []ElementType(ss)
 
-		s += string(element)
+	if y, ok := slice.([]string); ok {
+		// The stdlib is efficient for type []string
+		return strings.Join(y, glue)
+	} else {
+		// General case
+		parts := make([]string, len(ss))
+		for i, element := range ss {
+			mightBeString := element
+			parts[i] = mightBeString.String()
+		}
+		return strings.Join(parts, glue)
 	}
-
-	return s
 }
diff --git a/functions/main.go b/functions/main.go
index 8c81e99..6ad997d 100644
--- a/functions/main.go
+++ b/functions/main.go
@@ -43,7 +43,7 @@ var Functions = []struct {
 	{"Float64s", "float64s.go", ForAll, "n"},
 	{"Intersect", "intersect.go", ForNumbersAndStrings, "n"},
 	{"Ints", "ints.go", ForAll, "n"},
-	{"Join", "join.go", ForStrings, "n"},
+	{"Join", "join.go", ForAll, "n"},
 	{"JSONBytes", "json_bytes.go", ForAll, "n"},
 	{"JSONBytesIndent", "json_bytes_indent.go", ForAll, "n"},
 	{"JSONString", "json_string.go", ForAll, "n"},
diff --git a/pie/carpointers_pie.go b/pie/carpointers_pie.go
index 8b67355..b248b76 100755
--- a/pie/carpointers_pie.go
+++ b/pie/carpointers_pie.go
@@ -8,6 +8,7 @@ import (
 	"math/rand"
 	"sort"
 	"strconv"
+	"strings"
 )
 
 // All will return true if all callbacks return true. It follows the same logic
@@ -284,6 +285,24 @@ func (ss carPointers) Ints() Ints {
 	return result
 }
 
+// Join returns a string from joining each of the elements.
+func (ss carPointers) Join(glue string) (s string) {
+	var slice interface{} = []*car(ss)
+
+	if y, ok := slice.([]string); ok {
+		// The stdlib is efficient for type []string
+		return strings.Join(y, glue)
+	} else {
+		// General case
+		parts := make([]string, len(ss))
+		for i, element := range ss {
+			mightBeString := element
+			parts[i] = fmt.Sprintf("%v", mightBeString)
+		}
+		return strings.Join(parts, glue)
+	}
+}
+
 // JSONBytes returns the JSON encoded array as bytes.
 //
 // One important thing to note is that it will treat a nil slice as an empty
diff --git a/pie/cars_pie.go b/pie/cars_pie.go
index a51f4ce..0808f74 100755
--- a/pie/cars_pie.go
+++ b/pie/cars_pie.go
@@ -8,6 +8,7 @@ import (
 	"math/rand"
 	"sort"
 	"strconv"
+	"strings"
 )
 
 // All will return true if all callbacks return true. It follows the same logic
@@ -284,6 +285,24 @@ func (ss cars) Ints() Ints {
 	return result
 }
 
+// Join returns a string from joining each of the elements.
+func (ss cars) Join(glue string) (s string) {
+	var slice interface{} = []car(ss)
+
+	if y, ok := slice.([]string); ok {
+		// The stdlib is efficient for type []string
+		return strings.Join(y, glue)
+	} else {
+		// General case
+		parts := make([]string, len(ss))
+		for i, element := range ss {
+			mightBeString := element
+			parts[i] = fmt.Sprintf("%v", mightBeString)
+		}
+		return strings.Join(parts, glue)
+	}
+}
+
 // JSONBytes returns the JSON encoded array as bytes.
 //
 // One important thing to note is that it will treat a nil slice as an empty
diff --git a/pie/float64s_pie.go b/pie/float64s_pie.go
index fc67b8c..d58bc17 100755
--- a/pie/float64s_pie.go
+++ b/pie/float64s_pie.go
@@ -8,6 +8,7 @@ import (
 	"math/rand"
 	"sort"
 	"strconv"
+	"strings"
 )
 
 // Abs is a function which returns the absolute value of all the
@@ -357,6 +358,24 @@ func (ss Float64s) Ints() Ints {
 	return result
 }
 
+// Join returns a string from joining each of the elements.
+func (ss Float64s) Join(glue string) (s string) {
+	var slice interface{} = []float64(ss)
+
+	if y, ok := slice.([]string); ok {
+		// The stdlib is efficient for type []string
+		return strings.Join(y, glue)
+	} else {
+		// General case
+		parts := make([]string, len(ss))
+		for i, element := range ss {
+			mightBeString := element
+			parts[i] = fmt.Sprintf("%v", mightBeString)
+		}
+		return strings.Join(parts, glue)
+	}
+}
+
 // JSONBytes returns the JSON encoded array as bytes.
 //
 // One important thing to note is that it will treat a nil slice as an empty
diff --git a/pie/ints_pie.go b/pie/ints_pie.go
index d75bcdf..d9d044f 100755
--- a/pie/ints_pie.go
+++ b/pie/ints_pie.go
@@ -8,6 +8,7 @@ import (
 	"math/rand"
 	"sort"
 	"strconv"
+	"strings"
 )
 
 // Abs is a function which returns the absolute value of all the
@@ -357,6 +358,24 @@ func (ss Ints) Ints() Ints {
 	return result
 }
 
+// Join returns a string from joining each of the elements.
+func (ss Ints) Join(glue string) (s string) {
+	var slice interface{} = []int(ss)
+
+	if y, ok := slice.([]string); ok {
+		// The stdlib is efficient for type []string
+		return strings.Join(y, glue)
+	} else {
+		// General case
+		parts := make([]string, len(ss))
+		for i, element := range ss {
+			mightBeString := element
+			parts[i] = fmt.Sprintf("%v", mightBeString)
+		}
+		return strings.Join(parts, glue)
+	}
+}
+
 // JSONBytes returns the JSON encoded array as bytes.
 //
 // One important thing to note is that it will treat a nil slice as an empty
diff --git a/pie/strings_pie.go b/pie/strings_pie.go
index 8fb1191..3b6afdf 100755
--- a/pie/strings_pie.go
+++ b/pie/strings_pie.go
@@ -8,6 +8,7 @@ import (
 	"math/rand"
 	"sort"
 	"strconv"
+	"strings"
 )
 
 // All will return true if all callbacks return true. It follows the same logic
@@ -335,15 +336,20 @@ func (ss Strings) Ints() Ints {
 
 // Join returns a string from joining each of the elements.
 func (ss Strings) Join(glue string) (s string) {
-	for i, element := range ss {
-		if i > 0 {
-			s += glue
-		}
+	var slice interface{} = []string(ss)
 
-		s += string(element)
+	if y, ok := slice.([]string); ok {
+		// The stdlib is efficient for type []string
+		return strings.Join(y, glue)
+	} else {
+		// General case
+		parts := make([]string, len(ss))
+		for i, element := range ss {
+			mightBeString := element
+			parts[i] = fmt.Sprintf("%v", mightBeString)
+		}
+		return strings.Join(parts, glue)
 	}
-
-	return s
 }
 
 // JSONBytes returns the JSON encoded array as bytes.
diff --git a/template.go b/template.go
index a93e7ca..d0fc78e 100644
--- a/template.go
+++ b/template.go
@@ -495,17 +495,24 @@ func (ss SliceType) JSONStringIndent(prefix, indent string) string {
 `,
 	"Join": `package functions
 
+import "strings"
+
 // Join returns a string from joining each of the elements.
-func (ss StringSliceType) Join(glue string) (s string) {
-	for i, element := range ss {
-		if i > 0 {
-			s += glue
-		}
+func (ss SliceType) Join(glue string) (s string) {
+	var slice interface{} = []ElementType(ss)
 
-		s += string(element)
+	if y, ok := slice.([]string); ok {
+		// The stdlib is efficient for type []string
+		return strings.Join(y, glue)
+	} else {
+		// General case
+		parts := make([]string, len(ss))
+		for i, element := range ss {
+			mightBeString := element
+			parts[i] = mightBeString.String()
+		}
+		return strings.Join(parts, glue)
 	}
-
-	return s
 }
 `,
 	"Keys": `package functions


Test patch:
diff --git a/pie/carpointers_test.go b/pie/carpointers_test.go
index f37fc41..c3bec79 100644
--- a/pie/carpointers_test.go
+++ b/pie/carpointers_test.go
@@ -1336,3 +1336,10 @@ func TestCarPointers_ShiftAndUnshift(t *testing.T) {
 		})
 	}
 }
+
+func TestCarPointers_Join(t *testing.T) {
+	assert.Equal(t, "", carPointers(nil).Join("a"))
+	assert.Equal(t, "", carPointers{}.Join("a"))
+	car1, car2 := &car{Name: "maverick", Color: "red"}, &car{Name: "ferrari", Color: "red"}
+	assert.Equal(t, "maverick is red-ferrari is red", carPointers{car1, car2}.Join("-"))
+}
diff --git a/pie/cars_test.go b/pie/cars_test.go
index 66ab8fc..b04621b 100644
--- a/pie/cars_test.go
+++ b/pie/cars_test.go
@@ -1309,3 +1309,10 @@ func TestCars_ShiftAndUnshift(t *testing.T) {
 		})
 	}
 }
+
+func TestCars_Join(t *testing.T) {
+	assert.Equal(t, "", cars(nil).Join("a"))
+	assert.Equal(t, "", cars{}.Join("a"))
+	car1, car2 := car{Name: "maverick", Color: "red"}, car{Name: "ferrari", Color: "red"}
+	assert.Equal(t, "{maverick red}-{ferrari red}", cars{car1, car2}.Join("-"))
+}
diff --git a/pie/float64s_test.go b/pie/float64s_test.go
index 4b73933..afdfe12 100644
--- a/pie/float64s_test.go
+++ b/pie/float64s_test.go
@@ -1482,3 +1482,9 @@ func TestFloat64s_ShiftAndUnshift(t *testing.T) {
 		})
 	}
 }
+func TestFloat64s_Join(t *testing.T) {
+	assert.Equal(t, "", Float64s(nil).Join("a"))
+	assert.Equal(t, "", Float64s{}.Join("a"))
+	var f1, f2 float64 = 0.1, 20000000000000000
+	assert.Equal(t, "0.1;2e+16", Float64s{f1, f2}.Join(";"))
+}
diff --git a/pie/ints_test.go b/pie/ints_test.go
index b2e88d4..a92490b 100644
--- a/pie/ints_test.go
+++ b/pie/ints_test.go
@@ -1471,3 +1471,9 @@ func TestInts_ShiftAndUnshift(t *testing.T) {
 		})
 	}
 }
+func TestInts_Join(t *testing.T) {
+	assert.Equal(t, "", Ints(nil).Join("a"))
+	assert.Equal(t, "", Ints{}.Join("a"))
+	assert.Equal(t, "1-2-3", Ints{1, 2, 3}.Join("-"))
+	assert.Equal(t, "1--2-3", Ints{1, -2, 3}.Join("-"))
+}
diff --git a/pie/strings_bench_test.go b/pie/strings_bench_test.go
new file mode 100644
index 0000000..69d178f
--- /dev/null
+++ b/pie/strings_bench_test.go
@@ -0,0 +1,25 @@
+package pie
+
+import (
+	"strings"
+	"testing"
+)
+
+func BenchmarkStringsJoin(b *testing.B) {
+	ss := Strings{"A", "utility", "library", "for", "dealing", "with", "slices", "and", "maps", "that", "focuses", "on", "type", "safety", "and", "performance"}
+	for i := 0; i < b.N; i++ {
+		result := ss.Join(" ")
+		sinkString = result
+	}
+}
+
+func BenchmarkStdlibStringsJoin(b *testing.B) {
+	ss := Strings{"A", "utility", "library", "for", "dealing", "with", "slices", "and", "maps", "that", "focuses", "on", "type", "safety", "and", "performance"}
+	for i := 0; i < b.N; i++ {
+		result := strings.Join(ss, " ")
+		sinkString = result
+	}
+}
+
+// Prevent compiler from agressively optimizing away the result
+var sinkString string