[Webkit-unassigned] [Bug 16652] Firefox and JavaScriptCore differ in Number.toString(integer)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=16652


Sam Weinig <sam at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #99495|review?                     |review+
               Flag|                            |




--- Comment #11 from Sam Weinig <sam at webkit.org>  2011-07-01 13:47:26 PST ---
(From update of attachment 99495)
View in context: https://bugs.webkit.org/attachment.cgi?id=99495&action=review

Overall, this looks really good.  I think we should put the two classes in their own headers in WTF and even write some unit tests for them in TestWebKitAPI.  You should probably also move decomposeDouble to MathExtras.h and again, consider unit tests.

> Source/JavaScriptCore/runtime/NumberPrototype.cpp:130
> +// A value 1 greater than max uint16_t.
> +#define Uint16Infinity 0x10000

Please put a comment about why this is necessary to work around a compiler issue.  I also find the name a little confusing.  Perhaps be explicit and call it something like oneGreaterThanMaxUInt16.

> Source/JavaScriptCore/runtime/NumberPrototype.cpp:351
> +    bool isNormalized() const

Normalized is a bit odd here, since there are so many different concepts of normal in double-ish math.  Perhaps something lengthy including the word valid would help.

> Source/JavaScriptCore/runtime/NumberPrototype.cpp:494
> +    double fractionPart = number - floor(number);

The floor(number) is redundant, you can just use integerPart.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.