[
https://issues.apache.org/jira/browse/FLINK-8933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16475849#comment-16475849
]
ASF GitHub Bot commented on FLINK-8933:
---------------------------------------
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/6016
I am not sure we should be doing it like this.
The point of discouraging `Class.newInstance()` is not to copy replace this
by a different call. That will not magically make anything better. The whole
point is to go from a method that may sneakily throw undeclared exceptions, to
a method that declares exceptions.
All places that do not require a change to the try/catch statement (add
`InvokationTargetException`) have reasonable exception handling in place
already, and are not vulnerable to the motivating issue.
In fact, for now this makes thing worse because Exceptions get wrapped into
`InvokationTargetException`, with the root cause in the `getTargetException()`,
not in the exception chain. That would actually be a serious regression.
As a more general comment: Copy/replace style pull requests are a bit
tricky, in my opinion - they are rather easy to create (bulk fix tool) and
require committers to put up significant time to dig into the matter and to
double check that it actually does not cause a regression in the individual
spots that are changed. One can probably keep all committers busy for a year
with just such fixes without really changing anything for users for the better,
and having a high risk of regressions.
I would personally vote to not have many of those pull requests, or clearly
label them as "cosmetic" and give them a very low priority in reviewing.
Especially given that we have a decent backlog of pull requests for changes
that where users will see a difference.
> Avoid calling Class#newInstance
> -------------------------------
>
> Key: FLINK-8933
> URL: https://issues.apache.org/jira/browse/FLINK-8933
> Project: Flink
> Issue Type: Task
> Reporter: Ted Yu
> Assignee: vinoyang
> Priority: Minor
>
> Class#newInstance is deprecated starting in Java 9 -
> https://bugs.openjdk.java.net/browse/JDK-6850612 - because it may throw
> undeclared checked exceptions.
> The suggested replacement is getDeclaredConstructor().newInstance(), which
> wraps the checked exceptions in InvocationException.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
