Hi Naoto,
Looks good, thanks for the updates.
Roger
On 8/19/20 2:26 PM, naoto.s...@oracle.com wrote:
Hi Roger,
Thank you for your comments. I've addressed your suggestions and
created a new webrev below:
https://cr.openjdk.java.net/~naoto/8251499/webrev.03/
Naoto
On 8/19/20 8:33 AM,
Hi Roger,
Thank you for your comments. I've addressed your suggestions and created
a new webrev below:
https://cr.openjdk.java.net/~naoto/8251499/webrev.03/
Naoto
On 8/19/20 8:33 AM, Roger Riggs wrote:
Hi Naoto,
CompactNumberFormat.java:
269: The field "placeHolders" should be named
Hi Naoto,
CompactNumberFormat.java:
269: The field "placeHolders" should be named consistently with the
other holders of Patterns.
-> placeHolderPatterns
1632: missing space before ":"
2390: I'm not sure why the stream processing is preferable to a direct
forEach(...).
Hi Naoto,
That's nice! The change looks good to me.
Regards,
Joe
On 8/18/20 11:37 AM, naoto.s...@oracle.com wrote:
Hi Joe,
Thank you for your comment. I consolidated those duplicated pieces
into one piece. Did not make it a private method, though, as it would
need to return two results
Hi Joe,
Thank you for your comment. I consolidated those duplicated pieces into
one piece. Did not make it a private method, though, as it would need to
return two results from the method (cnfMultiplier and whether to return
immediately for no-placeholder cases).
Hi Roger,
Thank you for your comment. I added a brief comment in the issue on how
the implementation behaves in the problem case.
Naoto
On 8/18/20 8:19 AM, Roger Riggs wrote:
Hi Naoto,
I think the issue would benefit from a comment describing the solution.
Its not clear how the code
Hi Naoto,
I think the issue would benefit from a comment describing the solution.
Its not clear how the code addresses the issue.
Thanks, Roger
On 8/17/20 7:42 PM, naoto.s...@oracle.com wrote:
Hi Joe,
It turned out that the previous fix did not address plural format
cases. That means that
Hi Naoto,
Looks good overall. One nit, blocks 1633-1639 and 1642-1649 may share a
common private method with a parameter that takes either matchedPosIndex
or matchedNegIndex, if you want.
Best,
Joe
On 8/17/20 4:42 PM, naoto.s...@oracle.com wrote:
Hi Joe,
It turned out that the previous
Hi Joe,
It turned out that the previous fix did not address plural format cases.
That means that just making the divisor negative to indicate
non-placeholder cannot distinguish multiple plural cases with the same
divisor. Instead, I created a list of placeholders (minimum digits) for
each
Hi Naoto,
Looks good to me.
While a negative divisor representing no zeros is newly introduced, the
"divisor > 0" checks seem to have always been beneficial. I had to
count the number of ""s in COMPACT_PATTERN13 :-)
Have a great weekend!
Joe
On 8/14/2020 3:20 PM, naoto.s...@oracle.com
Hello,
Please review the fix for the following issue:
https://bugs.openjdk.java.net/browse/JDK-8251499
The proposed changeset is located at:
https://cr.openjdk.java.net/~naoto/8251499/webrev.00/
The current implementation of CompactNumberFormat assumes that there is
always the number
11 matches
Mail list logo
