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

Add clear and is_empty methods to Map #803

Merged
merged 5 commits into from Sep 26, 2022

Conversation

manu0466
Copy link
Contributor

This add two new utility methods to Map to make it more similar with the Rust BTreeMap

  1. clear: Removes all the elements inside Map
  2. is_empty: Returns true if the Map don't contains any element

To implement the two methods I rely on the iterator feature to get the keys of the elements inside the Map since I haven't find a better way to know the elements currently present inside the Map.

@CLAassistant
Copy link

CLAassistant commented Sep 19, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thank you for that contribution!
Two comments:

  • is there a reason why you created separate impl for that? I think it fits with same impl as save, load etc. so in your PR at line 112
  • would you mind implementing this in IndexedMap type to keep feature parity?

packages/storage-plus/src/map.rs Outdated Show resolved Hide resolved
@manu0466
Copy link
Contributor Author

@ueco-jb Thanks for the feedbacks!
I have applied your suggestions and added the implementation of clear and is_empty to IndexedMap.

Copy link
Member

@ueco-jb ueco-jb left a 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 your contribution.

@ueco-jb ueco-jb merged commit 6b7c704 into CosmWasm:main Sep 26, 2022
@manu0466 manu0466 deleted the manuel/map-clear branch October 8, 2022 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants