Skip to content

Rename MultiDict back to original MultiMap or SetMultiMap #98

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

Closed
er1c opened this issue Jun 29, 2020 · 2 comments
Closed

Rename MultiDict back to original MultiMap or SetMultiMap #98

er1c opened this issue Jun 29, 2020 · 2 comments

Comments

@er1c
Copy link

er1c commented Jun 29, 2020

I think it may make the most sense to rename MultiDict to SetMultiMap, and potentially make MultiMap into a generic trait.

  1. It helps remove the @Deprecation warning if renamed back to MultiMap
  2. "MultiMap" is a more universally used terminology for this type of collection. (e.g. Guava has ArrayListMultimap, ListMultimap, SetMultimap, and etc; C++ STL is std::multimap; Rust is MultiMap, Apache Commons is MultiMap, etc)
  3. The existing name also does support or convey the collection type of the key's values. (e.g. it currently implements Set, but there is no way to extend it to use an Array, Vector or List instead)
  4. The code still uses MultiMap in various locations (like in the hashcode!), and the CC[_] is MapFactory (e.g. not DictFactory)
  5. There are no other Dictionary terms in any of the Scala collections. It's Map for any scala code, only the java converters have the mention of a dictionary. MultiDict isn't scala idiomatic.
@SethTisue
Copy link
Member

duplicate of #17? cc @joshlemer

@er1c
Copy link
Author

er1c commented Jun 30, 2020

@SethTisue thanks, I missed that other issue, closing and moving comment to the other issue.

@er1c er1c closed this as completed Jun 30, 2020
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

No branches or pull requests

2 participants