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

Unclear guarantees about ListMultimap iteration order #487

Open
iinozemtsev opened this issue Dec 5, 2018 · 2 comments
Open

Unclear guarantees about ListMultimap iteration order #487

iinozemtsev opened this issue Dec 5, 2018 · 2 comments

Comments

@iinozemtsev
Copy link

Looking at https://pub.dartlang.org/documentation/quiver/latest/quiver.collection/ListMultimap-class.html, I have no idea about an iteration order.

Looking at the implementation, it seems that it is probably based on an insertion order, because private base class uses an empty map literal, and map literals should create LinkedHashMaps according to https://api.dartlang.org/stable/2.1.0/dart-core/Map/Map.html, but it is unclear whether this is a part of a contract, or just an implementation detail.

@yjbanov
Copy link
Member

yjbanov commented Dec 10, 2018

Judging by the use of unorderedEquals our own tests imply that we do not want to guarantee an iteration order. However, given that the implementation guarantees it and changing it will be a breaking change whether we like it or not, perhaps it makes sense to state that guarantee?

@cbracken what do you think?

@cbracken
Copy link
Member

cbracken commented Jan 11, 2019

I suspect the reason I didn't want to make that guarantee initially is that classes can be used as interfaces in Dart, and implementors of ListMultiMap shouldn't necessary be locked into our implementation choices.

In theory, the lack of a guarantee gives us flexibility in terms of future improvements; in practice, I very much doubt we'll ever make use of that flexibility.

Perhaps the best solution here is to do what we do for Map and point to iteration orders for ListMultimap and SetMultimap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants