Commit ed8993e3 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #47944 from allencloud/fix-CopyStrings-and-ShuffleStrings-when-slice-is-nil

Automatic merge from submit-queue (batch tested with PRs 48264, 48324, 48125, 47944, 47489) fix CopyStrings and ShuffleStrings for slice when slice is nil Signed-off-by: 's avatarallencloud <allen.sun@daocloud.io> **What this PR does / why we need it**: This PR fixes two functions in util/slice.go, in which I think `CopyStrings` and `ShuffleStrings` miss one case. The case is input data is nil, in this case I think the data returned should be nil as well rather than a non-nil slice with 0 element. In addition, I added some test code for this. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # NONE, I did not raise a issue for this code. I ran into this when code learning. **Special notes for your reviewer**: NONE **Release note**: ```release-note NONE ```
parents faf4e57f f98bc7d4
...@@ -26,6 +26,9 @@ import ( ...@@ -26,6 +26,9 @@ import (
// CopyStrings copies the contents of the specified string slice // CopyStrings copies the contents of the specified string slice
// into a new slice. // into a new slice.
func CopyStrings(s []string) []string { func CopyStrings(s []string) []string {
if s == nil {
return nil
}
c := make([]string, len(s)) c := make([]string, len(s))
copy(c, s) copy(c, s)
return c return c
...@@ -41,6 +44,9 @@ func SortStrings(s []string) []string { ...@@ -41,6 +44,9 @@ func SortStrings(s []string) []string {
// ShuffleStrings copies strings from the specified slice into a copy in random // ShuffleStrings copies strings from the specified slice into a copy in random
// order. It returns a new slice. // order. It returns a new slice.
func ShuffleStrings(s []string) []string { func ShuffleStrings(s []string) []string {
if s == nil {
return nil
}
shuffled := make([]string, len(s)) shuffled := make([]string, len(s))
perm := utilrand.Perm(len(s)) perm := utilrand.Perm(len(s))
for i, j := range perm { for i, j := range perm {
......
...@@ -22,15 +22,29 @@ import ( ...@@ -22,15 +22,29 @@ import (
) )
func TestCopyStrings(t *testing.T) { func TestCopyStrings(t *testing.T) {
src := []string{"a", "c", "b"} var src1 []string
dest := CopyStrings(src) dest1 := CopyStrings(src1)
if !reflect.DeepEqual(src1, dest1) {
t.Errorf("%v and %v are not equal", src1, dest1)
}
src2 := []string{}
dest2 := CopyStrings(src2)
if !reflect.DeepEqual(src2, dest2) {
t.Errorf("%v and %v are not equal", src2, dest2)
}
src3 := []string{"a", "c", "b"}
dest3 := CopyStrings(src3)
if !reflect.DeepEqual(src, dest) { if !reflect.DeepEqual(src3, dest3) {
t.Errorf("%v and %v are not equal", src, dest) t.Errorf("%v and %v are not equal", src3, dest3)
} }
src[0] = "A" src3[0] = "A"
if reflect.DeepEqual(src, dest) { if reflect.DeepEqual(src3, dest3) {
t.Errorf("CopyStrings didn't make a copy") t.Errorf("CopyStrings didn't make a copy")
} }
} }
...@@ -50,9 +64,16 @@ func TestSortStrings(t *testing.T) { ...@@ -50,9 +64,16 @@ func TestSortStrings(t *testing.T) {
} }
func TestShuffleStrings(t *testing.T) { func TestShuffleStrings(t *testing.T) {
src := []string{"a", "b", "c", "d", "e", "f"} var src []string
dest := ShuffleStrings(src) dest := ShuffleStrings(src)
if dest != nil {
t.Errorf("ShuffleStrings for a nil slice got a non-nil slice")
}
src = []string{"a", "b", "c", "d", "e", "f"}
dest = ShuffleStrings(src)
if len(src) != len(dest) { if len(src) != len(dest) {
t.Errorf("Shuffled slice is wrong length, expected %v got %v", len(src), len(dest)) t.Errorf("Shuffled slice is wrong length, expected %v got %v", len(src), len(dest))
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment