An implementation of Map. That allows for multiple inserts of a ke y …

[rwrozelle]

…retaining all inserted values and always returning the last inserted value through standard Map API. Additional method, multiple, allows access to all inserted values for a given key. Will be used in http and http_server packages for non-breaking extension of functionality for form parameters and header parameters that have multiple values for the same key.

Referenced Issues:
dart-lang/http#47
dart-lang/http#24
dart-archive/http_server#35
dart-lang/sdk#28142

[kevmoo]

@lrhn @nex3 thoughts?

[matanlurey]

Can we make sure this implementation is at least as good and complete as the one in Quiver so we can deprecated the latter?

…

On Wed, Jan 4, 2017, 12:50 PM Kevin Moore ***@***.***> wrote: @lrhn <https://github.com/lrhn> @nex3 <https://github.com/nex3> thoughts? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#47 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKQ7hSKrczSNfnoY8q5K0Zqi7iG4wT_ks5rO9u6gaJpZM4LatxT> .

[lrhn]

It doesn't sound like a safe approach.
It is basically introducing a subclass of Map that looks like a map, but which some methods can recognize and use the extended API of - without changing the type of the existing methods.
Which means that it's a Map that isn't really a Map, we just disguise it as one to get through the type-check, and then internally, it's treated as something different. Then it shouldn't be a Map at all.

It would mean that there is nothing in the type to suggest whether a class actually accepts and understands the "multi-map" - you can implement an interface that says that a method does, and just treat the argument as a normal map, and everybody seems happy.

I'd much rather prefer to actually have a real MultiMap type, and add methods that return or accept multimaps, or have methods that accept or create maps with iterables as keys (or either values or iterables).

[nex3]

I like this design. It's a good fit for APIs that we have a strong need for in http and shelf.

@lrhn I don't think it's right to say that this isn't really a Map. It implements Map, and all methods that are part of the Map interface follow the normal Map semantics and operating on a single value per key.

I think there's a lot of benefit for APIs that declare Map as a type to handle MultiMaps gracefully, but the real benefit comes from the other direction: APIs exposing them to the user. Consider the case of HTTP request headers. Headers can have multiple values, but it's rare. Most of the time users want to interact with them as though they're a normal single-valued map. But we clearly also need to expose multiple values to users who need them.

This MultiMap design is perfect for that case. It follows the normal Map API for the common case, while also providing specialized multi-value APIs for users who need them. Exposing a separate parallel multiHeaders getter would provide a much worse user experience.

[rwrozelle]

I've updated because I wasn't happy with my original code on the '[]=' operator. Decided it could introduce a breaking change if used in place of a Map as @nex3 intends. Are we at an impasse on this Pull Request? I'm obviously in alignment with @nex3, if it looks like a Map and acts like a Map, then its a Map regardless of the internals. To me this is the foundation of encapsulated programming. Multimap class is a specialized map that has additional capability. A separate argument could focus on the internals of the Multimap class and whether it is efficiently implemented.

[nex3]

@rwrozelle I'll talk to @lrhn either here or over email and we'll figure out a way to do this that we're all comfortable with.

[matanlurey]

I think we should consider closing this and revisit if it's not going in as-is.

[nex3]

I'm still discussing with Lasse offline (although that got interrupted by my leave).

[rwrozelle]

With this stalled, I moved my server side code over to Spring/Java, so I know longer have a pressing need to get this resolved.

[natebosch]

Ping?
Does anyone still need this? Anyone prepared to take on the cleanup from quiver if this goes in?

[matanlurey]

Closing. We can re-open if/when necessary.

[nex3]

This is still something I'm interested in getting in place for representing HTTP cookies, but I haven't been good about following up on the discussion about it.