Repository: elliotchance/pie
Pull request #194 (2023-06-08 18:01:36)

Add Remove function
I believe checking index and bounds is better than keep it without the check and panicking with a bit weird error.

chocolacula: @elliotchance any thoughts?
chocolacula: > I don't like the way it uses an output parameter. It should return a new slice not containing the element. Also, Delete a more common term used by languages when deleting something by it's key. And finally, are you sure this works with a single element? It looks like it would panic. 

Done. One element will be deleted correctly. Added a test for that case.
chocolacula: I tried a few approaches and chose the best one. I added a little benchmark:
```
BenchmarkDelete-8                   3372           355714 ns/op          8003676 B/op      3 allocs/op
BenchmarkDelete-8                   2890           357530 ns/op          8003675 B/op      3 allocs/op
BenchmarkDeleteShort-8          14604861           80.06 ns/op           176 B/op          3 allocs/op
BenchmarkDeleteShort-8          14294144           80.04 ns/op           176 B/op          3 allocs/op
BenchmarkDeleteSet-8                 184           6437496 ns/op         8003588 B/op      1 allocs/op
BenchmarkDeleteSet-8                 184           6431757 ns/op         8003589 B/op      1 allocs/op
BenchmarkDeleteSetShort-8        8599725           137.4 ns/op           128 B/op          1 allocs/op
BenchmarkDeleteSetShort-8        8649344           137.3 ns/op           128 B/op          1 allocs/op
```

