Hi Nishit,
Thanks for the update. Looks fine.
I have no more comments.
Roger
On 12/04/2018 10:50 AM, Nishit Jain wrote:
Thanks Roger,
Updated Webrev:
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.06/
Regards,
Nishit Jain
On 04-12-2018 00:58, Roger Riggs wrote:
Hi Nishit,
Thanks Roger,
Updated Webrev:
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.06/
Regards,
Nishit Jain
On 04-12-2018 00:58, Roger Riggs wrote:
Hi Nishit,
Thanks for the updates and cleanup.
CompactNumberFormat.java:
- 827: To locate a single character use:
if
Hi Nishit,
Thanks for the updates and cleanup.
CompactNumberFormat.java:
- 827: To locate a single character use:
if (pattern.indexOf(QUOTE) < 0) { ... }
- 1488: Since infinite returns do not depend on any of the code after
line 1454,
the 1488- 1494 could be moved to 1454. (It
Thanks Roger,
Updated the webrev based on thebelow suggested changes and some clean up.
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.05/
On 01-12-2018 01:03, Roger Riggs wrote:
Hi Nishit,
Some comments, a couple of possible bugs and several performance
related issues
could
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
Looks good to me.
Naoto
On 11/28/18 3:46 AM, Nishit Jain wrote:
Thanks Naoto,
Updated.
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.04/
Regards,
Nishit Jain
On 27-11-2018 20:55, naoto.s...@oracle.com wrote:
Hi Nishit,
On 11/26/18 11:11 PM, Nishit Jain wrote:
Hi Naoto,
On
Thanks Naoto,
Updated.
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.04/
Regards,
Nishit Jain
On 27-11-2018 20:55, naoto.s...@oracle.com wrote:
Hi Nishit,
On 11/26/18 11:11 PM, Nishit Jain wrote:
Hi Naoto,
On 26-11-2018 21:01, naoto.s...@oracle.com wrote:
Hi Nishit,
On
Hi Nishit,
On 11/26/18 11:11 PM, Nishit Jain wrote:
Hi Naoto,
On 26-11-2018 21:01, naoto.s...@oracle.com wrote:
Hi Nishit,
On 11/26/18 12:41 AM, Nishit Jain wrote:
Hi Naoto,
To add to my previous mail comment, the DecimalFormat spec also says
that
/*"DecimalFormat can be instructed to
Hi Naoto,
On 26-11-2018 21:01, naoto.s...@oracle.com wrote:
Hi Nishit,
On 11/26/18 12:41 AM, Nishit Jain wrote:
Hi Naoto,
To add to my previous mail comment, the DecimalFormat spec also says
that
/*"DecimalFormat can be instructed to format and parse scientific
notation only via a
Hi Nishit,
Thanks for all the updates. The tests looks ok.
On 11/26/2018 02:56 AM, Nishit Jain wrote:
Hi Roger,
Please find my comments belowand check the updated webrev.
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.02/
Regards,
Nishit Jain
On 22-11-2018 00:04, Roger Riggs
Hi Nishit,
On 11/26/18 12:41 AM, Nishit Jain wrote:
Hi Naoto,
To add to my previous mail comment, the DecimalFormat spec also says that
/*"DecimalFormat can be instructed to format and parse scientific
notation only via a pattern; there is currently no factory method that
creates a
Hi Naoto,
To add to my previous mail comment, the DecimalFormat spec also says that
/*"DecimalFormat can be instructed to format and parse scientific
notation only via a pattern; there is currently no factory method that
creates a scientific notation format. In a pattern, the exponent
Hi Roger,
Please find my comments belowand check the updated webrev.
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.02/
Regards,
Nishit Jain
On 22-11-2018 00:04, Roger Riggs wrote:
Hi Nishit,
Comments on the tests:
- The tests looks to be quite complete.
- Have the locale
Hi Naoto,
> I think DecimalFormat and CNF should behave the same, ie. 'E' should
be treated as the exponent without a quote.
Personally I don't think that the exponential parsing should be
supported by CompactNumberFormat, because the objective of compact
numbers is to represent numbers in
Hi Nishit,
On 11/21/18 12:53 AM, Nishit Jain wrote:
Hi Naoto,
Updated the webrev based on suggestions
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.01/
Changes made:
- Replaced List with String[] to be added to the the resource
bundles
Good.
- refactored
Hi Nishit,
Comments on the tests:
- The tests looks to be quite complete.
- Have the locale specific data been independently verified?
Or are they just assumed to be correct based on using CNF to
generate the formatted strings?
- Is there any overlap between the format and parse patterns
Hi Naoto,
Updated the webrev based on suggestions
http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.01/
Changes made:
- Replaced List with String[] to be added to the the resource
bundles
- refactored DecimalFormat.subparse() to be used by the CNF.parse(), to
reduce code
Hi Stephen,
Thanks for the comments.
The NumberFormat.Style class just defines two styles and currently it is
only used by CompactNumberFormat, making it a top level class would
require it to be more general purpose and increases the scope. In which
case evolving it at a later point of time
Hi Nishit,
On 11/18/18 10:29 PM, Nishit Jain wrote:
Hi Naoto,
Please check my comments inline.
On 17-11-2018 04:52, naoto.s...@oracle.com wrote:
Hi Nishit,
Here are my comments:
- CLDRConverter: As the compact pattern no more employs List,
can we eliminate stringListEntry/Element, and use
I'm not a big fan of having a class named `Style` as it is commonly used in
business logic. (Yes, its an inner class, but I still think the potential
for annoyance is high). java.time.* has `TextStyle`, but I don't think it
can be reused here. Maybe the class should be honest and called
Hi Naoto,
Please check my comments inline.
On 17-11-2018 04:52, naoto.s...@oracle.com wrote:
Hi Nishit,
Here are my comments:
- CLDRConverter: As the compact pattern no more employs List,
can we eliminate stringListEntry/Element, and use Array equivalent
instead?
Since the CNF design does
Hi Nishit,
Here are my comments:
- CLDRConverter: As the compact pattern no more employs List,
can we eliminate stringListEntry/Element, and use Array equivalent instead?
- CompactNumberFormat.java
Multiple locations: Use StringBuilder instead of StringBuffer.
line 268: The link points to
Hi,
Please review this non trivial feature addition to NumberFormat API.
The existing NumberFormat API provides locale based support for
formatting and parsing numbers which includes formatting decimal,
percent, currency etc, but the support for formatting a number into a
human readable or
23 matches
Mail list logo
