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: maps: add function to check existence of a key #67377

Open
gudvinr opened this issue May 15, 2024 · 3 comments
Open

proposal: maps: add function to check existence of a key #67377

gudvinr opened this issue May 15, 2024 · 3 comments
Labels
Milestone

Comments

@gudvinr
Copy link

gudvinr commented May 15, 2024

Proposal Details

A lot of code uses pattern like this:

if _, ok := somemap[somekey]; ok {
    ...
}

just to check if key exists in a map.

And, with *Func functions from strings, slices, etc packages it makes code look unnecessary ugly.
For example, to filter out elements of a slice that exist in a map you need to do this:

type Buzz struct {
    ID string
}

var fizzies map[string]int
var buzzies []Buzz

...

buzzies = slices.DeleteFunc(buzzies, func (buzz Buzz) bool {
    _, ok := fizzies[buzz.ID]
    return ok
})

Instead of, let's say, this:

buzzies = slices.DeleteFunc(buzzies, func (buzz Buzz) bool {
    return maps.Contains(fizzies, buzz.ID)
})

That might be subjective, but one-line return statement looks less clunky.

I deliberately used this example because simple one, with var buzzies []string might raise unnecessary questions like "why not add something like HasKeyFunc(map[K]V) func(K) bool" which will make this example look like this:

buzzies = slices.DeleteFunc(buzzies, maps.HasKeyFunc(fizzies))

Which looks pretty, but doesn't make other cases any more readable than first example.

Bike-shedding section

I don't care about naming, but already implied maps.Contains because it's common across std.

One might argue that semantics of e.g. slices.Contains differ from maps.Contains because slices.Contains uses slice values instead of indices.
But there is an opposite argument of delete(map, key) that follows this pattern for maps while slices.Delete uses indices instead of values.

@gopherbot gopherbot added this to the Proposal milestone May 15, 2024
@apparentlymart
Copy link

apparentlymart commented May 15, 2024

I have also written plenty of examples like this. I've also fielded questions from folks new to Go about what it means when they encountered it for the first time in existing code, since it's not very easy to search for what this weird bunch of punctuation means.

I like the idea of giving this common operation a name, even if it's not saving that many lines of code. (I tend to care much more about code being easy to understand for unfamiliar readers than about number of lines of code, in any event.)

If I might join the bikeshedding discussion: since Contains could potentially mean either "has matching key" or "has matching value", I'd suggest maps.HasKey as a clearer alternative of similar length:

buzzies = slices.DeleteFunc(buzzies, func (buzz Buzz) bool {
    return maps.HasKey(fizzies, buzz.ID)
})

To me that makes the meaning pretty clear without having to consult the documentation, and isn't onerously long.

Although I would be considerably more skeptical about it, if we do decide to also have a function for testing whether any of the elements have a matching value, analogous to slices.Contains, it could be called maps.HasValue to continue the pattern.

@andig
Copy link
Contributor

andig commented May 20, 2024

More often than HasKey- which has an alternative 2-line syntax- I find myself looking for HasKeyFunc e.g. to do case insensitive checks for key existence. At the same time I'm aware that I need to chose between fixing all map keys before checking or paying the overhead with every single check.

buzzies = slices.DeleteFunc(buzzies, func (buzz Buzz) bool {
    return maps.HasKeyFunc(fizzies, func(key string) {
        return strings.EqualFold(buzz.ID, key)
    })
})

@gudvinr
Copy link
Author

gudvinr commented May 21, 2024

More often than HasKey- which has an alternative 2-line syntax- I find myself looking for HasKeyFunc e.g. to do case insensitive checks for key existence. At the same time I'm aware that I need to chose between fixing all map keys before checking or paying the overhead with every single check.

buzzies = slices.DeleteFunc(buzzies, func (buzz Buzz) bool {
    return maps.HasKeyFunc(fizzies, func(key string) {
        return strings.EqualFold(buzz.ID, key)
    })
})

If you need to do that often, you probably are doing something very wrong. To perform case-insensitive search (or any similar operation that needs to modify key before comparison) you need to lookup all map keys. That defeats one purpose of the map, which is quick key lookup.

If you need to do this kind of search, better way is to append records in e.g. upper case and then transform input value to upper case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

4 participants