http://codereview.chromium.org/63100/diff/1/3 File test/mjsunit/array-shift.js (right):
http://codereview.chromium.org/63100/diff/1/3#newcode29 Line 29: arr = [1, 2, 3, 4, 5]; On 2009/04/08 01:01:18, iposva wrote: > The fact that the array indices and the values do not line up is rather > confusing. To distinguish indices and values, can you use ["a", "b", "c", "d", > "e"]? Or if you prefer not to use strings then use variables var a = 10; var b = > 20; ... arr = [a, b, c, d, e]; > In any case it would more clearly express your intentions if you used a > different value range for the array contents at least: arr = [100, 101, 102, > 103, 104, 105]; Done. http://codereview.chromium.org/63100/diff/1/3#newcode41 Line 41: assertArrayEquals([2,,4,,], arr); On 2009/04/08 01:01:18, iposva wrote: > Do you need the extra comma here before the closing bracket? Yeah, there is a hole here. The array equals would fail if it were omitted. http://codereview.chromium.org/63100/diff/1/3#newcode87 Line 87: assertFalse(1999 in arr); This test is verifying splicing, not that generic array creation/setup works. I could add more asserts, but I think it is gratuitous because we have other tests for that and because this test will still fail (albeit later). On 2009/04/08 01:01:18, iposva wrote: > How about also asserting that the indices between 2000 and 2100 are in the array > before the splicing operations? Similarly for the next chunks of missing and > available indices. http://codereview.chromium.org/63100/diff/1/2 File test/mjsunit/array-splice-webkit.js (right): http://codereview.chromium.org/63100/diff/1/2#newcode61 Line 61: arr = [1,2,3,4,5]; On 2009/04/08 01:01:18, iposva wrote: > Same concern about mixing indices and values... Done. http://codereview.chromium.org/63100 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
