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

[Feature]: Consider using IndexMap<Value, Value> instead of Vec<(Value, Value)> in Value #81

Open
1 task done
nakedible-p opened this issue May 10, 2023 · 0 comments
Labels
enhancement New feature or request

Comments

@nakedible-p
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Description

The readme offers this rationale:

This decision was made because this type preserves the order of the pairs on the wire. Further, for those that need the properties of BTreeMap or HashMap, you can simply collect() the values into the respective type. This provides maximum flexibility.

I believe using IndexMap could be an even better compromise:

  • Order of pairs on the wire is preserved
  • IndexMap performance is quite good, on par with HashMap for many uses
  • Looking up a key from an object does not require collecting to Map first, or linear traversal through all entries

Currently, using Value for any traversal is really tedious, and really expensive if there are many map lookups, reducing it's usability significantly.

Acceptance Criteria

No response

Suggestions for a technical implementation

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: New
Development

No branches or pull requests

1 participant