I installed and ran this. I love it. Let's not just assume we have to
work on the code until the results are 0 reports, though.
Lots of these are intentional, checked and OK, or just cases where the
bugfinder doesn't really understand what the purpose of the method is. I
think mostly it is a list that we should go through and satisfy
ourselves that all is OK. Really what we have here are 534 instances
where it would be easy to have a bug, and we are being told to be careful.
Things like this:
try {
while (readLine() != null) {
Method discards result of readLine after checking if it is nonnull
The value returned by readLine is discarded after checking to see if the
return value is non-null. In almost all situations, if the result is
non-null, you will want to use that non-null value. Calling readLine
again will give you a different line.
Whatever you do, don't "fix" that! readLine() sets the global line, and
FindBugs is not realizing that. The return of the line as well as the
setting of the global is a design efficiency so that the line can be
read and checked in this simple way.
Here's a good one, though:
public void run() {
if (reader != null) {
openReader(reader);
} else {
InputStream[] istream = new InputStream[nameAsGivenInThread.length];
for (int i = 0; i < nameAsGivenInThread.length; i++) {
Object t =
getInputStreamOrErrorMessageFromName(nameAsGivenInThread[i]);
if (! (t instanceof InputStream)) {
errorMessage = (t == null
? "error opening:" + nameAsGivenInThread
: (String)t);
nameAsGivenInThread is an array, so this IS a bug!
Oh, here's another, in Eval:
case Token.model:
int modelIndex = modelNumberParameter(++i);
if (modelIndex < 0) {
propertyName = "fixed";
propertyValue = Boolean.TRUE;
break;
}
propertyName = "modelIndex";
propertyValue = new Integer(modelIndex);
>>MISSING break;
case Token.string:
Oh, PdbReader....
int occupancy = 100;
float floatOccupancy = parseFloat(line, 54, 60);
if (floatOccupancy != Float.NaN)
Yes, well. We need to coordinate our efforts here. Nico, if you are
CERTAIN that something should be changed, by all means do so. But if you
have any question at all, please let me take a look.
Really great that you found this, Nico.
I'm off to Chicago through Wednesday.
Bob
Nicolas Vervelle wrote:
> Hi everyone,
>
> what about using a tool like Findbugs to do some debugging and code
> cleaning of Jmol ?
> I am testing it right now and going to fix a few things reported by
> Findbugs.
>
> To run Findbugs on Jmol code :
> 1. go to the URL http://indbugs.cs.umd.edu/demo/jnlp/findbugs.jnlp (it
> will run Findbugs using Java Web Start)
> 2. create a new project:
> - Name: Jmol ;)
> - Class archives and directories to analyze: Jmol/bin
> - Source directories: Jmol/src
>
> After a few minutes, the result is 574 reports
>
> Nico
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Jmol-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/jmol-developers