Support optional capacity on creation #15
Conversation
|
Thanks for the PR @fenollp , might adding a test too? |
|
I think it would be great to have more general solution for the options. type Option struct {
capacity int
}
func mergeOptions(opts ...Option) Option {
var res Option
for _, opt := range opts {
if opt.capacity > 0 {
res.capacity = opt.capacity
}
}
return res
}
func WithCapacity(v int) Option {
return Option{capacity: v}
}
func New[K comparable, V any](opts ...Option) *OrderedMap[K, V] {
opt := mergeOptions(opts...)
pairs = make(map[K]*Pair[K, V], opt.capacity)
...
}this is more flexible solution, the option structure can be easily extended |
|
or using the functions based approach: type Option func(opt *option)
type option struct {
capacity int
}
func mergeOptions(opts ...Option) option {
res := &option{}
for _, opt := range opts {
opt(res)
}
return res
}
func WithCapacity(v int) Option {
return func(opt *option) {
opt.capacity = v
}
}
func New[K comparable, V any](opts ...Option) *OrderedMap[K, V] {
opt := mergeOptions(opts...)
pairs = make(map[K]*Pair[K, V], opt.capacity)
...
} |
|
@SVilgelm : I agree in general, but in this specific case:
So I'm inclined to keep this as is? Thoughts? |
| _ = New[int, string](1, 2, 3) | ||
| }) | ||
|
|
||
| om := New[int, string](-1) |
There was a problem hiding this comment.
I'm actually surprised that this doesn't panic...? That goes straight to make(map[K]*Pair[K, V], capacity[0]), and make does panic when given a < 0 capacity?
There was a problem hiding this comment.
No panic, just min(capacity, 0)
See https://go.dev/play/p/sFwJi7omFA9
There was a problem hiding this comment.
Interesting. https://go.dev/play/p/TiqBeNAUqe- does panic. I'll have to look more into that one :)
| om := New[int, string](-1) | ||
| om.Set(1337, "quarante-deux") | ||
| assert.Equal(t, 1, om.Len()) | ||
| } |
There was a problem hiding this comment.
Would be nice to add a test with a > 0 capacity that also checks that om.pairs has the expected capacity.
There was a problem hiding this comment.
I don't think there's a way to get a map's capacity though: golang/go#52157
Signed-off-by: Pierre Fenoll <[email protected]>
|
Rebased!
Agreed! |
No description provided.