Looks fine.
Thanks, Roger
On 12/11/2018 01:33 PM, Naoto Sato wrote:
Hi Roger,
On 12/11/18 10:12 AM, Roger Riggs wrote:
Hi Naoto,
Looks ok,
I intended only the number of elements to be defined as a constant.
The other factors can be hard coded.
OK, I revised it again:
http://cr.openjdk.java.net/~naoto/8215194/webrev.02/
In the test, you will still have to edit the test when the number
changes.
I meant to avoid that edit. Though then may be there is not need for
the test at all.
Yes, if we just reflectively use the constant in
Character.UnicodeBlock in the test, it is guaranteed to succeed no
matter what. Thus I added the assertion. Still, the test ofHashMap()
succeeds till the block additions surpasses that 1024 capacity (thus
this was overlooked on Unicode 11 upgrade).
Naoto
Roger
On 12/11/2018 12:59 PM, Naoto Sato wrote:
Hi Roger,
Thanks. I updated it as suggested (incl. test using reflection):
http://cr.openjdk.java.net/~naoto/8215194/webrev.01/
Naoto
On 12/11/18 7:57 AM, Roger Riggs wrote:
Hi Naoto,
Since the value changes from time to time, it would give it some
visibility
if it were defined using a private final int (or float)
private final int MAP_CAPACITY = 667;
Though I suppose the test can't use the value without using
reflection.
But it would lower the maintenance in the long term.
$.02, Roger
On 12/11/2018 09:51 AM, Naoto Sato wrote:
Hi,
Please review the fix for the following issue:
https://bugs.openjdk.java.net/browse/JDK-8215194
The proposed fix is located at:
http://cr.openjdk.java.net/~naoto/8215194/webrev.00/
This one line fix is for the correctness of the initial map size
of Character.UnicodeBlock.
Naoto
