Skip to content

Commit 05ecaea

Browse files
committed
slices: improve SliceP documentation and test structure
Enhance SliceP documentation to clarify behaviour and constraints: - Document valid range for p parameter (0.0-1.0). - Clarify in-place sorting behaviour with warning. - Document return value for invalid inputs. - Improve example code with better variable naming. Refactor SliceP tests to comply with TESTING.md requirements: - Extract test case arrays into factory functions. - Reduce TestSliceP from 56 lines to 5 lines. - Add slicePIntTestCases(), slicePFloatTestCases(), slicePStringTestCases() returning []TestCase. Addresses coderabbitai review comments about documentation clarity and function length compliance. Signed-off-by: Alejandro Mery <amery@apptly.co>
1 parent 38a1a6c commit 05ecaea

File tree

3 files changed

+74
-38
lines changed

3 files changed

+74
-38
lines changed

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,9 @@ Key distinctions from `IsZero`:
184184
#### Utilities
185185

186186
* `SliceRandom[T](slice)` - select random element, returns (value, found).
187-
* `SliceP[T](slice,p)` - return the p-th percentile of the slice
187+
* `SliceP[T](slice,p)` - return the p-th percentile of the slice (p must be
188+
0.0-1.0). Sorts the slice in-place. Returns the zero value for empty slices
189+
or invalid p.
188190

189191
### List Operations (container/list)
190192

slices.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,15 +277,18 @@ func doSliceSort[T any](x []T, less func(a, b T) bool) {
277277
}
278278

