On Fri, 27 May 2022 18:40:32 GMT, XenoAmess <d...@openjdk.java.net> wrote:

>> as title.
> 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.


PR: https://git.openjdk.java.net/jdk/pull/8302

Reply via email to