From 9f4a11771a076e7230e271f37022f0a26e4eb9bb Mon Sep 17 00:00:00 2001 From: Matthew Edge Date: Wed, 18 Sep 2024 12:03:45 -0400 Subject: [PATCH 1/4] Proposal for generics-based Setter --- setter/setter.go | 15 ++++++ setter/setter_test.go | 109 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/setter/setter.go b/setter/setter.go index b643ad0..fee658f 100644 --- a/setter/setter.go +++ b/setter/setter.go @@ -53,6 +53,21 @@ func SetDefault(dest interface{}, defaultValue ...interface{}) { } } +func SetDefaultNew[T comparable](dest *T, defaultValues ...T) { + if IsZeroNew(*dest) { + for _, value := range defaultValues { + if !IsZeroNew(value) { + *dest = value + } + } + } +} + +func IsZeroNew[T comparable](value T) bool { + var zero T + return value == zero +} + // Assign the first value that is not empty or of zero value. // This panics if the value is not a pointer or if value and // default value are not of the same type. diff --git a/setter/setter_test.go b/setter/setter_test.go index c01aac8..770d2d3 100644 --- a/setter/setter_test.go +++ b/setter/setter_test.go @@ -145,3 +145,112 @@ func TestIsNil(t *testing.T) { assert.True(t, setter.IsNil(thing)) assert.False(t, setter.IsNil(&MyImplementation{})) } + +// --------------------------------------------------------- + +var newRes string + +func BenchmarkSetterNew(b *testing.B) { + var r string + for i := 0; i < b.N; i++ { + setter.SetDefaultNew(&r, "", "", "42") + } + newRes = r +} + +var oldRes string + +func BenchmarkSetter(b *testing.B) { + var r string + for i := 0; i < b.N; i++ { + setter.SetDefault(&r, "", "", "42") + } + oldRes = r +} + +func TestSetterNew_IfEmpty(t *testing.T) { + var conf struct { + Foo string + Bar int + } + assert.Equal(t, "", conf.Foo) + assert.Equal(t, 0, conf.Bar) + + // Should apply the default values + setter.SetDefaultNew(&conf.Foo, "default") + setter.SetDefaultNew(&conf.Bar, 200) + + assert.Equal(t, "default", conf.Foo) + assert.Equal(t, 200, conf.Bar) + + conf.Foo = "thrawn" + conf.Bar = 500 + + // Should NOT apply the default values + setter.SetDefaultNew(&conf.Foo, "default") + setter.SetDefaultNew(&conf.Bar, 200) + + assert.Equal(t, "thrawn", conf.Foo) + assert.Equal(t, 500, conf.Bar) +} + +func TestSetterNew_IfDefaultPrecedence(t *testing.T) { + var conf struct { + Foo string + Bar string + } + assert.Equal(t, "", conf.Foo) + assert.Equal(t, "", conf.Bar) + + // Should use the final default value + envValue := "" + setter.SetDefaultNew(&conf.Foo, envValue, "default") + assert.Equal(t, "default", conf.Foo) + + // Should use envValue + envValue = "bar" + setter.SetDefaultNew(&conf.Bar, envValue, "default") + assert.Equal(t, "bar", conf.Bar) +} + +func TestSetterNew_IsEmpty(t *testing.T) { + var count64 int64 + var thing string + + // Should return true + assert.Equal(t, true, setter.IsZeroNew(count64)) + assert.Equal(t, true, setter.IsZeroNew(thing)) + + thing = "thrawn" + count64 = int64(1) + assert.Equal(t, false, setter.IsZeroNew(count64)) + assert.Equal(t, false, setter.IsZeroNew(thing)) +} + +// Not possible now given compiler warnings +// func TestSetterNew_IfEmptyTypePanic(t *testing.T) { +// defer func() { +// if r := recover(); r != nil { +// assert.Equal(t, "reflect.Set: value of type int is not assignable to type string", r) +// } +// }() + +// var thing string +// // Should panic +// setter.SetDefaultNew(&thing, 1) +// assert.Fail(t, "Should have caught panic") +// } + +// Not possible now given argument is now a pointer to T +// func TestSetterNew_IfEmptyNonPtrPanic(t *testing.T) { +// defer func() { +// if r := recover(); r != nil { +// assert.Equal(t, "setter.SetDefault: Expected first argument to be of type reflect.Ptr", r) +// } +// }() + +// var thing string +// // Should panic +// setter.SetDefaultNew(thing, "thrawn") +// assert.Fail(t, "Should have caught panic") +// } From 55cb412e53ea647a74d20b9264c9b1f9c4ec9867 Mon Sep 17 00:00:00 2001 From: Matthew Edge Date: Wed, 18 Sep 2024 12:09:26 -0400 Subject: [PATCH 2/4] Fix docker compose CLI usage --- setter/setter.go | 1 + 1 file changed, 1 insertion(+) diff --git a/setter/setter.go b/setter/setter.go index fee658f..0e446a7 100644 --- a/setter/setter.go +++ b/setter/setter.go @@ -58,6 +58,7 @@ func SetDefaultNew[T comparable](dest *T, defaultValues ...T) { for _, value := range defaultValues { if !IsZeroNew(value) { *dest = value + return } } } From c99bf2608ef8b1fa4a5f5299b5c92ecb37ba2866 Mon Sep 17 00:00:00 2001 From: Matthew Edge Date: Thu, 19 Sep 2024 11:53:34 -0400 Subject: [PATCH 3/4] Add dedicated DefaultSlice function given comparable constraint. Add benchmark for slices --- setter/setter.go | 34 +++++++++++++++++++++++- setter/setter_test.go | 62 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 84 insertions(+), 12 deletions(-) diff --git a/setter/setter.go b/setter/setter.go index 0e446a7..30e062a 100644 --- a/setter/setter.go +++ b/setter/setter.go @@ -53,7 +53,23 @@ func SetDefault(dest interface{}, defaultValue ...interface{}) { } } -func SetDefaultNew[T comparable](dest *T, defaultValues ...T) { +// Default assigns the first non-zero default value to `dest` +// if `dest`, itself, is the zero value of type T. +// To work with slices: use DefaultSlice +// +// var config struct { +// Verbose *bool +// Foo string +// Bar int +// } +// holster.Default(&config.Foo, "default") +// holster.Default(&config.Bar, 200) +// +// Supply additional default values and SetDefault will +// choose the first default that is not of zero value +// +// holster.SetDefault(&config.Foo, os.Getenv("FOO"), "default") +func Default[T comparable](dest *T, defaultValues ...T) { if IsZeroNew(*dest) { for _, value := range defaultValues { if !IsZeroNew(value) { @@ -64,6 +80,22 @@ func SetDefaultNew[T comparable](dest *T, defaultValues ...T) { } } +// DefaultSlice assigns the first non-empty default value to `dest` if +// `dest`, itself, is an empty slice. +func DefaultSlice[T comparable](dest *[]T, defaultValues ...[]T) { + if len(*dest) == 0 { + for _, value := range defaultValues { + if len(value) != 0 { + *dest = make([]T, len(value)) + copy(*dest, value) + return + } + } + } +} + +// IsZeroNew compares the given value to its Golang-specified zero value. +// It works for any type T that satisfies comparable. func IsZeroNew[T comparable](value T) bool { var zero T return value == zero diff --git a/setter/setter_test.go b/setter/setter_test.go index 770d2d3..f80a632 100644 --- a/setter/setter_test.go +++ b/setter/setter_test.go @@ -20,6 +20,7 @@ import ( "github.com/mailgun/holster/v4/setter" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestIfEmpty(t *testing.T) { @@ -148,24 +149,46 @@ func TestIsNil(t *testing.T) { // --------------------------------------------------------- -var newRes string +var newStrRes string func BenchmarkSetterNew(b *testing.B) { var r string for i := 0; i < b.N; i++ { - setter.SetDefaultNew(&r, "", "", "42") + setter.Default(&r, "", "", "42") } - newRes = r + newStrRes = r } -var oldRes string +var oldStrRes string func BenchmarkSetter(b *testing.B) { var r string for i := 0; i < b.N; i++ { setter.SetDefault(&r, "", "", "42") } - oldRes = r + oldStrRes = r +} + +var newSliceRs []string + +func BenchmarkSetterNew_Slice(b *testing.B) { + r := make([]string, 0, 3) + b.ResetTimer() + for i := 0; i < b.N; i++ { + setter.DefaultSlice(&r, []string{}, []string{"welcome all", "to a benchmark", "of SILLY proportions"}) + } + newSliceRs = r +} + +var oldSliceRs []string + +func BenchmarkSetter_Slice(b *testing.B) { + r := make([]string, 0, 3) + b.ResetTimer() + for i := 0; i < b.N; i++ { + setter.SetDefault(&r, []string{""}, []string{"welcome all", "to a benchmark", "of SILLY proportions"}) + } + oldSliceRs = r } func TestSetterNew_IfEmpty(t *testing.T) { @@ -177,8 +200,8 @@ func TestSetterNew_IfEmpty(t *testing.T) { assert.Equal(t, 0, conf.Bar) // Should apply the default values - setter.SetDefaultNew(&conf.Foo, "default") - setter.SetDefaultNew(&conf.Bar, 200) + setter.Default(&conf.Foo, "default") + setter.Default(&conf.Bar, 200) assert.Equal(t, "default", conf.Foo) assert.Equal(t, 200, conf.Bar) @@ -187,13 +210,30 @@ func TestSetterNew_IfEmpty(t *testing.T) { conf.Bar = 500 // Should NOT apply the default values - setter.SetDefaultNew(&conf.Foo, "default") - setter.SetDefaultNew(&conf.Bar, 200) + setter.Default(&conf.Foo, "default") + setter.Default(&conf.Bar, 200) assert.Equal(t, "thrawn", conf.Foo) assert.Equal(t, 500, conf.Bar) } +func TestSetterNew_Slices(t *testing.T) { + var foo []string + require.Len(t, foo, 0) + + // Should apply the default values + setter.DefaultSlice(&foo, []string{"default"}) + require.Len(t, foo, 1) + assert.Equal(t, "default", foo[0]) + + foo = []string{"thrawn"} + + // Should NOT apply the default values + setter.DefaultSlice(&foo, []string{"default"}) + require.Len(t, foo, 1) + assert.Equal(t, "thrawn", foo[0]) +} + func TestSetterNew_IfDefaultPrecedence(t *testing.T) { var conf struct { Foo string @@ -204,12 +244,12 @@ func TestSetterNew_IfDefaultPrecedence(t *testing.T) { // Should use the final default value envValue := "" - setter.SetDefaultNew(&conf.Foo, envValue, "default") + setter.Default(&conf.Foo, envValue, "default") assert.Equal(t, "default", conf.Foo) // Should use envValue envValue = "bar" - setter.SetDefaultNew(&conf.Bar, envValue, "default") + setter.Default(&conf.Bar, envValue, "default") assert.Equal(t, "bar", conf.Bar) } From 1dc5951f60874b0363b3bc9afb6f308f25795dbb Mon Sep 17 00:00:00 2001 From: Matthew Edge Date: Mon, 23 Sep 2024 09:05:38 -0400 Subject: [PATCH 4/4] Fix commented test methods --- setter/setter_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setter/setter_test.go b/setter/setter_test.go index f80a632..85fe416 100644 --- a/setter/setter_test.go +++ b/setter/setter_test.go @@ -277,7 +277,7 @@ func TestSetterNew_IsEmpty(t *testing.T) { // var thing string // // Should panic -// setter.SetDefaultNew(&thing, 1) +// setter.Default(&thing, 1) // assert.Fail(t, "Should have caught panic") // } @@ -291,6 +291,6 @@ func TestSetterNew_IsEmpty(t *testing.T) { // var thing string // // Should panic -// setter.SetDefaultNew(thing, "thrawn") +// setter.Default(thing, "thrawn") // assert.Fail(t, "Should have caught panic") // }