#284 Fixes for invalid formula generation

Needs_Review
closed
cdk-1.4.x (181)
5
2012-10-28
2010-10-16
Rajarshi Guha
No

The associated patches simplify the MolecularFormula class and fix various issues in the MolecularFormulaManipulator class. This is primarily meant to address bug 3071473. It does involve modifications to the Isotope class, by providing a proper equals and hashCode class - the reason is that the MolecularFormula class was reimiplementing behavior (ie equals) that would be more appropriately located in Isotope. Overall, code is simplified and easier to work with

Unit tests have been added

Discussion

  • Rajarshi Guha
    Rajarshi Guha
    2010-10-16

    (Also the firs patch included some of my testing code by mistake, but later patches clean this up.)

     
  • Looks good.

     
  • Rajarshi Guha
    Rajarshi Guha
    2010-10-17

    Did this get pushed? I don't see it in the logs for 1.4.x

     
  • Yeah, got pushed... but accidentally as branch :)

     
  • Now pushed to proper branch. Thanx for letting me know about this stupid error :)

     
  • Reopened patch. I broke hundreds of unit tests :) That will teach me not to run the unit tests before committing...

    I guess the patch needs some revisions ;)

     
  • Rajarshi Guha
    Rajarshi Guha
    2010-10-19

    latest patch fixes the bug and doesn't break anything else :)

     
  • Fails to compile with 'ant clean dist-all test-dist-all':

    compile-module:
    [echo] Compiling classes for CDK's formula module from src/main...
    [echo] Datafiles defined: ${module.datafiles.present}
    [echo] Extra files defined: ${module.extrafiles.present}
    [echo] CDK dependencies defined: true
    [echo] Library dependencies defined: true
    [echo] Developer Library dependencies defined: ${module.devellibdepends.present}
    [echo] Autogenerated files specified: ${module.autogenerated.present}
    [mkdir] Created dir: /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula
    [mkdir] Created dir: /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/formula
    [copy] Copying 19 files to /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula
    [javac] Compiling 19 source files to /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/formula
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:37: cannot find symbol
    [javac] symbol : class DefaultChemObjectBuilder
    [javac] location: package org.openscience.cdk
    [javac] import org.openscience.cdk.DefaultChemObjectBuilder;
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:38: cannot find symbol
    [javac] symbol : class ChemFile
    [javac] location: package org.openscience.cdk
    [javac] import org.openscience.cdk.ChemFile;
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:39: package org.openscience.cdk.io does not exist
    [javac] import org.openscience.cdk.io.MDLReader;
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:40: cannot find symbol
    [javac] symbol : class CDKHydrogenAdder
    [javac] location: package org.openscience.cdk.tools
    [javac] import org.openscience.cdk.tools.CDKHydrogenAdder;
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:41: cannot find symbol
    [javac] symbol : class SmilesParser
    [javac] location: package org.openscience.cdk.smiles
    [javac] import org.openscience.cdk.smiles.SmilesParser;
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:69: cannot find symbol
    [javac] symbol : class SmilesParser
    [javac] location: class org.openscience.cdk.tools.manipulator.MolecularFormulaManipulator
    [javac] SmilesParser sp = new SmilesParser(DefaultChemObjectBuilder.getInstance());
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:69: cannot find symbol
    [javac] symbol : class SmilesParser
    [javac] location: class org.openscience.cdk.tools.manipulator.MolecularFormulaManipulator
    [javac] SmilesParser sp = new SmilesParser(DefaultChemObjectBuilder.getInstance());
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:69: cannot find symbol
    [javac] symbol : variable DefaultChemObjectBuilder
    [javac] location: class org.openscience.cdk.tools.manipulator.MolecularFormulaManipulator
    [javac] SmilesParser sp = new SmilesParser(DefaultChemObjectBuilder.getInstance());
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:80: cannot find symbol
    [javac] symbol : class CDKHydrogenAdder
    [javac] location: class org.openscience.cdk.tools.manipulator.MolecularFormulaManipulator
    [javac] CDKHydrogenAdder ha = CDKHydrogenAdder.getInstance(DefaultChemObjectBuilder.getInstance());
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:80: cannot find symbol
    [javac] symbol : variable DefaultChemObjectBuilder
    [javac] location: class org.openscience.cdk.tools.manipulator.MolecularFormulaManipulator
    [javac] CDKHydrogenAdder ha = CDKHydrogenAdder.getInstance(DefaultChemObjectBuilder.getInstance());
    [javac] ^
    [javac] /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/tools/manipulator/MolecularFormulaManipulator.java:80: cannot find symbol
    [javac] symbol : variable CDKHydrogenAdder
    [javac] location: class org.openscience.cdk.tools.manipulator.MolecularFormulaManipulator
    [javac] CDKHydrogenAdder ha = CDKHydrogenAdder.getInstance(DefaultChemObjectBuilder.getInstance());
    [javac] ^
    [javac] Note: /mnt/sda9/egonw/var/Projects/SourceForge/git/cdk/build/src/formula/org/openscience/cdk/formula/rules/IsotopePatternRule.java uses unchecked or unsafe operations.
    [javac] Note: Recompile with -Xlint:unchecked for details.
    [javac] 11 errors

    It seems to come from the debug code... I also spotted a use of STDOUT which you could perhaps remove...

     
  • Rajarshi Guha
    Rajarshi Guha
    2010-10-20

    It compiles cleanly for me when applied to 1.4.x. I've reuploaded the patch just to be sure

     
  • Yeah, this patch works fine here.