[#88] min aggregator bug fixed, tests added #164

Merged
fyrchik merged 2 commits from AndrewDanilin/frostfs-sdk-go:88-fix_min_aggregator_and_add_tests_for_them into master 2023-10-03 07:05:04 +00:00
2 changed files with 52 additions and 4 deletions
Showing only changes of commit d13062b811 - Show all commits

View file

@ -23,7 +23,7 @@ type (
}
minAgg struct {
min float64
min *float64

What about having a bool field like minFound bool? (with a meaning minFound == true if and only if a.min != nil in the current implementation.
Your solution is correct, however it allocates (didn't check, but it should). We execute this routine on every operation with an object, so no allocations would be nice here.

What about having a bool field like `minFound bool`? (with a meaning `minFound == true` if and only if `a.min != nil` in the current implementation. Your solution is correct, however it allocates (didn't check, but it should). We execute this routine on every operation with an object, so no allocations would be nice here.

Good advice, I'll change it.

Good advice, I'll change it.
}
meanIQRAgg struct {
@ -102,13 +102,17 @@ func (a *meanAgg) Compute() float64 {
}
func (a *minAgg) Add(n float64) {
if a.min == 0 || n < a.min {
a.min = n
if a.min == nil || n < *a.min {
a.min = &n
}
}
func (a *minAgg) Compute() float64 {
return a.min
if a.min == nil {
return 0
}
return *a.min
}
func (a *meanIQRAgg) Add(n float64) {

44
netmap/aggregator_test.go Normal file
View file

@ -0,0 +1,44 @@
package netmap
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestMinAgg(t *testing.T) {
tests := []struct {
vals []float64
res float64
}{
{
vals: []float64{1, 2, 3, 0, 10},
res: 0,
},
{
vals: []float64{10, 0, 10, 0},
res: 0,
},
{
vals: []float64{0, 1, 2, 3, 10},
res: 0,
},
{
vals: []float64{0, 0, 0, 0},
res: 0,
},
{
vals: []float64{10, 10, 10, 10},
res: 10,
},
}
for _, test := range tests {
a := newMinAgg()
for _, val := range test.vals {
a.Add(val)
}
require.Equal(t, test.res, a.Compute())
}
}