Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Wed, 1 Jun 2022 18:26:17 GMT, Naoto Sato wrote:
>> XenoAmess has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> do it as naotoj said
>
> src/java.base/share/classes/java/util/Calendar.java line 2648:
>
>> 2646: set.add("gregory");
>> 2647: set.add("buddhist");
>> 2648: set.add("japanese");
>
> This can be replaced with `SET = Set.of("gregory", "buddhist", "japanese");`.
@naotoj Yes it can. I did a further clean up to it, please have a look.
-
PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Wed, 1 Jun 2022 17:34:04 GMT, Stuart Marks wrote: >> XenoAmess has updated the pull request incrementally with one additional >> commit since the last revision: >> >> do it as naotoj said > > test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 441: > >> 439: } >> 440: } >> 441: > > This unifies the test cases between the Set and Map factories, which > accomplishes the primary goal. Good. > > The unification is achieved through classic object-oriented polymorphism, > which works fine, though which is rather verbose. This could probably be > reduced with some tinkering on the model, but it's probably reaching the > point where additional tinkering on the model isn't worth it. I'm ok with > sticking with this approach for now. Maybe we can clean it up later, or maybe > not -- it's at least fairly straightforward. > > One issue that contributes to the verbosity is the repeated null checking. > The null checking enables the test code to proceed and end up with -1 as the > capacity if there's a null in there somewhere. This will cause the assertion > to fail. This is good in that it will call attention to itself (as opposed to > silently passing or something). However, if the test cases are set up > properly, they should never run into null. If the null checking weren't done, > an unexpected null will throw NPE, which will be caught be the framework and > reported as an error. > > That seems perfectly fine to me, so I'd suggest simply removing the null > checking. That would also reduce the bulkiness of infrastructure. @stuart-marks done. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > do it as naotoj said Changes to `net` and `http` look good. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > do it as naotoj said `java.io` and `java.nio` look all right. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote:
>> as title.
>
> XenoAmess has updated the pull request incrementally with one additional
> commit since the last revision:
>
> do it as naotoj said
Reviewed i18n-related changes and they look good. One minor suggestion in
`Calendar`, but that can be applied later.
src/java.base/share/classes/java/util/Calendar.java line 2648:
> 2646: set.add("gregory");
> 2647: set.add("buddhist");
> 2648: set.add("japanese");
This can be replaced with `SET = Set.of("gregory", "buddhist", "japanese");`.
-
Marked as reviewed by naoto (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > do it as naotoj said Reviewers for i18n, net, nio, and security, please review call site changes in your areas. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8302
