[Webkit-unassigned] [Bug 147602] [INTL] Implement Intl.NumberFormat.prototype.resolvedOptions ()

[bugzilla-daemon at webkit.org]

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

--- Comment #20 from Sukolsak Sakshuwong <sukolsak at gmail.com> ---
(In reply to comment #18)
> Comment on attachment 267624 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=267624&action=review
> 
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:92
> > +    Vector<String> keyLocaleData({
> 
> Seems unfortunate to re-create this vector every time the function is
> called. Might consider changing the function’s interface to return a const&
> unless it can involve computing a unique value and using a static
> NeverDestroyed for this particular case.

This function is passed to resolveLocale. Other classes that use resolveLocale can pass functions that compute a unique value to it, such as sortLocaleData in IntlCollator.cpp.

There is a way to optimize it, which requires some changes to the algorithm in the spec. But I think we should wait until all Intl classes are implemented first.

> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:158
> > +    for (const auto& currencyMinorUnit : currencyMinorUnits) {
> 
> This list might be long enough that's binary search would be faster than a
> linear one.

Done. Not sure if I do it correctly. Could you please take a look?

> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:159
> > +        if (currency == currencyMinorUnit.first)
> 
> Is it correct that this is a case sensitive check?

Yes, it is correct. Before we call we this function, we convert the currency to uppercase letters.

> > Source/JavaScriptCore/runtime/IntlObject.cpp:239
> > +        if (std::isnan(doubleValue) || doubleValue < static_cast<double>(minimum) || doubleValue > static_cast<double>(maximum)) {
> 
> There is a way to write this without an explicit check for NaN.
> 
>     If (!(x >= min && x <= max))

Done. Nice trick!

> > Source/JavaScriptCore/runtime/IntlObject.cpp:245
> > +        return static_cast<unsigned>(doubleValue);
> 
> This has peculiar behavior for max + 0.1; do we have test cases that cover
> values just slightly under min and slightly over max?

Yes, we have test cases for those cases. For example, the allowed range of minimumIntegerDigits is [1, 21]. So, we test
- 0    (throws)
- 0.9  (throws)
- 1
- 21
- 21.1 (throws)
- 22   (throws)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151221/8f566821/attachment-0001.html>