<i18n dev> RFR 8177552: Compact Number Formatting support

[Roger Riggs]

Hi Nishit,

Some comments, a couple of possible bugs and several  performance 
related issues
could be deferred. Both formatting and parsing seem to be quite heavyweight
due to looping and combinatorics.


CompactNumberFormat.java


661, 727, 1507: Is there a reason to convert the divisor to a string and 
then re-parse it to create a new BigDecimal?
         (IntelliJ observes that divide is called without a rounding 
argument.)
         Given that the divisors are all powers of 10 and the digit list 
is a base 10 set of digits
         it seems more efficient to just move the decimal point then to 
do the math.
         BTW, the divisor.toString() is preferred over concat with "".  
(looks like a hack).

         It would be more efficient to write two methods that would pass 
the Number
         and return a BigInteger or BigDecimal by dispatching on the 
concrete type and
         using the appropriate constructor.

781:  @param prefix - the parameter name is suffix

804: move the int start = inside the if.

826:  expandAffix can be more efficient if tests the pattern for the 
presence of QUOTE
         and returns the pattern if the QUOTE is not present.  That 
would be the most common case.

914: Reduce the number of compares by reordering to:
         if number > currentValue; multiply and continue
         if number < currentValue break;
         else ==; assign matched index and break;

         In the normal case, there will be only one compare in the loop 
until it is to exit.

1109:  IntelliJ observes that within the case QUOTE, the if (ch == 
QUOTE) is always true
       so the if is redundant.

1205:  It looks odd to prepend two characters '- to the prefix.  Is the 
single quote correct?
       Where's the closing quote if so.

1394: matchedPosPrefix.length() is compared to negativePrefix.length().
       That's an unusual mixing of positive and negative! Please re-check.

1363:  Can there be an early exit from this loop if one of the prefixes 
is identified?
       The pattern of comparing for a prefix/suffix and the length might 
warrant
       creating a private method to reduce the repetitive parts of the code.

1593: extra space between "- ("; should be the same style as 1591

1627, 1363: Is an early exit from this loop possible?
        or a quick comparison to avoid the regionMatches.
        Do the regionMatches *only if* the prefix/suffix is longer than 
the suffix already compared?

1721:  IntelliJ observes that if (gotNeg) is redundant since 1708 rules 
out them being both true or both false.

Thanks, Roger

>
> On 11/28/18 3:46 AM, Nishit Jain wrote:
>> Thanks Naoto,
>>
>> Updated.
>>
>> http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.04/
>>