Re: [Bug 11398] New: [IndexedDB] Methods that take multiple optional parameters should instead take an options object

On Tue, Dec 14, 2010 at 3:44 AM, Jeremy Orlow <jorlow@chromium.org> wrote:

> Oops.  + list again.
> On Mon, Dec 13, 2010 at 6:35 PM, Jeremy Orlow <jorlow@chromium.org> wrote:
>
>> On Sat, Dec 11, 2010 at 1:20 AM, Jonas Sicking <jonas@sicking.cc> wrote:
>>
>>> On Fri, Dec 10, 2010 at 7:27 AM, Jeremy Orlow <jorlow@chromium.org>
>>> wrote:
>>> >
>>> > In addition to createObjectStore, I also intend to convert the
>>> following
>>> > over:
>>> >
>>> > IDBObjectStore.createIndex
>>>
>>> Sounds good.
>>>
>>> > IDBObjectStore.openCursor
>>> > IDBIndex.openCursor
>>> > IDBIndex.openKeyCursor
>>>
>>> I'm not convinced about these. It seems that this on average creates
>>> more syntax for authors rather than less. I see the main advantage
>>> with the options objects when you have so many arguments that it's
>>> hard to know which is which. Or when so many are optional that you end
>>> up having to specify the default value for a bunch just to specify the
>>> one you want to. In both cases these are of course fairly fuzzy
>>> limits. But it doesn't seem like either really applies here.
>>>
>>> I chatted with Brendan Eich about this and he made a good point. Do we
>>> know if all combinations are really going to be common? I.e. are all
>>> of the following going to be common:
>>>
>>> openCursor();
>>> openCursor(range);
>>> openCursor(range, dir);
>>> openCursor(null, dir);
>>>
>>> If not, could we order the two optional arguments such that it's rare
>>> to need to specify the second, but not the first?
>>>
>>
>> My biggest concern is about the future, if we add more params.
>>
>> If we're not worrying at all about the future, then I'd probably agree
>> that as is is best (since a cursor over everything backwards probably isn't
>> super common).
>>
>
I couldn't imagine why we would need additional arguments for openCursor?


>
>>
>>> > IDBKeyRange.bound
>>>
>>> On this one I feel rather strongly that option objects is overkill.
>>> Not only is it fairly clear what the arguments mean since they
>>> pair-wise match up to the two required arguments. I could even go with
>>> the following signature:
>>>
>>> IDBKeyRange bound(
>>>  in any lower,
>>>  in any upper,
>>>  in optional boolean lowerOpen,
>>>  in boolean upperOpen);
>>>
>>> That way you have to specify open-ness for both if you want to specify
>>> it for either.
>>>
>>
>> I didn't realize that was valid IDL and that's what it meant!  Sure,
>> you're probably right this is overkill. Requiring that they be in pairs or
>> not be there at all seems reasonable.
>>
>
The WebIDL spec has been going back and forth on this issue, but I believe
this is currently allowed. In any case I think parity with the other
constructors is pretty important. And if the optional syntax changes, we can
always do

IDBKeyRange bound(in any lower, in any upper, in boolean lowerOpen, in
boolean upperOpen);
IDBKeyRange bound(in any lower, in any upper);

To archive the same behavior.



>>  > We did all of these two weeks ago in Chromium and have gotten some
>>> feedback.
>>> >  The main downside is that typos are silently ignored by JavaScript.
>>>  We
>>> > considered throwing if someone passed in an option we didn't recognize,
>>> but
>>> > this would make it impossible to add more options later (which is one
>>> of the
>>> > main reasons for doing this change).  I think what we might do is just
>>> log
>>> > something in the console with this happens.  (Should the spec actually
>>> make
>>> > a recommendation to this effect?)  Besides that, I think overall we're
>>> happy
>>> > with the change.
>>>
>>> I'm with Pablo here. It seems unlikely that we'd introduce an argument
>>> in the future which we'd want old implementations to just silently
>>> ignore. For example, if we add 'required' as a boolean argument to
>>> createIndex in v2, I'd think we'd want an implementation which doesn't
>>> support that to throw. Similarly, if we add 'expression' as a way to
>>> solve bug 10000, we'd want a v1 implementation to throw, no?
>>>
>>
>> You're probably right, but how should we tell people to detect whether
>> certain implementations support certain features?  (Try/catches?  That seems
>> pretty ugly.
>>
>
I agree that try/catches are ugly for feature detection. But what is the
alternative? If we don't throw then feature detection becomes even harder,
no?

Actually, the main concern I have is that throwing requires enumerating all
properties on the JS-object which can be somewhat slow.

/ Jonas