Bug 87077 - [Forms][Meter][Progress] Change function signature of parseToDoubleForNumberType
Summary: [Forms][Meter][Progress] Change function signature of parseToDoubleForNumberType
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on:
Blocks: 80009
  Show dependency treegraph
 
Reported: 2012-05-21 20:53 PDT by yosin
Modified: 2012-05-22 00:52 PDT (History)
3 users (show)

See Also:


Attachments
Patch 1 (19.16 KB, patch)
2012-05-21 22:41 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 2 (19.17 KB, patch)
2012-05-22 00:10 PDT, yosin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-05-21 20:53:14 PDT
This proposal is for 
* reducing number of lines in call sites for ease of maintenance, 
* be functional style (no side effect and increasing chance to use floating pointer register)

== Current Pattern 1 ==
double numberValue;
if (!parseToDoubleForNumberType(string, &numberValue))
  return defaultValue;
ASSERT(isfinite(numberValue));
return numberValue;

== New Pattern 1 ==
return parseToDoubleForNumberType(string, defaultValue)

== Current Pattern  2 ==
double max;
bool ok = parseToDoubleForNumberType(string, &max);
if (!ok || max <= 0)
  return 1;
return max;

== New Pattern 2 ==
return max(parseToDoubleForNumberType(string), 1);

== Current Pattern 3 ==
return !value.isEmpty() && !parseToDoubleForNumberType(value, 0);

== New Pattern 3 ==
return !value.isEmpty() && isfinite(parseToDubleForNumberType(value));

== Current Pattern 4 ==
double min = 0;
parseToDoubleForNumberType(string, &min);
return min;

== New Pattern 4 ==
return parseToDoubleForNumberType(string, 0);
Comment 1 yosin 2012-05-21 22:41:50 PDT
Created attachment 143191 [details]
Patch 1
Comment 2 yosin 2012-05-21 23:25:01 PDT
Comment on attachment 143191 [details]
Patch 1

Could you review changes?

This change helps introducing decimal arithmetic for allowing us to use const reference of Decimal object.

Thanks in advance.
Comment 3 Hajime Morrita 2012-05-22 00:06:52 PDT
Comment on attachment 143191 [details]
Patch 1

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

> Source/WebCore/html/parser/HTMLParserIdioms.h:48
> +double parseToDoubleForNumberType(const String&, double);

It's worth giving a parameter name since the intention isn't clear for this case.

> Source/WebCore/html/parser/HTMLParserIdioms.h:50
> +double parseToDoubleForNumberTypeWithDecimalPlaces(const String&, unsigned*, double);

Ditto.
Comment 4 yosin 2012-05-22 00:10:34 PDT
Created attachment 143207 [details]
Patch 2
Comment 5 yosin 2012-05-22 00:11:14 PDT
Comment on attachment 143207 [details]
Patch 2

Thanks for quick response.
I updated as you suggested.
Could you review again?
Thanks in advance.
Comment 6 WebKit Review Bot 2012-05-22 00:52:39 PDT
Comment on attachment 143207 [details]
Patch 2

Clearing flags on attachment: 143207

Committed r117929: <http://trac.webkit.org/changeset/117929>
Comment 7 WebKit Review Bot 2012-05-22 00:52:54 PDT
All reviewed patches have been landed.  Closing bug.