Hi Christopher,
I agree that we should follow all kinds of good practices, and
FindBugs probably has a valid set of them in its default rules.
But... for the ErrorCode naming I'm a bit afraid to end up with many
long names for each extension of the base ErrorCode pseudo-enum.
And that this could reduce readability of code i.o. improving it.
E.g. for a typical usage of an extended ErrorCode such as
org.eclipse.triquetrum.processing.ErrorCode : in
DefaultTaskProcessingBroker there is a line like :
futResult.completeExceptionally(new
ProcessingException(ErrorCode.TASK_UNHANDLED, "No service found for
" + task, null));
I think this is quite readable, and better than e.g.
futResult.completeExceptionally(new
ProcessingException(ProcessingErrorCode.TASK_UNHANDLED, "No service
found for " + task, null));
We could maybe start using static imports if we go for the
specialized names? Then such code would become
futResult.completeExceptionally(new
ProcessingException(TASK_UNHANDLED, "No service found for " + task,
null));
Personally I'm not fond of static imports, but I see many of our
younger developers using them, so maybe I'm just a bit old-fashioned
;-)
What do you think?
regards
erwin
Op 25/01/2016 om 20:44 schreef
Christopher Brooks:
Erwin,
I've set up FindBugs, see
https://hudson.eclipse.org/triquetrum/job/triquetrum/findbugs
There are a bunch of warnings like https://hudson.eclipse.org/triquetrum/job/triquetrum/4/findbugsResult/module.232003606/
|
The class name
org.eclipse.triquetrum.validation.ErrorCode shadows
the simple name of the superclass
org.eclipse.triquetrum.ErrorCode
This class has a simple name that is identical to
that of its superclass, except that its superclass is
in a different package (e.g., alpha.Foo
extends beta.Foo). This can be
exceptionally confusing, create lots of situations in
which you have to look at import statements to resolve
references and creates many opportunities to
accidently define methods that do not override methods
in their superclasses.
|
One fix would be to rename
org.eclipse.triquetrum.validation.ErrorCode to
org.eclipse.triquetrum.validation.ErrorCodeValidation or
something.
Or, we could ignore it.
I've found that fixing as many FindBugs warnings as possible is
really helpful because it makes the actual bugs standout. The
downside of fixing these bugs is that some of the changes are not
natural.
What are your thoughts?
_Christopher
--
Christopher Brooks, PMP University of California
Academic Program Manager & Software Engineer US Mail: 337 Cory Hall
CHESS/iCyPhy/Ptolemy/TerraSwarm Berkeley, CA 94720-1774
cxh@xxxxxxxxxxxxxxxxx, 707.332.0670 (Office: 545Q Cory)
|
