(doc) Simplify CompilationFailureException #19
Conversation
rhowe
force-pushed
the
simpler-failureexception
branch
from
06dc84a
to
cee6ad8
Compare
|
Should this change have a Jira ticket? |
| @@ -43,13 +43,11 @@ public static String longMessage( List<CompilerMessage> messages ) | |||
| { | |||
| StringBuilder sb = new StringBuilder(); | |||
|
|
|||
| if ( messages != null ) | |||
| for ( CompilerMessage compilerError : messages ) | |||
There was a problem hiding this comment.
Let's use this opportunity to improve javadoc as well and to make clear that messages cannot be null.
There was a problem hiding this comment.
Sounds good to me - how's it looking now?
There was a problem hiding this comment.
Should these methods be private static or just inlined into the constructor? They have no other callers
There was a problem hiding this comment.
Oh, it's not possible to inline them because of the super() call
If the messages list is null, this class will throw a NullPointerException in its shortMessage() method, so having a check for a null message list in longMessage() seems unnecessary. Remove it.
rhowe
force-pushed
the
simpler-failureexception
branch
from
cee6ad8
to
8a5e02d
Compare
rhowe
force-pushed
the
simpler-failureexception
branch
from
8a5e02d
to
83b0343
Compare
|
I'm afraid the tests aren't working for me locally - |
|
Tests passed for me on an OS X device |
|
@rfscholte any love for this little cleanup? |
|
Jenkins was happy too, so PR is merged. Thanks for the patch! |
This class can be a little bit more concise.
The nullity check in
longMessage()is made redundant by the lack of an equivalent check inshortMessage.CompilationFailureExceptionis thrown in two places inAbstractCompilerMojoand both of those are guaranteed to pass a non-null object.Whilst we're in here, remove a redundant local variable in
shortMessage()and merge theStringBuildercalls onto a single line.