wk8 / go-ordered-map

Optimal implementation of ordered maps for Golang - ie maps that remember the order in which keys were inserted.
Apache License 2.0
523 stars 40 forks source link

Support optional capacity on creation #15

Closed fenollp closed 1 year ago

wk8 commented 1 year ago

Thanks for the PR @fenollp , might adding a test too?

SVilgelm commented 1 year ago

I think it would be great to have more general solution for the options. something like


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

SVilgelm commented 1 year ago

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)
  ...
}
wk8 commented 1 year ago

@SVilgelm : I agree in general, but in this specific case:

  1. it's nice to have an API that stays as close as possible to that of maps
  2. I can't really think off the top of my head of any other options that could make sense in the future

So I'm inclined to keep this as is? Thoughts?

fenollp commented 1 year ago

Rebased!

So I'm inclined to keep this as is? Thoughts?

Agreed!