Re: [w3c/IndexedDB] Rework key generator algorithm (#153)

>From my perspective, it looks good, but I have a few concerns:

1. Nit: I think in the context of spec language, "possibly update" sounds better than "maybe update".

2. I believe in the algorithm to "maybe update the key generator":

> 5. If |value| is greater than |generator|'s [=key generator/current number=],
>       then set |generator|'s [=key generator/current number=] to |value|.

should be something like the following (unless the former behavior is being changed):

> 5. If |value| is greater than |generator|'s [=key generator/current number=],
>       then set |generator|'s [=key generator/current number=] to |value| (rounded up to the nearest integer if it has a decimal value).

(Note that it is safe to use this language since you set `value` before this step to a maximum of 2^53.)

3. In the algorithm to "generate a key", with these steps:

> 3. If |key| is greater than 2<sup>53</sup> (9007199254740992), then return failure.
>
> 4. Increase |generator|'s  [=key generator/current number=] by 1.

...an implementation note should perhaps be added to mention that if `key` is already at 9007199254740992, then incrementing by one in the likes of ECMAScript will still lead to 9007199254740992 and thus failure will never be reached (and a value may be unintentionally overwritten). An alternative to adding an implementation note (and I think a safer one) would be changing the max instead to Number.MAX_SAFE_INTEGER (i.e., 2^53 - 1).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/IndexedDB/pull/153#issuecomment-281550162