You can find them in separate branch in [delete_test.go](https://github.com/chocolacula/pie/blob/delete_benchmarks/v2/delete_test.go)
chocolacula: @elliotchance please review

Patch:
diff --git a/v2/delete.go b/v2/delete.go
new file mode 100644
index 0000000..6c254d1
--- /dev/null
+++ b/v2/delete.go
@@ -0,0 +1,35 @@
+package pie
+
+import "sort"
+
+// Removes elements at indices in idx from input slice, returns resulting slice.
+// If an index is out of bounds, skip it.
+func Delete[T any](ss []T, idx ...int) []T {
+	// short path O(n) inplace
+	if len(idx) == 1 {
+		i := idx[0]
+
+		if i < 0 || i >= len(ss) {
+			return ss
+		}
+		return append(ss[:i], ss[i+1:]...)
+	}
+
+	// long path O(mLog(m) + n)
+	sort.Ints(idx)
+
+	ss2 := make([]T, 0, len(ss))
+
+	prev := 0
+	for _, i := range idx {
+		if i < 0 || i >= len(ss) {
+			continue
+		}
+		// Copy by consecutive chunks instead of one by one
+		ss2 = append(ss2, ss[prev:i]...)
+		prev = i + 1
+	}
+	ss2 = append(ss2, ss[prev:]...)
+
+	return ss2
+}
diff --git a/v2/of.go b/v2/of.go
index 90a0b30..e6f44e9 100644
--- a/v2/of.go
+++ b/v2/of.go
@@ -203,3 +203,9 @@ func (o OfSlice[T]) Top(n int) OfSlice[T] {
 func (o OfSlice[T]) Unshift(elements ...T) OfSlice[T] {
 	return OfSlice[T]{Unshift(o.Result, elements...)}
 }
+
+// Removes element at index in idx from input slice, returns resulting slice.
+// If an index in idx out of bounds, skip it.
+func (o OfSlice[T]) Delete(idx ...int) OfSlice[T] {
+	return OfSlice[T]{Delete(o.Result, idx...)}
+}
diff --git a/v2/of_numeric.go b/v2/of_numeric.go
index 0a9b4fc..1507b24 100644
--- a/v2/of_numeric.go
+++ b/v2/of_numeric.go
@@ -2,8 +2,9 @@ package pie
 
 import (
 	"context"
-	"golang.org/x/exp/constraints"
 	"math/rand"
+
+	"golang.org/x/exp/constraints"
 )
 
 // OfNumeric encapsulates a slice to be used in multiple chained operations.
@@ -231,3 +232,7 @@ func (o OfNumericSlice[T]) Unique() OfNumericSlice[T] {
 func (o OfNumericSlice[T]) Unshift(elements ...T) OfNumericSlice[T] {
 	return OfNumericSlice[T]{Unshift(o.Result, elements...)}
 }
+
+func (o OfNumericSlice[T]) Delete(idx ...int) OfNumericSlice[T] {
+	return OfNumericSlice[T]{Delete(o.Result, idx...)}
+}
diff --git a/v2/of_ordered.go b/v2/of_ordered.go
index 3baa7c0..c1160dd 100644
--- a/v2/of_ordered.go
+++ b/v2/of_ordered.go
@@ -2,8 +2,9 @@ package pie
 
 import (
 	"context"
-	"golang.org/x/exp/constraints"
 	"math/rand"
+
+	"golang.org/x/exp/constraints"
 )
 
 // OfOrdered encapsulates a slice to be used in multiple chained operations.
@@ -199,3 +200,7 @@ func (o OfOrderedSlice[T]) Unique() OfOrderedSlice[T] {
 func (o OfOrderedSlice[T]) Unshift(elements ...T) OfOrderedSlice[T] {
 	return OfOrderedSlice[T]{Unshift(o.Result, elements...)}
 }
+
+func (o OfOrderedSlice[T]) Delete(idx ...int) OfOrderedSlice[T] {
+	return OfOrderedSlice[T]{Delete(o.Result, idx...)}
+}


Test patch:
diff --git a/v2/delete_test.go b/v2/delete_test.go
new file mode 100644
index 0000000..6c80bff
--- /dev/null
+++ b/v2/delete_test.go
@@ -0,0 +1,61 @@
+package pie_test
+
+import (
+	"github.com/elliotchance/pie/v2"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+var deleteTests = []struct {
+	ss       []int
+	idx      []int
+	expected []int
+}{
+	// idx out of bounds
+	{
+		[]int{1, 2},
+		[]int{-1},
+		[]int{1, 2},
+	},
+	{
+		[]int{1, 2},
+		[]int{2},
+		[]int{1, 2},
+	},
+	// remove from empty slice
+	{
+		[]int{},
+		[]int{0},
+		[]int{},
+	},
+	{
+		[]int{1},
+		[]int{0},
+		[]int{},
+	},
+	{
+		[]int{1, 2, 3, 4, 5},
+		[]int{2},
+		[]int{1, 2, 4, 5},
+	},
+	{
+		[]int{1, 2, 3, 4, 5},
+		[]int{1, 3},
+		[]int{1, 3, 5},
+	},
+	// mixed indices
+	{
+		[]int{1, 2, 3, 4, 5},
+		[]int{1, -1, 5, 3},
+		[]int{1, 3, 5},
+	},
+}
+
+func TestDelete(t *testing.T) {
+	for _, test := range deleteTests {
+
+		t.Run("", func(t *testing.T) {
+			assert.Equal(t, test.expected, pie.Delete(test.ss, test.idx...))
+		})
+	}
+}
diff --git a/v2/of_numeric_test.go b/v2/of_numeric_test.go
index f54a9f5..daf7648 100644
--- a/v2/of_numeric_test.go
+++ b/v2/of_numeric_test.go
@@ -23,4 +23,12 @@ func TestOfONumeric(t *testing.T) {
 
 		assert.Equal(t, []float64{1.23}, names)
 	})
+
+	t.Run("delete", func(t *testing.T) {
+		names := pie.OfNumeric([]float64{1.23, 4.56}).
+			Delete(1).
+			Result
+
+		assert.Equal(t, []float64{1.23}, names)
+	})
 }
diff --git a/v2/of_ordered_test.go b/v2/of_ordered_test.go
index 71d1b32..44fc676 100644
--- a/v2/of_ordered_test.go
+++ b/v2/of_ordered_test.go
@@ -23,4 +23,12 @@ func TestOfOrdered(t *testing.T) {
 
 		assert.Equal(t, []string{"Bob"}, names)
 	})
+
+	t.Run("delete", func(t *testing.T) {
+		names := pie.Of([]string{"Bob", "Sally", "John", "Jane"}).
+			Delete(2, 3).
+			Result
+
+		assert.Equal(t, []string{"Bob", "Sally"}, names)
+	})
 }
diff --git a/v2/of_test.go b/v2/of_test.go
index e542ac5..32d8e67 100644
--- a/v2/of_test.go
+++ b/v2/of_test.go
@@ -28,4 +28,12 @@ func TestOf(t *testing.T) {
 
 		assert.Equal(t, []string{"Bob", "Sally"}, names)
 	})
+
+	t.Run("delete", func(t *testing.T) {
+		names := pie.Of([]string{"Bob", "Sally", "John", "Jane"}).
+			Delete(2, 3).
+			Result
+
+		assert.Equal(t, []string{"Bob", "Sally"}, names)
+	})
 }