On Wed, 12 May 2021 15:20:28 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:

>> This PR replaces Class.newInstance() deprecated method with 
>> Contructor.newinstance().
>
> Ajit Ghaisas has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix apps and unit tests

I left a couple comments (in tests), where the replacement code isn't 
equivalent.

modules/javafx.controls/src/test/java/test/javafx/scene/control/cell/ParameterisedPrebuiltCellTest.java
 line 73:

> 71:         try {
> 72:             cell = cellClass.getDeclaredConstructor().newInstance();
> 73:         } catch (Exception e) {

This will now catch and ignore runtime exceptions as well, where it previously 
would cause the test to fail. This could end up missing a real problem. The two 
additional checked exceptions, along with any runtime exceptions, should cause 
the test to fail. In fact, I don't know why the existing exceptions are ignored 
(unless there is a compelling reason to ignore them that I'm not aware of), so 
probably the best thing to do would be to eliminate the try / catch entirely, 
and instead add a `throws Exception` to the method declaration.

modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java line 1329:

> 1327:                             value = 
> type.getDeclaredConstructor().newInstance();
> 1328:                         } catch (Exception e) {
> 1329:                             throw constructLoadException(e);

This isn't quite equivalent, since it will now wrap a `RuntimeException` in a 
`LoadException`, but I think that's OK in this case.

modules/javafx.graphics/src/test/java/test/com/sun/javafx/test/binding/ReflectionHelper.java
 line 46:

> 44:         try {
> 45:             return cls.getDeclaredConstructor().newInstance();
> 46:         } catch (final Exception e) {

This could re-wrap a `RuntimeException` needlessly, but is OK in this case 
(it's a test and it won't change whether or not it fails). The more equivalent 
fix would be to either list the checked exception individually, or to add a 
`catch (RuntimeException)` before the `catch (Exception)` and re-throw the rte 
without wrapping it.

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

PR: https://git.openjdk.java.net/jfx/pull/491

Reply via email to