-
Notifications
You must be signed in to change notification settings - Fork 182
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
Added a procedure for getting all the keys in a hashmap #741
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @degawa for this PR. LGTM! I have only a few minor suggestions.
doc/specs/stdlib_hashmaps.md
Outdated
@@ -1148,6 +1154,9 @@ Procedures to modify the content of a map: | |||
|
|||
Procedures to report the content of a map: | |||
|
|||
* `map % get_all_keys( all_keys )` - Returns all the keys | |||
presented in the map; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be "present" or "presented"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use "presented in" to mean "located in," "existing in," or "contained in" here.
I cannot answer whether "present" or "presented" is correct due to my poor English skills, so I need help to create a natural expression, especially a phrase referring to "keys in a hash map."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@milancurcic Any idea how it should be written?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jvdp1
The following expression is used in the documentation of Map in Java:
keySet()
Returns a Set view of the keys contained in this map.
So, I would like to change "presented" to "contained" to follow this expression. I would appreciate your comments.
Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@degawa "contained" sounds good to me. Please, go ahead!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @jvdp1, for giving me a supportive push forward.
I fixed the documentation for the get_all_keys
and pushed it.
I would appreciate a review.
Co-authored-by: Jeremie Vandenplas <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for this PR @degawa !
To add a procedure for getting all the keys in a hashmap, the subroutine
get_all_keys
is added to the hashmap types.get_all_keys
is implemented as a subroutine rather than a function to maintain consistency with the other procedures, especiallyget_other_data
.The tasks done are summarized as follows:
hashmap_type
chaining_hashmap_type
andopen_hashmap_type
.closes #732