Bug 70572 - Refactor OptionsObject.cpp
Summary: Refactor OptionsObject.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-20 19:14 PDT by Kentaro Hara
Modified: 2011-12-22 02:30 PST (History)
5 users (show)

See Also:


Attachments
Patch (11.55 KB, patch)
2011-10-20 19:45 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff
patch for commit (16.00 KB, patch)
2011-10-26 17:38 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2011-10-20 19:14:23 PDT
For example, OptionsObject::getKeyBool() is an alias of OptionsObject::getKeyValue(const String& key, bool& value). We should remove OptionsObject::getKeyXXXX() (XXXX is some specific type) and unify them into OptionsObject::getKeyValue(const String& key, XXXX& value).

c.f. Corresponding JSC methods are unified into JSDictionary::convertValue(JSC::ExecState*, JSC::JSValue, XXXX&).
Comment 1 Kentaro Hara 2011-10-20 19:45:45 PDT
Created attachment 111890 [details]
Patch
Comment 2 Adam Barth 2011-10-26 09:45:11 PDT
Comment on attachment 111890 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=111890&action=review

> Source/WebCore/bindings/v8/OptionsObject.h:54
> +    bool getKeyValue(const String&, bool&) const;
> +    bool getKeyValue(const String&, int32_t&) const;
> +    bool getKeyValue(const String&, double&) const;
> +    bool getKeyValue(const String&, String&) const;
> +    bool getKeyValue(const String&, ScriptValue&) const;

Why not just call these functions "get" ?

> Source/WebCore/bindings/v8/OptionsObject.h:61
> +    bool getKeyValueWithUndefinedOrNullCheck(const String&, String&) const;

getWithUndefinedOrNullCheck ?
Comment 3 Kentaro Hara 2011-10-26 17:38:14 PDT
Created attachment 112624 [details]
patch for commit
Comment 4 Kentaro Hara 2011-10-26 17:39:53 PDT
(In reply to comment #2)
> (From update of attachment 111890 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111890&action=review
> 
> > Source/WebCore/bindings/v8/OptionsObject.h:54
> > +    bool getKeyValue(const String&, bool&) const;
> > +    bool getKeyValue(const String&, int32_t&) const;
> > +    bool getKeyValue(const String&, double&) const;
> > +    bool getKeyValue(const String&, String&) const;
> > +    bool getKeyValue(const String&, ScriptValue&) const;
> 
> Why not just call these functions "get" ?
> 
> > Source/WebCore/bindings/v8/OptionsObject.h:61
> > +    bool getKeyValueWithUndefinedOrNullCheck(const String&, String&) const;
> 
> getWithUndefinedOrNullCheck ?

Done and committed. Thanks!!
Comment 5 WebKit Review Bot 2011-10-27 07:52:07 PDT
Comment on attachment 112624 [details]
patch for commit

Clearing flags on attachment: 112624

Committed r98572: <http://trac.webkit.org/changeset/98572>
Comment 6 Eric Seidel (no email) 2011-12-21 14:24:59 PST
Attachment 111890 [details] was posted by a committer and has review+, assigning to Kentaro Hara for commit.