On Wed, Nov 21, 2012 at 3:12 AM, Alex Russell <a...@dojotoolkit.org> wrote:
> On Nov 20, 2012, at 8:08 PM, Tab Atkins Jr. <jackalm...@gmail.com> wrote:
>> Nope, that's not good enough. For example, you have to do input
>> cleanup (replacing lone surrogates with U+FFFD, escaping &, etc.)
>> which affects whether two keys are "the same". This affects set()'s
>> replacing behavior, and add()'s coalescing behavior. Heck, even
>> without extra cleanup, just the fact that it requires string keys
>> means you need to eagerly preserve the invariant - foo.set([1,2],
>> 'bar') and foo.set('1,2', 'bar') should both set the same key. So,
>> the stringifying must be eager.
>
> Well *that's* clearly untrue. It's possible to have any number of other
> behaviors than "string keys" and put your cleanups in other places (getAll(),
> etc.). I get that you might not want to do that, but it's a choice, and one
> we don't have to accept at face value.
Dude, my example showed why you have to do that, unless you track
additional metadata and eat an O(n) cost on get() (and adding that
additional metadata would *also* require you to override the set()
method, so you're not gaining anything).
What's the output of this code?
query.set("1,2", "foo");
query.set([1,2], "bar");
query.get([1]+",2");
It had better be "bar", because that's how string maps work. To
ensure this, though, you'll need to keep track of the order things are
set in. (Do Maps automatically iterate in key-creation order? That
doesn't help - you can add a third "query.set('1,2', 'baz')" line in
the example, and you should then get "baz" out.) Then, you have to do
a walk through the entire map, string-converting every key, to figure
out which version of the key was set last, so you can return it.
(On further thought, it's worse than that. Because you can associate
multiple values to each key, you have to keep track of the creation
time of *each value*, so you can then sort them by creation time after
coalescing when doing a getAll() call.)
This is not a realistic scenario. No one would ever do this, or
countenance browsers doing this under the hood. It's terrible, and
the only way around it is to preprocess eagerly to ensure that your
keys+values actually match the type contract that you're setting out.
>> However, if you do a normal subclass, then people can do silly things
>> like "Map.prototype.set(query, key, val);", which would skip
>> URLQuery's own set() method, avoiding the invariant preservation and
>> the URL manipulation.
>
> So what? Unless there's a reason to disallow that, it seems fine by me. If
> people have a handle to the query object, one assumes they can already do all
> manner of silly things.
The reason to disallow it is that *it doesn't make sense*. Allowing
people to *think* they're setting a key in the URLQuery, but having it
not actually work or add anything into the query (or worse, having it
"work" but in completely unpredictable ways, as keys are munged
together in unpredictable orders) is terrible API design. There's no
benefit to it whatsoever.
For the purposes of this API, Map.prototype.set is a completely
unrelated method that happens to share a name with something on
URLQuery.prototype.
>> We could avoid these problems by only "subclassing" in the sense that
>> URLQuery.prototype = new Map(); (or Object.create(Map.prototype)),
>> but not invoking the Map constructor and actually storing data in a
>> closure-hidden internal Map, so instanceof would work but using Map's
>> own methods wouldn't.
>>
>> Or, we could ignore the whole thing, and just make sure that we
>> ducktype as a Map.
>
> That's some pretty weak sauce. Lining up the types and designing APIs in the
> native way is the charge, and we have to do our best by it, lest we just
> continue to add magic to the platform.
Then help me figure out how to fix it. So far you've just insisted
that we can do the simple thing, despite it having terrible
side-effects.
(I'll say, though - ducktyping is a pretty native way to do things, too.)
~TJ
_______________________________________________
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss
