More Java 7 related code improvement (20181027) #317
Conversation
obfischer
changed the title
| @@ -292,7 +292,8 @@ else if ( event.isEndElement() ) | |||
| { | |||
| // we want only those parts of pluginManagement that are defined in this project | |||
| Map<String, String> pluginManagement = new HashMap<>(); | |||
| try | |||
|
|
|||
| if ( model.getBuild().getPluginManagement() != null ) | |||
| { | |||
| for ( Plugin plugin : model.getBuild().getPluginManagement().getPlugins() ) | |||
There was a problem hiding this comment.
You can still get NullPointers here, if model.getBuild().getPluginManagement().getPlugins() returns null.
As long as you removed the try/catch statement, I think there should be a nullity verfiy before the for loop, or put back the try/catch
There was a problem hiding this comment.
I checked every getter if it could return null or not. A call to getPlugins() can never return null. Furthermore we would have a lot of NullPointerExceptions during the integration tests.
| // guess there are no plugins here | ||
| } | ||
| try | ||
| for ( Profile profile : model.getProfiles() ) |
There was a problem hiding this comment.
Same case here, you can still get NullPointers here, if model.getProfiles() returns null.
| { | ||
| try | ||
| for ( Plugin plugin : profile.getBuild().getPluginManagement().getPlugins() ) |
There was a problem hiding this comment.
Same case here, you can still get NullPointers if profile.getBuild().getPluginManagement().getPlugins() returns null
|
This PR is stale because it has been open 365 days with no activity. Remove stale label or comment or this will be closed in 30 days. |
|
Hi @fabiojb, I would like to discuss this pull request again with you, if it is fine for you. When I did my first contribution to the Versions plugin, it was very hard for me to deal with the existing code base. The programming style reminded me of C and was very hard to read. I mean it was very hard to understand the logic. So the goal of this pull request was to make the code easier to read and to understand. Improving the code base would help to get new contributors and to ease the adding of new features. I am looking forward to your answer and the dicussion with you. |
|
I will close this PR and will start to work on a new one. |
|
Hi @obfischer sorry for the delayed response. I can still review your code, but I cannot merge it, as I dont have the privileges to. |
|
Hi @fabiojb, no problem at all. I will open an issue and would like to start a discussion how to modernize the code base of the plugin. |
Serveral minor improvements to the code base as usage of the diamond operator, simplified exception handling and removal of not need try-catch-statements for NullPointerExceptions.