Bug 16682

Summary: array_object.cpp needs a bath
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Remove minInt, maxInt
sam: review+
Apply wkstyle to array_object.cpp
sam: review+
More small cleanup to array_object.cpp sam: review+

Description Eric Seidel (no email) 2007-12-30 22:04:42 PST
array_object.cpp needs a bath

Ok, so I was on a long flight, and figured I might poke my head around JSC a bit more. Sadly, it's hard to actually read any code in some of these old files.  So I cleaned it up a little.  Still not much more readable.
Comment 1 Eric Seidel (no email) 2007-12-30 22:07:46 PST
Created attachment 18201 [details]
Remove minInt, maxInt

Add a few planned renames for JavaScriptCore
---
 JavaScriptCore/kjs/array_object.cpp   |   15 ++++++++-------
 JavaScriptCore/kjs/operations.cpp     |   10 ----------
 JavaScriptCore/kjs/operations.h       |    3 ---
 WebKitTools/Scripts/do-webcore-rename |    4 ++++
 4 files changed, 12 insertions(+), 20 deletions(-)
Comment 2 Eric Seidel (no email) 2007-12-30 22:07:47 PST
Created attachment 18202 [details]
Apply wkstyle to array_object.cpp

 JavaScriptCore/kjs/array_object.cpp |  250 +++++++++++++++++------------------
 1 files changed, 119 insertions(+), 131 deletions(-)
Comment 3 Eric Seidel (no email) 2007-12-30 22:07:49 PST
Created attachment 18203 [details]
More small cleanup to array_object.cpp

 JavaScriptCore/kjs/array_object.cpp |  154 +++++++++++++++-------------------
 1 files changed, 68 insertions(+), 86 deletions(-)
Comment 4 Sam Weinig 2007-12-30 22:24:24 PST
Comment on attachment 18201 [details]
Remove minInt, maxInt

This looks good, though I am not used to seeing the std::max/min with a template parameter.  Are you sure that is kosher?  Also, I think the change to do-webcore-rename should go in a separate commit.  And you need CHANGELOGS.
Comment 5 Darin Adler 2007-12-30 22:27:00 PST
Comment on attachment 18202 [details]
Apply wkstyle to array_object.cpp

-namespace KJS {
+namespace KJS
+{

This may be the wkstyle, but it's not what the style guidelines say.

-  : ArrayInstance(objProto, 0)
+        : ArrayInstance(objProto, 0)

Same here. It should be four spaces, not eight.
         if (curArg->isObject() &&
-            curObj->inherits(&ArrayInstance::info)) {
+                curObj->inherits(&ArrayInstance::info)) {

Why is this indented 10 spaces?
Comment 6 Darin Adler 2007-12-30 22:28:52 PST
Comment on attachment 18203 [details]
More small cleanup to array_object.cpp

+            ((ArrayInstance* )thisObj)->sort(exec, sortFunction);

+            ((ArrayInstance* )thisObj)->sort(exec);

Should delete the spaces here. Or use static_cast.
Comment 7 Sam Weinig 2007-12-30 22:36:21 PST
Comment on attachment 18202 [details]
Apply wkstyle to array_object.cpp

nice.  r=me
Comment 8 David Kilzer (:ddkilzer) 2007-12-31 06:09:09 PST
(In reply to comment #4)
> Also, I think the change to do-webcore-rename should go in a separate commit.

r29043

The other changes landed in r29045, r29046, and r29047.