[MJAVADOC-825] Prefer NullPointerExceptions for null arguments by elharo · Pull Request #350 · apache/maven-javadoc-plugin (original) (raw)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given IAEx being declared as thrown by, perhaps NPEx could also be added for style consistency?
Or IAEx declaration removed?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both have @throws clauses. Or did you mean something else?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IllegalArgumentException is RTEx (as NPEx is). And is declared as being thrown by this method. Thus - let's add NullPointerException to the throws list.
Or remove throws.
Here, and in the second modified method.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that's already done in this PR. Are you looking at the left side (old code) instead of the right side (new code)?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this:

protected static String extractJavadocVersion(String output) throws IllegalArgumentException {

I'd like to see this instead:

protected static String extractJavadocVersion(String output) throws IllegalArgumentException, NullPointerException {