Java Puzzlers and FindBugs. Running then on the CDK silent module

The CDK has been using PMD for quite a while yet, but there is another tool, called FindBugs. I had seen this before, but until I watched two Java Puzzler videos John sent me (here is one), I had not used that much. There is a nice Eclipse plugin, and you can run it on any Java package.

As I was procrastinating anyway (I should be preparing my core teaching qualifications portfolio and prepare a presentation on the accessibility of chromatin in the human genome, in an upcoming ENCODE discuthon. Mind you, you have a lot of individual DNA molecules in your cells! Just your core 92 to start with and then your mitochondrial DNA (does all mitochondria in one cell have the same DNA??), and the fact that your DNA in your toes is unlikely to be the same on your nose (I learned that at the DiXA meeting in Berlin), or that your microbiome with its own DNA has a huge influence on your well-being?? Well, that was our dinner table discussion anyway).... (still breathing...)

... so, while I was procrastinating, I ran FindBugs on the org.openscience.cdk.silent package. I mean, what could possible go wrong? ...

Thirteen possible problems! I mean, seriously, this is the core of the CDK!
Here are three unit tests to uncover three of the found issues. If you like, take them "CDK Puzzlers"...

  @Test public void testCompare_MassNumberIntegers() {
    Isotope iso = new Isotope(Elements.CARBON);
    iso.setMassNumber(new Integer(12));
    Isotope iso2 = new Isotope(Elements.CARBON);
    iso2.setMassNumber(new Integer(12));
    Assert.assertTrue(iso.compare(iso2));
  }

So, we create two ¹²C isotopes, which should be the same. Of course, this test failed. The culprit is a == comparison in the compare() methods code, and Integer objects are not the same. Doing the same starting with ints goes better, even after casting, and the next test does not fail:

  @Test public void testCompare_MassNumber() {
    Isotope iso = new Isotope(Elements.CARBON);
    iso.setMassNumber(12);
    Isotope iso2 = new Isotope(Elements.CARBON);
    iso2.setMassNumber((int)12.0);
    Assert.assertTrue(iso.compare(iso2));
  }

And, indeed, using Integer.valueOf() helps too, something that PMD is keen on suggesting too: the next unit test runs fine too:

  @Test public void testCompare_MassNumberIntegers_ValueOf() {
    Isotope iso = new Isotope(Elements.CARBON);
    iso.setMassNumber(Integer.valueOf(12));
    Isotope iso2 = new Isotope(Elements.CARBON);
    iso2.setMassNumber(Integer.valueOf(12));
    Assert.assertTrue(iso.compare(iso2));
  }

The others two issues I wrote tests for, have the same underlying issue, causing these two tests to fail. But note that now we do not even need to use objects:

  @Test public void testCompare_ExactMass() {
    Isotope iso = new Isotope(Elements.CARBON);
    iso.setExactMass(12.000000);
    Isotope iso2 = new Isotope(Elements.CARBON);
    iso2.setExactMass(12.0);
    Assert.assertTrue(iso.compare(iso2));
  }

  @Test public void testCompare_NaturalAbundance() {
    Isotope iso = new Isotope(Elements.CARBON);
    iso.setNaturalAbundance(12.000000);
    Isotope iso2 = new Isotope(Elements.CARBON);
    iso2.setNaturalAbundance(12.0);
    Assert.assertTrue(iso.compare(iso2));
  }

The tests are filed as patch here.

I think my peer review has just become a bit more tough...