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

Reply via email to