[webkit-reviews] review denied: [Bug 3667] Core JavaScript 1.5 Reference:Objects:Array:forEach : [Attachment 2611] Patch that adds forEach(), some(), and every() Javascript 1.5 functionality to JavascriptCore

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Fri Jun 24 08:45:10 PDT 2005


Darin Adler <darin at apple.com> has denied Francisco Tolmasky
<tolmasky at gmail.com>'s request for review:
Bug 3667: Core JavaScript 1.5 Reference:Objects:Array:forEach
http://bugzilla.opendarwin.org/show_bug.cgi?id=3667

Attachment 2611: Patch that adds forEach(), some(), and every() Javascript 1.5
functionality to JavascriptCore
http://bugzilla.opendarwin.org/attachment.cgi?id=2611&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Looks great. There are some minor issues remaining that should be fixed before
we land this.

The URL in the comment only documents forEach, so it's not quite right any
more.

Formatting issues:

    1) Comment inside the case is not indented to match the code.
    2) "if(id" should be "if (id"
    3) single-line if and else, should be broken into multiple lines
    4) "for ( unsigned" should be "for (unsigned"
    5) indenting of if statements at end of loop looks like it's either wrong
or using tabs
    6) no need for else after an if statement that does a break

I think the if statement that sets up the result before the loop would be
clearer if it had a separate case for Some and for Every; the current version
is unnecessarily clever in a way that makes it slightly less clear.

The predicateResult boolean should be declared where it's initialized. There's
no reason to declare it outside the loop.



More information about the webkit-reviews mailing list