Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow initial elements as variadic into NewOrderedMap #45

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

pd93
Copy link
Contributor

@pd93 pd93 commented Sep 13, 2024

Closes #43

Adds a variadic argument to NewOrderedMap which allows the user to specify a set of initial data when initialising an ordered map. In my initial proposal I suggested a new function called NewOrderedMapWithElements. However, this isn't necessary to maintain backwards compatibility since the argument is variadic and still works when no arguments are set.

I also considered a functional options style API which would allow for greater compatibility with any future changes:

type OrderedMapOption[K comparable, V any] func(*OrderedMap[K, V])

type OrderedMap[K comparable, V any] struct {
	kv map[K]*Element[K, V]
	ll list[K, V]
}

func NewOrderedMap[K comparable, V any](opts ...OrderedMapOption[K, V]) *OrderedMap[K, V] {
	om := &OrderedMap[K, V]{
		kv: make(map[K]*Element[K, V]),
	}
	for _, opt := range opts {
		opt(om)
	}
	return om
}

func WithInitialData[K comparable, V any](els ...*Element[K, V]) OrderedMapOption[K, V] {
	return func(m *OrderedMap[K, V]) {
		for _, el := range els {
			m.Set(el.Key, el.Value)
		}
	}
}

However, this may be overkill for what is currently quite a simple API. It also loses the ability to initialise the underlying map with the correct size.

We could consider setting the size of the underlying list too, but I assume you are not doing this currently to save allocations. Maybe we only initialise if len(els) > 0?


This change is Reviewable

v2/orderedmap.go Outdated Show resolved Hide resolved
@pd93 pd93 force-pushed the initialise-with-data branch from aed8ebe to e624256 Compare November 27, 2024 14:00
@elliotchance elliotchance merged commit ce850a7 into elliotchance:master Nov 27, 2024
2 checks passed
@elliotchance
Copy link
Owner

Thanks @pd93 !

@pd93 pd93 deleted the initialise-with-data branch November 28, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initialising an ordered map with data
2 participants