diff --git a/slice/remove.go b/slice/remove.go index ad8e298..9611f8d 100644 --- a/slice/remove.go +++ b/slice/remove.go @@ -6,33 +6,18 @@ package slice // Remove modifies the contents of the given slice and performs no allocations // or copies. func Remove[T comparable](slice []T, i, j int) []T { - // Nothing to do - if len(slice) == 0 { + // Nothing to do for emtpy slices or starting indecies outside range + if len(slice) == 0 || i > len(slice) { return slice } // Prevent invalid slice indexing - if i < 0 || j > len(slice)-1 { + if i < 0 || j > len(slice) { return slice } + // Removing the last element is a simple re-slice + if i == len(slice)-1 && j >= len(slice) { + return slice[:i] + } // Note: this modifies the slice return append(slice[:i], slice[j:]...) } - -// RemoveAll removes all instances of the given value in the given slice, in place, preserving element order. -// RemoveAll modifies the contents of the given slice. -// If value is not present in the slice, or if the slice is nil or empty, the slice is returned unaltered. -// RemoveAll performs no allocations or copies. -// Note: DO NOT use this for long-lived slices as the elements removed are not garbage collected -func RemoveAll[T comparable](slice []T, value T) []T { - // Nothing to do - if len(slice) == 0 { - return slice - } - b := slice[:0] - for _, v := range slice { - if v != value { - b = append(b, v) - } - } - return b -} diff --git a/slice/remove_test.go b/slice/remove_test.go index c642db7..74b8d51 100644 --- a/slice/remove_test.go +++ b/slice/remove_test.go @@ -19,12 +19,6 @@ func ExampleRemove() { // [1 3] } -func ExampleRemoveAll() { - s := []int{1, 2, 3, 2, 4, 2} - fmt.Println(slice.RemoveAll(s, 2)) - // Output: [1 3 4] -} - func TestRemove(t *testing.T) { for _, test := range []struct { name string @@ -37,23 +31,35 @@ func TestRemove(t *testing.T) { inSlice: []int{}, outSlice: []int{}, }, { - name: "out of range should not panic or modify the slice", - i: 4, - j: 5, + name: "nil check", + i: 1, + j: 3, + inSlice: nil, + outSlice: nil, + }, { + name: "start index out of range should not panic or modify the slice", + i: -2, + j: 2, inSlice: []int{2}, outSlice: []int{2}, }, { - name: "correct start index but invalid end index", + name: "end index out of range should not panic or modify the slice", i: 0, - j: 2, + j: 5, + inSlice: []int{2}, + outSlice: []int{2}, + }, { + name: "invalid start and end index should not panic or modify the slice", + i: 4, + j: 7, inSlice: []int{2}, outSlice: []int{2}, }, { name: "single value remove", i: 0, j: 1, - inSlice: []int{2}, - outSlice: []int{}, + inSlice: []int{1, 2, 3}, + outSlice: []int{2, 3}, }, { name: "middle value removed", i: 1, @@ -61,78 +67,28 @@ func TestRemove(t *testing.T) { inSlice: []int{1, 2, 3}, outSlice: []int{1, 3}, }, { - name: "multi-value remove", - i: 1, + name: "last value removed", + i: 2, j: 3, inSlice: []int{1, 2, 3}, - outSlice: []int{1}, + outSlice: []int{1, 2}, }, { - name: "nil check", + name: "multi-value remove", i: 1, j: 3, - inSlice: nil, - outSlice: nil, - }, {}} { - t.Run(test.name, func(t *testing.T) { - got := slice.Remove(test.inSlice, test.i, test.j) - for i, expected := range test.outSlice { - assert.Equal(t, expected, got[i]) - } - }) - } -} - -func TestRemoveAll(t *testing.T) { - for _, test := range []struct { - name string - remove string - inSlice []string - outSlice []string - }{{ - name: "empty slice", - remove: "localhost", - inSlice: []string{}, - outSlice: []string{}, - }, { - name: "nil check", - remove: "localhost", - inSlice: nil, - outSlice: nil, - }, { - name: "only value should be removed", - remove: "localhost", - inSlice: []string{"localhost"}, - outSlice: []string{}, - }, { - name: "beginning of list", - remove: "localhost", - inSlice: []string{"localhost", "foo"}, - outSlice: []string{"foo"}, - }, { - name: "end of list", - remove: "localhost", - inSlice: []string{"foo", "localhost"}, - outSlice: []string{"foo"}, - }, { - name: "middle of the list", - remove: "localhost", - inSlice: []string{"foo", "localhost", "bar"}, - outSlice: []string{"foo", "bar"}, - }, { - name: "all instances should be removed", - remove: "localhost", - inSlice: []string{"localhost", "foo", "localhost"}, - outSlice: []string{"foo"}, + inSlice: []int{1, 2, 3}, + outSlice: []int{1}, }, { - name: "order and repeats shouldn't matter", - remove: "localhost", - inSlice: []string{"foo", "localhost", "bar", "localhost", "localhost", "baz.com"}, - outSlice: []string{"foo", "bar", "baz.com"}, + name: "end of slice gut check for invalid ending index", + i: 2, + j: 5, // This index being out of bounds shouldn't matter + inSlice: []int{1, 2, 3}, + outSlice: []int{1, 2}, }} { t.Run(test.name, func(t *testing.T) { - out := slice.RemoveAll(test.inSlice, test.remove) - for i, expected := range test.outSlice { - assert.Equal(t, expected, out[i]) + got := slice.Remove(test.inSlice, test.i, test.j) + for i := range test.outSlice { + assert.Equal(t, test.outSlice[i], got[i]) } }) } @@ -150,14 +106,3 @@ func BenchmarkRemove(b *testing.B) { } out = o } - -func BenchmarkRemoveAll(b *testing.B) { - o := []string{} - in := []string{"localhost", "mg.example.com", "testlabs.io", "localhost", "m.example.com.br", "localhost", "localhost"} - - b.ResetTimer() - for i := 0; i < b.N; i++ { - o = slice.RemoveAll(in, "localhost") - } - out = o -}