Accessing self-bond should be null.

cdk · May 3, 2018 · 93a1ad6 · 93a1ad6
1 parent d066abe
commit 93a1ad6
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 4 deletions.
diff --git a/base/data/src/main/java/org/openscience/cdk/AtomContainer2.java b/base/data/src/main/java/org/openscience/cdk/AtomContainer2.java
@@ -1656,8 +1656,7 @@ public final Iterable<IBond> bonds() {
         @Override
         public IBond getBond(IAtom atom) {
             for (IBond bond : bonds) {
-                if (bond.getBegin().equals(atom) ||
-                    bond.getEnd().equals(atom))
+                if (bond.getOther(this).equals(atom))
                     return bond;
             }
             return null;

diff --git a/base/silent/src/main/java/org/openscience/cdk/silent/AtomContainer2.java b/base/silent/src/main/java/org/openscience/cdk/silent/AtomContainer2.java
@@ -1618,8 +1618,7 @@ public final Iterable<IBond> bonds() {
         @Override
         public IBond getBond(IAtom atom) {
             for (IBond bond : bonds) {
-                if (bond.getBegin().equals(atom) ||
-                    bond.getEnd().equals(atom))
+                if (bond.getOther(this).equals(atom))
                     return bond;
             }
             return null;

diff --git a/...st-interfaces/src/test/java/org/openscience/cdk/interfaces/AbstractAtomContainerTest.java b/...st-interfaces/src/test/java/org/openscience/cdk/interfaces/AbstractAtomContainerTest.java
@@ -40,6 +40,7 @@
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.CoreMatchers.sameInstance;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -3200,4 +3201,24 @@ public void changeConnectedAtomsAfterAddBond() {
         assertTrue(clonedSgroup.getBonds().contains(clone.getBond(0)));
         assertTrue(clonedSgroup.getBonds().contains(clone.getBond(1)));
     }
+
+    @Test
+    public void getSelfBond() {
+        IAtomContainer mol = (IAtomContainer) newChemObject();
+        IAtom          a1  = mol.getBuilder().newAtom();
+        IAtom          a2  = mol.getBuilder().newAtom();
+        IAtom          a3  = mol.getBuilder().newAtom();
+        IBond          b1  = mol.getBuilder().newBond();
+        IBond          b2  = mol.getBuilder().newBond();
+        b1.setAtom(a1, 0);
+        b1.setAtom(a2, 1);
+        b2.setAtom(a2, 0);
+        b2.setAtom(a3, 1);
+        mol.addAtom(a1);
+        mol.addAtom(a2);
+        mol.addAtom(a3);
+        mol.addBond(b1);
+        mol.addBond(b2);
+        assertThat(mol.getBond(a1, a1), is(nullValue()));
+    }
 }