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

and_modify method should be part of the HashMap's Documentation. #98122

Closed
SeniorMars opened this issue Jun 15, 2022 · 5 comments
Closed

and_modify method should be part of the HashMap's Documentation. #98122

SeniorMars opened this issue Jun 15, 2022 · 5 comments
Assignees
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@SeniorMars
Copy link
Contributor

SeniorMars commented Jun 15, 2022

I understand that and_modify deals with the Entry enum; however, I believe the method would be advantageous to developers when dealing with HashMaps.

A common pattern I see when using hashmaps is something like this:

strings.into_iter().for_each(|string| {
    let value = map.entry(string).or_insert(0);
    *value += 1;
});

However, with and_modify this turns into this:

strings.into_iter().for_each(|string| {
    map.entry(string)
        .and_modify(|value| *value += 1)
        .or_insert(1);
});

Which leans more into idiomatic rust. I just learned of and_modify today, and I believe more rust users would use it if they knew it existed. I think the problem is that and_modify is part of the Entry API.

@eggyal
Copy link
Contributor

eggyal commented Jun 15, 2022

To be clear, are you suggesting that the documentation for HashMap::entry should include an example using and_modify? That seems pretty reasonable.

@rustbot label +A-docs +E-easy

@rustbot rustbot added A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Jun 15, 2022
@SeniorMars
Copy link
Contributor Author

Yes, I am. Can I work on the PR for the issue? Thanks!

@SeniorMars
Copy link
Contributor Author

@rustbot claim

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 16, 2022
…ylan-DPC

Entry and_modify doc

This PR modifies the documentation for [HashMap](https://doc.rust-lang.org/std/collections/struct.HashMap.html#) and [BTreeMap](https://doc.rust-lang.org/std/collections/struct.BTreeMap.html#) by introducing examples for `and_modify`. `and_modify` is a function that tends to give more idiomatic rust code when dealing with these data structures -- yet it lacked examples and was hidden away. This PR adds that and addresses rust-lang#98122.

I've made some choices which I tried to explain in my commits. This is my first time contributing to rust, so hopefully, I made the right choices.
@SkiFire13
Copy link
Contributor

strings.into_iter().for_each(|string| {
    let value = map.entry(string).or_insert(0);
    *value += 1;
});

I would shorten this to:

strings.into_iter().for_each(|string| {
    *map.entry(string).or_insert(0) += 1;
});

IMO this is more idiomatic than the and_modify example, but this is probably subjective. It just seems more linear and thus simplier to me:

  • if an entry is empty the default value is 0 (this is in addition to what normally happens)
  • then increase it (this always happens)

In contrast with and_modify:

  • if there's an entry increase it (which is conditional on not being the first element)
  • else set it to 1 (which is condition to being the first element)

Anyway with #98125 I suppose this could be closed?

@SeniorMars
Copy link
Contributor Author

So honestly, I agree with what you are saying! However, if we look at the HashMap documentation right now we see this:

// update a key, guarding against the key possibly not being set
let stat = player_stats.entry("attack").or_insert(100);
*stat += random_stat_buff();

I believe that and_modfiy should be used for this type of example. However, for the example I gave which was not the best I admit, I agree that it would be simpler than to do it in one liner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

4 participants