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

proposal: x/exp/maps: add Entries #54012

Closed
dsnet opened this issue Jul 23, 2022 · 14 comments
Closed

proposal: x/exp/maps: add Entries #54012

dsnet opened this issue Jul 23, 2022 · 14 comments

Comments

@dsnet
Copy link
Member

dsnet commented Jul 23, 2022

There are helper functions to extract the keys and the values, but not one to extract both together.

I propose adding:

type Entry[K comparable, V any] struct {
	K K
	V V
}

func Entries[M ~map[K]V, K comparable, V any](m M) []Entry[K, V] {
	r := make([]Entry[K, V], 0, len(m))
	for k, v := range m {
		r = append(r, Entry[K, V]{k, v})
	}
	return r
}

I often find myself sorting a map based on the value, but needing the key and value paired together.

This API would ease that pattern:

entries := maps.Entries(m)
slices.SortFunc(entries, func(x, y maps.Entry[K, V]) bool {
    return x.V < y.V
})
@rsc
Copy link
Contributor

rsc commented Mar 15, 2023

I'm skeptical about this. If we do this, maps.Entry[K, V] is going to end up all over everyone's code base as a generic pair type (in code that has nothing to do with maps).

@DeedleFake
Copy link

How about something that allows the user to use their own type via a function, such as

func EntriesFunc[K comparable, V any, M ~map[K]V, P any](m M, f func(K, V) P) (entries []P) {
  entries = make([]P, 0, len(m))
  for k, v := range m {
    entries = append(entries, f(k, v))
  }
  return entries
}

It seems a bit less useful to me, but it could still be useful in certain circumstances.

@ianlancetaylor
Copy link
Contributor

@DeedleFake I feel like we're staying away from that kind of map/filter/reduce functions until we get a handle on iterators.

@DeedleFake
Copy link

If the goal is to collect it into a slice, the question of whether it'll need a predefined Entry or Pair type or not is still going to need answering.

@dsnet
Copy link
Member Author

dsnet commented Mar 15, 2023

If we do this, maps.Entry[K, V] is going to end up all over everyone's code base as a generic pair type

That feels like unnecessary fear of what could happen. It would be more concerning if K was constrained to any rather than comparable. As it stand, it's not quite a fully generic pair type.

@rsc
Copy link
Contributor

rsc commented Mar 29, 2023

The comment says the API would ease this pattern:

entries := maps.Entries(m)
slices.SortFunc(entries, func(x, y maps.Entry[K, V]) bool {
    return x.V < y.V
})

but I don't see what happens next in this pattern. Is the code going to return a []maps.Entry? Why is that better than a more specific type? It's unclear why code would ever want to have a variable with type maps.Entry or []maps.Entry instead of something more specific.

@rsc
Copy link
Contributor

rsc commented Mar 29, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@ainar-g
Copy link
Contributor

ainar-g commented Mar 30, 2023

@rsc, the main use for this pattern in the code I work with is iteration of maps in a defined (here, sorted) order. So some kind of maps.Ordered(m, f(k K, v V) (cont bool)) API would probably cover a large part of the need for maps.Entries, at least in my experience.

@fzipp
Copy link
Contributor

fzipp commented Mar 30, 2023

@ainar-g

keys := maps.Keys(m)
slices.Sort(keys)
for _, k := range keys {
	v := m[k]
	// ...
}

@ainar-g
Copy link
Contributor

ainar-g commented Mar 30, 2023

@fzipp, thanks, but I've known about that idiom and had already used it in the past. The issue with that one is that it iterates over a map twice, whereas an entries version would only need one iteration, since the values would be grouped together with the keys.

@fzipp
Copy link
Contributor

fzipp commented Mar 30, 2023

@ainar-g In the maps.Entries example I count multiple iterations as well:

entries := maps.Entries(m)
slices.SortFunc(entries, func(x, y maps.Entry[K, V]) bool {
    return x.V < y.V
})
  1. to extract the entries
  2. to sort the entries
  3. and then (not shown) a third time to iterate over the sorted entries

Or are you now talking about this hypothetical maps.Ordered?

@ainar-g
Copy link
Contributor

ainar-g commented Mar 30, 2023

I meant map iterations in particular. Also, this is getting a bit off-topic, as my first comment, responding to rsc, was an example of a possible use of maps.Entry based on my experience as opposed to a full API proposal.

@rsc
Copy link
Contributor

rsc commented Apr 6, 2023

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 12, 2023

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Apr 12, 2023
@golang golang locked and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

7 participants