On 28 Mar 2013 13:27, <[email protected]> wrote:
>
> LGTM with comments
>
>
> https://codereview.chromium.org/13071006/diff/1/src/array.js
> File src/array.js (right):
>
> https://codereview.chromium.org/13071006/diff/1/src/array.js#newcode44
> src/array.js:44: if (length == 2 && intervals[0] == -1) {
> Nit: using === might be more accurate (and marginally more efficient).
> Same in other places below.
>
> https://codereview.chromium.org/13071006/diff/1/src/array.js#newcode44
> src/array.js:44: if (length == 2 && intervals[0] == -1) {
> Do you actually need to test that length is 2? An index of -1 seems
> unambiguous under the new invariant.

And in fact, can't you simply replace the whole case of "interval" by a
plain number for the upper bound?

>
> https://codereview.chromium.org/13071006/diff/1/src/array.js#newcode1254
> src/array.js:1254: // A single interval.
> Nit: adjust comment ("An interval")
>
> https://codereview.chromium.org/13071006/diff/1/src/array.js#newcode1312
> src/array.js:1312: // A single interval.
> Same here
>
> https://codereview.chromium.org/13071006/

-- 
-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- 
You received this message because you are subscribed to the Google Groups 
"v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to