RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor}

Aleksey Shipilev shade at openjdk.org
Thu May 30 19:30:14 UTC 2024


On Wed, 22 May 2024 19:48:49 GMT, Chen Liang <liach at openjdk.org> wrote:

>> I really see no reason to try and save on re-use of this one-line method for _everything_. In fact, I do not quite see a very compelling reason to even have the utility method. Sharing the code between `Method` and `Constructor` is clean enough and provides a good balance between reuse and cleanliness. Everything else can have the copy of the method definition, if needed.
>
> Alternatively, if a utility method is overkill, we can inline these to `Executable`:
> 
> public Class<?>[] getParameterTypes() {
>     var shared = getSharedParameterTypes();
>     return shared.length == 0 ? shared : shared.clone();
> }
> 
> And the overrides in `Method` and `Constructor` will simply call super; the declarations are kept to preserve the API documentation.

I had to read JLS to confirm that changing the `abstract` method to non-abstract one does not break compatibility. 

I am still thinking that we are overthinking this: the readability/maintainability benefits for introducing a one-liner utility method are slim at best. I believe we are spending the disproportionate time on this. So if we cannot agree where to put the utility method -- which implies there is no good place for it -- let's not do it at all. Inline the ternary selector in 4 affected places, and be done with it.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19327#discussion_r1611304563


More information about the core-libs-dev mailing list