Fix Outputs in Hashmap Examples #60014
Conversation
|
Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at: https://dart-review.googlesource.com/c/sdk/+/406781 Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly. Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR). |
|
https://dart-review.googlesource.com/c/sdk/+/406781 has been updated with the latest commits from this pull request. |
mraleph
left a comment
mraleph
left a comment
There was a problem hiding this comment.
Thank you for the contribution.
It is true that the last example contains a bug (5 instead of 6). However all other change are unnecessary because HashMap is unordered. See my comments.
sdk/lib/collection/hash_map.dart
Outdated
| /// final gasGiants = {6: 'Jupiter', 5: 'Saturn'}; | ||
| /// planets.addEntries(gasGiants.entries); | ||
| /// print(planets); // fx {5: Saturn, 6: Jupiter, 3: Earth, 4: Mars} | ||
| /// print(planets); // fx {3: Earth, 4: Mars, 6: Jupiter, 5: Saturn} |
There was a problem hiding this comment.
That's not a bug. HashMap is unordered which means any output is possible here. That's why it says fx.
Consider instead
/// print(planets); // fx {5: Saturn, 6: Jupiter, 3: Earth, 4: Mars}, no particular order of keys is guaranteed
or something similar
cc @lrhn
| /// // 5 Saturn | ||
| /// // 4 Mars | ||
| /// // 3 Earth | ||
| /// // 4 Mars |
There was a problem hiding this comment.
Unnecessary change. You can undo this as explained above order is not guaranteed with HashMap.
sdk/lib/collection/hash_map.dart
Outdated
| /// ``` | ||
| /// To remove an entry with a specific key, use [remove]. | ||
| /// ```dart continued | ||
| /// final removeValue = planets.remove(5); |
There was a problem hiding this comment.
A smaller change would be to just replace 5 by 6 here and not change subsequent lines.
|
https://dart-review.googlesource.com/c/sdk/+/406781 has been updated with the latest commits from this pull request. |
3 similar comments
|
https://dart-review.googlesource.com/c/sdk/+/406781 has been updated with the latest commits from this pull request. |
|
https://dart-review.googlesource.com/c/sdk/+/406781 has been updated with the latest commits from this pull request. |
|
https://dart-review.googlesource.com/c/sdk/+/406781 has been updated with the latest commits from this pull request. |
mraleph
left a comment
mraleph
left a comment
There was a problem hiding this comment.
LGTM. Thank you.
I have submitted corresponding Gerrit CL to CQ. Note: when CQ submits CL this PR will close without merging. That does not mean we rejected PR - that's just how things work, GitHub is just a mirror of an upstream repository.
2 participants
Fixes flutter/website#11632
Contribution guidelines:
dart format.Note that this repository uses Gerrit for code reviews. Your pull request will be automatically converted into a Gerrit CL and a link to the CL written into this PR. The review will happen on Gerrit but you can also push additional commits to this PR to update the code review.