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