Only store the atomic number of an atom and not it's redundant symbol.

cdk · Jun 27, 2018 · e9327af · e9327af
1 parent c2f7763
commit e9327af
Show file tree

Hide file tree

Showing 14 changed files with 84 additions and 40 deletions.
diff --git a/base/core/src/main/java/org/openscience/cdk/config/AtomTypeFactory.java b/base/core/src/main/java/org/openscience/cdk/config/AtomTypeFactory.java
@@ -26,6 +26,7 @@
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.openscience.cdk.exception.CDKException;
 import org.openscience.cdk.exception.NoSuchAtomTypeException;
@@ -280,8 +281,7 @@ public IAtomType[] getAtomTypes(String symbol) {
         logger.debug("Request for atomtype for symbol ", symbol);
         List<IAtomType> atomList = new ArrayList<IAtomType>();
         for (IAtomType atomType : atomTypes.values()) {
-            // logger.debug("  does symbol match for: ", atomType);
-            if (atomType.getSymbol().equals(symbol)) {
+            if (Objects.equals(atomType.getSymbol(), symbol)) {
                 atomList.add(atomType);
             }
         }

diff --git a/base/data/src/main/java/org/openscience/cdk/Element.java b/base/data/src/main/java/org/openscience/cdk/Element.java
@@ -25,6 +25,7 @@
 package org.openscience.cdk;
 
 import com.google.common.base.Objects;
+import org.openscience.cdk.config.Elements;
 import org.openscience.cdk.interfaces.IElement;
 import org.openscience.cdk.tools.periodictable.PeriodicTable;
 
@@ -60,18 +61,14 @@ public class Element extends ChemObject implements Serializable, IElement, Clone
      */
     private static final long serialVersionUID = 3062529834691231436L;
 
-    /** The element symbol for this element as listed in the periodic table. */
-    protected String          symbol;
-
     /** The atomic number for this element giving their position in the periodic table. */
-    protected Integer         atomicNumber     = (Integer) CDKConstants.UNSET;
+    protected Integer atomicNumber = null;
 
     /**
      * Constructs an empty Element.
      */
     public Element() {
         super();
-        this.symbol = null;
     }
 
     /**
@@ -83,7 +80,6 @@ public Element() {
      */
     public Element(IElement element) {
         super(element);
-        this.symbol = element.getSymbol();
         this.atomicNumber = element.getAtomicNumber();
     }
 
@@ -94,7 +90,8 @@ public Element(IElement element) {
      * @param   symbol The element symbol that this element should have.
      */
     public Element(String symbol) {
-        this(symbol, PeriodicTable.getAtomicNumber(symbol));
+        super();
+        setSymbolInternal(symbol);
     }
 
     /**
@@ -105,7 +102,6 @@ public Element(String symbol) {
      * @param   atomicNumber  The atomicNumber of this element.
      */
     public Element(String symbol, Integer atomicNumber) {
-        this.symbol = symbol;
         this.atomicNumber = atomicNumber;
     }
 
@@ -153,7 +149,11 @@ public void setAtomicNumber(Integer atomicNumber) {
      */
     @Override
     public String getSymbol() {
-        return this.symbol;
+        if (atomicNumber == null)
+            return null;
+        if (atomicNumber == 0)
+            return "R";
+        return Elements.ofNumber(atomicNumber).symbol();
     }
 
     /**
@@ -165,10 +165,17 @@ public String getSymbol() {
      */
     @Override
     public void setSymbol(String symbol) {
-        this.symbol = symbol;
+        setSymbolInternal(symbol);
         notifyChanged();
     }
 
+    private void setSymbolInternal(String symbol) {
+        if (symbol == null)
+            this.atomicNumber = null;
+        else
+            this.atomicNumber = Elements.ofString(symbol).number();
+    }
+
     @Override
     public String toString() {
         StringBuffer resultString = new StringBuffer(32);
@@ -206,6 +213,6 @@ public boolean compare(Object object) {
             return false;
         }
         Element elem = (Element) object;
-        return Objects.equal(atomicNumber, elem.atomicNumber) && Objects.equal(symbol, elem.symbol);
+        return Objects.equal(atomicNumber, elem.atomicNumber);
     }
 }
diff --git a/base/data/src/main/java/org/openscience/cdk/PseudoAtom.java b/base/data/src/main/java/org/openscience/cdk/PseudoAtom.java
@@ -83,11 +83,11 @@ public PseudoAtom(String label) {
      */
     public PseudoAtom(IElement element) {
         super(element);
+        setAtomicNumber(0);
         if (element instanceof IPseudoAtom) {
             this.label = ((IPseudoAtom) element).getLabel();
         } else {
-            super.symbol = "R";
-            this.label = element.getSymbol();
+            this.label = "R";
         }
     }
 

diff --git a/base/data/src/test/java/org/openscience/cdk/ElementTest.java b/base/data/src/test/java/org/openscience/cdk/ElementTest.java
@@ -72,7 +72,7 @@ public void testElement_String() {
     @Test
     public void testElement_X() {
         IElement e = new Element("X");
-        Assert.assertEquals("X", e.getSymbol());
+        Assert.assertEquals("R", e.getSymbol());
         // and it should not throw exceptions
         Assert.assertNotNull(e.getAtomicNumber());
         Assert.assertThat(e.getAtomicNumber(), is(0));
@@ -108,8 +108,10 @@ public void compareDiffSymbol() {
 
     @Test
     public void compareDiffAtomicNumber() {
-        Element e1 = new Element(new String("H"), 1);
-        Element e2 = new Element(new String("H"), null);
+        Element e1 = new Element();
+        Element e2 = new Element();
+        e1.setAtomicNumber(1);
+        e1.setAtomicNumber(2);
         Assert.assertFalse(e1.compare(e2));
     }
 }
diff --git a/base/datadebug/src/test/java/org/openscience/cdk/debug/DebugElementTest.java b/base/datadebug/src/test/java/org/openscience/cdk/debug/DebugElementTest.java
@@ -68,7 +68,7 @@ public void testDebugElement_String() {
     @Test
     public void testElement_X() {
         IElement e = new DebugElement("X");
-        Assert.assertEquals("X", e.getSymbol());
+        Assert.assertEquals("R", e.getSymbol());
         // and it should not throw exceptions
         Assert.assertNotNull(e.getAtomicNumber());
         Assert.assertThat(e.getAtomicNumber(), is(0));

diff --git a/base/silent/src/main/java/org/openscience/cdk/silent/Element.java b/base/silent/src/main/java/org/openscience/cdk/silent/Element.java
@@ -23,6 +23,7 @@
 
 import com.google.common.base.Objects;
 import org.openscience.cdk.CDKConstants;
+import org.openscience.cdk.config.Elements;
 import org.openscience.cdk.interfaces.IElement;
 import org.openscience.cdk.tools.periodictable.PeriodicTable;
 
@@ -56,18 +57,14 @@ public class Element extends ChemObject implements Serializable, IElement, Clone
      */
     private static final long serialVersionUID = 3062529834691231436L;
 
-    /** The element symbol for this element as listed in the periodic table. */
-    protected String          symbol;
-
     /** The atomic number for this element giving their position in the periodic table. */
-    protected Integer         atomicNumber     = (Integer) CDKConstants.UNSET;
+    protected Integer atomicNumber = null;
 
     /**
      * Constructs an empty Element.
      */
     public Element() {
         super();
-        this.symbol = null;
     }
 
     /**
@@ -79,7 +76,6 @@ public Element() {
      */
     public Element(IElement element) {
         super(element);
-        this.symbol = element.getSymbol();
         this.atomicNumber = element.getAtomicNumber();
     }
 
@@ -90,7 +86,8 @@ public Element(IElement element) {
      * @param   symbol The element symbol that this element should have.
      */
     public Element(String symbol) {
-        this(symbol, PeriodicTable.getAtomicNumber(symbol));
+        this();
+        setSymbolInternal(symbol);
     }
 
     /**
@@ -101,7 +98,6 @@ public Element(String symbol) {
      * @param   atomicNumber  The atomicNumber of this element.
      */
     public Element(String symbol, Integer atomicNumber) {
-        this.symbol = symbol;
         this.atomicNumber = atomicNumber;
     }
 
@@ -148,7 +144,11 @@ public void setAtomicNumber(Integer atomicNumber) {
      */
     @Override
     public String getSymbol() {
-        return this.symbol;
+        if (atomicNumber == null)
+            return null;
+        if (atomicNumber == 0)
+            return "R";
+        return Elements.ofNumber(atomicNumber).symbol();
     }
 
     /**
@@ -160,7 +160,14 @@ public String getSymbol() {
      */
     @Override
     public void setSymbol(String symbol) {
-        this.symbol = symbol;
+        setSymbolInternal(symbol);
+    }
+
+    private void setSymbolInternal(String symbol) {
+        if (symbol == null)
+            this.atomicNumber = null;
+        else
+            this.atomicNumber = Elements.ofString(symbol).number();
     }
 
     @Override
@@ -200,6 +207,6 @@ public boolean compare(Object object) {
             return false;
         }
         Element elem = (Element) object;
-        return Objects.equal(atomicNumber, elem.atomicNumber) && Objects.equal(symbol, elem.symbol);
+        return Objects.equal(atomicNumber, elem.atomicNumber);
     }
 }
diff --git a/base/silent/src/main/java/org/openscience/cdk/silent/PseudoAtom.java b/base/silent/src/main/java/org/openscience/cdk/silent/PseudoAtom.java
@@ -83,11 +83,11 @@ public PseudoAtom(String label) {
      */
     public PseudoAtom(IElement element) {
         super(element);
+        setAtomicNumber(0);
         if (element instanceof IPseudoAtom) {
             this.label = ((IPseudoAtom) element).getLabel();
         } else {
-            super.symbol = "R";
-            this.label = element.getSymbol();
+            this.label = "R";
         }
     }
 

diff --git a/base/silent/src/test/java/org/openscience/cdk/silent/ElementTest.java b/base/silent/src/test/java/org/openscience/cdk/silent/ElementTest.java
@@ -69,7 +69,7 @@ public void testElement_String() {
     @Test
     public void testElement_X() {
         IElement e = new Element("X");
-        Assert.assertEquals("X", e.getSymbol());
+        Assert.assertEquals("R", e.getSymbol());
         // and it should not throw exceptions
         Assert.assertNotNull(e.getAtomicNumber());
         Assert.assertThat(e.getAtomicNumber(), is(0));

diff --git a/base/test-interfaces/src/test/java/org/openscience/cdk/interfaces/AbstractElementTest.java b/base/test-interfaces/src/test/java/org/openscience/cdk/interfaces/AbstractElementTest.java
@@ -43,8 +43,8 @@ public void testSetSymbol_String() {
     @Test
     public void testGetSymbol() {
         IElement e = (IElement) newChemObject();
-        e.setSymbol("X");
-        Assert.assertEquals("X", e.getSymbol());
+        e.setSymbol("Ir");
+        Assert.assertEquals("Ir", e.getSymbol());
     }
 
     @Test

diff --git a/storage/ctab/src/test/java/org/openscience/cdk/io/MDLV2000ReaderTest.java b/storage/ctab/src/test/java/org/openscience/cdk/io/MDLV2000ReaderTest.java
@@ -1047,7 +1047,7 @@ public void testPseudoAtomLabels() throws Exception {
         molecule = reader.read(molecule);
         reader.close();
         assertTrue(molecule.getAtom(4) instanceof IPseudoAtom);
-        Assert.assertEquals("Gln", molecule.getAtom(4).getSymbol());
+        Assert.assertEquals("R", molecule.getAtom(4).getSymbol());
         IPseudoAtom pa = (IPseudoAtom) molecule.getAtom(4);
         Assert.assertEquals("Gln", pa.getLabel());
     }

diff --git a/storage/ctab/src/test/java/org/openscience/cdk/io/MDLV3000ReaderTest.java b/storage/ctab/src/test/java/org/openscience/cdk/io/MDLV3000ReaderTest.java
@@ -111,7 +111,7 @@ public void testPseudoAtomLabels() throws Exception {
             molecule = reader.read(molecule);
             reader.close();
             Assert.assertTrue(molecule.getAtom(9) instanceof IPseudoAtom);
-            Assert.assertEquals("Leu", molecule.getAtom(9).getSymbol());
+            Assert.assertEquals("R", molecule.getAtom(9).getSymbol());
             IPseudoAtom pa = (IPseudoAtom) molecule.getAtom(9);
             Assert.assertEquals("Leu", pa.getLabel());
         }

diff --git a/storage/smiles/src/test/java/org/openscience/cdk/smiles/BeamToCDKTest.java b/storage/smiles/src/test/java/org/openscience/cdk/smiles/BeamToCDKTest.java
@@ -439,7 +439,7 @@ public void readAtomClass() throws Exception {
     @Test
     public void erroneousLabels_tRNA() throws Exception {
         IAtomContainer ac = convert("[tRNA]CC");
-        assertThat(ac.getAtom(0).getSymbol(), is("*"));
+        assertThat(ac.getAtom(0).getSymbol(), is("R"));
         assertThat(ac.getAtom(0), is(instanceOf(IPseudoAtom.class)));
         assertThat(((IPseudoAtom) ac.getAtom(0)).getLabel(), is("tRNA"));
     }
@@ -449,7 +449,7 @@ public void erroneousLabels_tRNA() throws Exception {
     @Test
     public void erroneousLabels_nested() throws Exception {
         IAtomContainer ac = convert("[now-[this]-is-mean]CC");
-        assertThat(ac.getAtom(0).getSymbol(), is("*"));
+        assertThat(ac.getAtom(0).getSymbol(), is("R"));
         assertThat(ac.getAtom(0), is(instanceOf(IPseudoAtom.class)));
         assertThat(((IPseudoAtom) ac.getAtom(0)).getLabel(), is("now-[this]-is-mean"));
     }

diff --git a/tool/pcore/src/main/java/org/openscience/cdk/pharmacophore/PharmacophoreAtom.java b/tool/pcore/src/main/java/org/openscience/cdk/pharmacophore/PharmacophoreAtom.java
@@ -47,6 +47,7 @@ public class PharmacophoreAtom extends Atom {
 
     private String smarts;
     private int[]  matchingAtoms;
+    private String symbol;
 
     /**
      * Create a pharmacophore group.
@@ -107,6 +108,16 @@ public String getSmarts() {
         return smarts;
     }
 
+    @Override
+    public void setSymbol(String symbol) {
+        this.symbol = symbol;
+    }
+
+    @Override
+    public String getSymbol() {
+        return symbol;
+    }
+
     /**
      * Set the atoms of a target molecule that correspond to this group.
      * 

diff --git a/tool/pcore/src/main/java/org/openscience/cdk/pharmacophore/PharmacophoreQueryAtom.java b/tool/pcore/src/main/java/org/openscience/cdk/pharmacophore/PharmacophoreQueryAtom.java
@@ -44,6 +44,7 @@ public class PharmacophoreQueryAtom extends Atom implements IQueryAtom {
 
     private String smarts;
     private IQueryAtomContainer[] compiledSmarts;
+    private String symbol;
 
     /**
      * Creat a new query pharmacophore group
@@ -52,7 +53,7 @@ public class PharmacophoreQueryAtom extends Atom implements IQueryAtom {
      * @param smarts The SMARTS pattern to be used for matching
      */
     public PharmacophoreQueryAtom(String symbol, String smarts) {
-        setSymbol(symbol);
+        this.symbol = symbol;
         this.smarts = smarts;
         // Note that we allow a special form of SMARTS where the | operator
         // represents logical or of multi-atom groups (as opposed to ','
@@ -63,6 +64,22 @@ public PharmacophoreQueryAtom(String symbol, String smarts) {
             compiledSmarts[i] = SMARTSParser.parse(subSmarts[i], null);
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public String getSymbol() {
+        return this.symbol;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void setSymbol(String symbol) {
+        this.symbol = symbol;
+    }
+
     /**
      * Get the SMARTS pattern for this pharmacophore group.
      *