279279
// SliceP returns the p-th percentile element from a slice.
280-
// The slice will be sorted in place before calculating the percentile.
280+
// The slice is sorted in place (modifying the original slice) before
281+
// calculating the percentile. p must be between 0.0 (minimum) and 1.0
282+
// (maximum). Returns the zero value for empty slices or when p is outside
283+
// the valid range.
281284
// This is commonly used for performance metrics and latency measurements
282285
// to identify outliers while excluding the worst values.
283-
// If the slice is empty, returns the zero value of type T.
284286
//
285287
// Example:
286288
//
287289
// responseTimes := []int{100, 150, 120, 450, 89, 2000, 110, 130, 140, 160}
288-
// p := SliceP(responseTimes,0.99) // Returns value at 99th percentile
290+
// p99 := SliceP(responseTimes, 0.99) // Returns the value at the 99th percentile
291+
// // Note: responseTimes is now sorted
289292
func SliceP[T Ordered](a []T, p float64) T {
290293
result := doP(a, p, func(d, e T) bool {
291294
return d < e

slices_test.go

Lines changed: 65 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,16 @@ func (tc slicePIntTestCase) Test(t *testing.T) {
631631
AssertEqual(t, tc.expected, result, "SliceP(%v, %.2f)", tc.input, tc.p)
632632
}
633633

634+
// Factory function for slicePIntTestCase
635+
func newSlicePIntTestCase(name string, input []int, p float64, expected int) slicePIntTestCase {
636+
return slicePIntTestCase{
637+
name: name,
638+
input: input,
639+
p: p,
640+
expected: expected,
641+
}
642+
}
643+
634644
// Test cases for SliceP function with float64
635645
type slicePFloatTestCase struct {
636646
name string
@@ -653,6 +663,16 @@ func (tc slicePFloatTestCase) Test(t *testing.T) {
653663
AssertEqual(t, tc.expected, result, "SliceP(%v, %.2f)", tc.input, tc.p)
654664
}
655665

666+
// Factory function for slicePFloatTestCase
667+
func newSlicePFloatTestCase(name string, input []float64, p, expected float64) slicePFloatTestCase {
668+
return slicePFloatTestCase{
669+
name: name,
670+
input: input,
671+
p: p,
672+
expected: expected,
673+
}
674+
}
675+
656676
// Test cases for SliceP function with string
657677
type slicePStringTestCase struct {
658678
name string
@@ -675,59 +695,70 @@ func (tc slicePStringTestCase) Test(t *testing.T) {
675695
AssertEqual(t, tc.expected, result, "SliceP(%v, %.2f)", tc.input, tc.p)
676696
}
677697

678-
func TestSliceP(t *testing.T) {
679-
// Test with integers
680-
intTestCases := []slicePIntTestCase{
698+
// Factory function for slicePStringTestCase
699+
func newSlicePStringTestCase(name string, input []string, p float64, expected string) slicePStringTestCase {
700+
return slicePStringTestCase{
701+
name: name,
702+
input: input,
703+
p: p,
704+
expected: expected,
705+
}
706+
}
707+
708+
func slicePIntTestCases() []TestCase {
709+
return []TestCase{
681710
// Empty slice
682-
{name: "empty slice", input: S[int](), p: 0.5, expected: 0},
711+
newSlicePIntTestCase("empty slice", S[int](), 0.5, 0),
683712

684713
// Single element
685-
{name: "single element p=0.0", input: S(42), p: 0.0, expected: 42},
686-
{name: "single element p=0.5", input: S(42), p: 0.5, expected: 42},
687-
{name: "single element p=1.0", input: S(42), p: 1.0, expected: 42},
714+
newSlicePIntTestCase("single element p=0.0", S(42), 0.0, 42),
715+
newSlicePIntTestCase("single element p=0.5", S(42), 0.5, 42),
716+
newSlicePIntTestCase("single element p=1.0", S(42), 1.0, 42),
688717

689718
// Common percentiles
690-
{name: "P50 (median)", input: S(5, 2, 8, 1, 9), p: 0.50, expected: 5},
691-
{name: "P95", input: S(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), p: 0.95, expected: 10},
692-
{name: "P99", input: S(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), p: 0.99, expected: 10},
719+
newSlicePIntTestCase("P50 (median)", S(5, 2, 8, 1, 9), 0.50, 5),
720+
newSlicePIntTestCase("P95", S(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), 0.95, 10),
721+
newSlicePIntTestCase("P99", S(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), 0.99, 10),
693722

694723
// Min and max
695-
{name: "P0 (min)", input: S(9, 3, 7, 1, 5), p: 0.0, expected: 1},
696-
{name: "P100 (max)", input: S(9, 3, 7, 1, 5), p: 1.0, expected: 9},
724+
newSlicePIntTestCase("P0 (min)", S(9, 3, 7, 1, 5), 0.0, 1),
725+
newSlicePIntTestCase("P100 (max)", S(9, 3, 7, 1, 5), 1.0, 9),
697726

698727
// Unsorted data
699-
{name: "unsorted P25", input: S(10, 5, 8, 3, 1, 9, 4, 7, 2, 6), p: 0.25, expected: 3},
700-
{name: "unsorted P75", input: S(10, 5, 8, 3, 1, 9, 4, 7, 2, 6), p: 0.75, expected: 8},
728+
newSlicePIntTestCase("unsorted P25", S(10, 5, 8, 3, 1, 9, 4, 7, 2, 6), 0.25, 3),
729+
newSlicePIntTestCase("unsorted P75", S(10, 5, 8, 3, 1, 9, 4, 7, 2, 6), 0.75, 8),
701730

702731
// Negative numbers
703-
{name: "negative P50", input: S(-5, -2, -8, -1, -9), p: 0.5, expected: -5},
704-
{name: "mixed signs P50", input: S(-2, 5, -1, 3, 0), p: 0.5, expected: 0},
732+
newSlicePIntTestCase("negative P50", S(-5, -2, -8, -1, -9), 0.5, -5),
733+
newSlicePIntTestCase("mixed signs P50", S(-2, 5, -1, 3, 0), 0.5, 0),
705734

706735
// Duplicates
707-
{name: "duplicates P50", input: S(3, 1, 3, 1, 3, 1, 3), p: 0.5, expected: 3},
736+
newSlicePIntTestCase("duplicates P50", S(3, 1, 3, 1, 3, 1, 3), 0.5, 3),
708737

709738
// Invalid percentiles
710-
{name: "invalid p=-0.1", input: S(1, 2, 3, 4, 5), p: -0.1, expected: 0},
711-
{name: "invalid p=1.5", input: S(1, 2, 3, 4, 5), p: 1.5, expected: 0},
739+
newSlicePIntTestCase("invalid p=-0.1", S(1, 2, 3, 4, 5), -0.1, 0),
740+
newSlicePIntTestCase("invalid p=1.5", S(1, 2, 3, 4, 5), 1.5, 0),
712741
}
742+
}
713743

714-
RunTestCases(t, intTestCases)
715-
716-
// Test with float64
717-
floatTestCases := []slicePFloatTestCase{
718-
{name: "float P50", input: S(3.14, 1.41, 2.71, 0.5, 4.0), p: 0.5, expected: 2.71},
719-
{name: "float P90", input: S(1.1, 2.2, 3.3, 4.4, 5.5, 6.6, 7.7, 8.8, 9.9, 10.0), p: 0.9, expected: 9.9},
720-
{name: "float empty", input: S[float64](), p: 0.5, expected: 0.0},
744+
func slicePFloatTestCases() []TestCase {
745+
return []TestCase{
746+
newSlicePFloatTestCase("float P50", S(3.14, 1.41, 2.71, 0.5, 4.0), 0.5, 2.71),
747+
newSlicePFloatTestCase("float P90", S(1.1, 2.2, 3.3, 4.4, 5.5, 6.6, 7.7, 8.8, 9.9, 10.0), 0.9, 9.9),
748+
newSlicePFloatTestCase("float empty", S[float64](), 0.5, 0.0),
721749
}
750+
}
722751

723-
RunTestCases(t, floatTestCases)
724-
725-
// Test with strings
726-
stringTestCases := []slicePStringTestCase{
727-
{name: "string P50", input: S("cherry", "apple", "banana", "date"), p: 0.5, expected: "banana"},
728-
{name: "string P0", input: S("zebra", "alpha", "beta"), p: 0.0, expected: "alpha"},
729-
{name: "string P100", input: S("zebra", "alpha", "beta"), p: 1.0, expected: "zebra"},
752+
func slicePStringTestCases() []TestCase {
753+
return []TestCase{
754+
newSlicePStringTestCase("string P50", S("cherry", "apple", "banana", "date"), 0.5, "banana"),
755+
newSlicePStringTestCase("string P0", S("zebra", "alpha", "beta"), 0.0, "alpha"),
756+
newSlicePStringTestCase("string P100", S("zebra", "alpha", "beta"), 1.0, "zebra"),
730757
}
758+
}
731759

732-
RunTestCases(t, stringTestCases)
760+
func TestSliceP(t *testing.T) {
761+
RunTestCases(t, slicePIntTestCases())
762+
RunTestCases(t, slicePFloatTestCases())
763+
RunTestCases(t, slicePStringTestCases())
733764
}

0 commit comments

Comments
 (0)