Code simplifications in AbstractMojo#47
Conversation
olamy
left a comment
olamy
left a comment
There was a problem hiding this comment.
lgtm
just minor comment.
maybe it's only me but I find adding final not adding any real value here
|
|
||
| StringBuilder fileName = new StringBuilder( resultFinalName ); | ||
|
|
||
| final String fileName; |
There was a problem hiding this comment.
not sure what is the need if final here?
There was a problem hiding this comment.
No real need, it's mostly just syntactic sugar in this case. It's somewhat a statement of intent indicating that the value will be calculated only once.
This was the kind of feedback I was seeking though - if you're happy with the changeset in principle, I'll take the finals out, cut a Jira ticket and make the PR more presentable etc.
There was a problem hiding this comment.
all good. please remove those final as I don't think they make sense for a local variable. except adding more useless code.
btw I'm more a sugar free person if it's not really needed 🤣
if you want to create a jira why not but I'm not sure we need a Jira for such code polishing.
it's up2u
There was a problem hiding this comment.
The less I need to touch Jira the better. OK, I've removed the finals and updated the PR description
There was a problem hiding this comment.
Oops, OK the final final is gone
| } | ||
| } | ||
|
|
||
| final String archiverName = containsModuleDescriptor ? "mjar" : "jar"; |
There was a problem hiding this comment.
oh oh, I know people they insinst on final...
Inverting this test leads to a more readable flow.
We can remove all branching from this method and return the check directly.
3 participants
Clean up a few overly-verbose code constructs:
StringBuilderwith string concatenation.elsebranch by returning early inAbstractMojo#projectHasAlreadySetAnArtifact()AbstractMojo#hasClassifier()