Skip to content

Commit 46ffbb4

Browse files
johnmayegonw
johnmay
authored and
egonw
committed
Resolved NoNotify fails on AtomParity. Error was due to subclassing of AtomParity. Also the assertEquals params were swapped as the assertion was the wrong way.
Change-Id: Idebf543ef01e1f81f9bdb1264a1dea0088c8d752 Signed-off-by: Egon Willighagen <egonw@users.sourceforge.net>
1 parent 938306a commit 46ffbb4

3 files changed

Lines changed: 177 additions & 3 deletions

File tree

‎src/main/org/openscience/cdk/nonotify/NNAtomParity.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@
2929
package org.openscience.cdk.nonotify;
3030

3131
import org.openscience.cdk.AtomParity;
32+
import org.openscience.cdk.annotations.TestMethod;
3233
import org.openscience.cdk.interfaces.IAtom;
34+
import org.openscience.cdk.interfaces.IAtomParity;
35+
import org.openscience.cdk.interfaces.IBond;
36+
37+
import java.util.Map;
3338

3439
/**
3540
* @cdk.module nonotify
@@ -49,6 +54,31 @@ public NNAtomParity(
4954
int parity) {
5055
super(centralAtom, first, second, third, fourth, parity);
5156
}
57+
58+
/**
59+
* @inheritDoc
60+
*/
61+
@TestMethod("testMap_Map_Map,testMap_Null_Map,testMap_Map_Map_NullElement,testMap_Map_Map_EmptyMapping")
62+
@Override
63+
public IAtomParity map(Map<IAtom, IAtom> atoms, Map<IBond, IBond> bonds) {
64+
65+
if(atoms == null) // not using bond mapping
66+
throw new IllegalArgumentException("null atom mapping provided");
67+
68+
IAtom[] neighbors = getSurroundingAtoms();
69+
70+
// could map neighbours with a for loop but we need to pull individuals
71+
// atoms for the constructor
72+
return new NNAtomParity(
73+
getAtom() != null ? atoms.get(getAtom()) : null,
74+
neighbors[0] != null ? atoms.get(neighbors[0]) : null,
75+
neighbors[1] != null ? atoms.get(neighbors[1]) : null,
76+
neighbors[2] != null ? atoms.get(neighbors[2]) : null,
77+
neighbors[3] != null ? atoms.get(neighbors[3]) : null,
78+
getParity()
79+
);
80+
81+
}
5282

5383
}
5484

‎src/test/org/openscience/cdk/interfaces/AbstractAtomContainerTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ public abstract class AbstractAtomContainerTest extends AbstractChemObjectTest {
404404

405405
IStereoElement element = elements.next();
406406

407-
Assert.assertEquals("cloned element was incorrect class", element.getClass(), chirality.getClass());
407+
Assert.assertEquals("cloned element was incorrect class", chirality.getClass(), element.getClass());
408408
assertThat("too many stereo elements", elements.hasNext(), is(not(true)));
409409

410410
// we've tested the class already - cast is okay
@@ -472,7 +472,7 @@ public abstract class AbstractAtomContainerTest extends AbstractChemObjectTest {
472472

473473
IStereoElement element = elements.next();
474474

475-
Assert.assertEquals("cloned element was incorrect class", element.getClass(), dbStereo.getClass());
475+
Assert.assertEquals("cloned element was incorrect class", dbStereo.getClass(), element.getClass());
476476
assertThat("too many stereo elements", elements.hasNext(), is(not(true)));
477477

478478
// we've tested the class already - cast is okay
@@ -546,7 +546,7 @@ public abstract class AbstractAtomContainerTest extends AbstractChemObjectTest {
546546

547547
IStereoElement element = elements.next();
548548

549-
Assert.assertEquals("cloned element was incorrect class", element.getClass(), chirality.getClass());
549+
Assert.assertEquals("cloned element was incorrect class", chirality.getClass(), element.getClass());
550550
assertThat("too many stereo elements", elements.hasNext(), is(not(true)));
551551

552552
// we've tested the class already - cast is okay

‎src/test/org/openscience/cdk/nonotify/NNAtomParityTest.java‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@
3131
import org.openscience.cdk.interfaces.AbstractAtomParityTest;
3232
import org.openscience.cdk.interfaces.IAtom;
3333
import org.openscience.cdk.interfaces.IAtomParity;
34+
import org.openscience.cdk.interfaces.IChemObjectBuilder;
35+
36+
import java.util.Collections;
37+
import java.util.HashMap;
38+
import java.util.Map;
39+
40+
import static org.hamcrest.CoreMatchers.is;
41+
import static org.hamcrest.CoreMatchers.not;
42+
import static org.hamcrest.CoreMatchers.sameInstance;
3443

3544
/**
3645
* Checks the functionality of the {@link NNAtomParity}.
@@ -74,4 +83,139 @@ public class NNAtomParityTest extends AbstractAtomParityTest {
7483
Assert.assertNotNull(parity);
7584
}
7685

86+
87+
@Test public void testMap_Map_Map() throws CloneNotSupportedException {
88+
89+
IChemObjectBuilder builder = NoNotificationChemObjectBuilder.getInstance();
90+
91+
IAtom c1 = builder.newInstance(IAtom.class, "C");
92+
IAtom o2 = builder.newInstance(IAtom.class, "O");
93+
IAtom n3 = builder.newInstance(IAtom.class, "N");
94+
IAtom c4 = builder.newInstance(IAtom.class, "C");
95+
IAtom h5 = builder.newInstance(IAtom.class, "H");
96+
97+
// new stereo element
98+
IAtomParity original = new AtomParity(c1,
99+
o2,n3,c4,h5,
100+
2);
101+
102+
// clone the atoms and place in a map
103+
Map<IAtom,IAtom> mapping = new HashMap<IAtom,IAtom>();
104+
IAtom c1clone = (IAtom) c1.clone(); mapping.put(c1, c1clone);
105+
IAtom o2clone = (IAtom) o2.clone(); mapping.put(o2, o2clone);
106+
IAtom n3clone = (IAtom) n3.clone(); mapping.put(n3, n3clone);
107+
IAtom c4clone = (IAtom) c4.clone(); mapping.put(c4, c4clone);
108+
IAtom h5clone = (IAtom) h5.clone(); mapping.put(h5, h5clone);
109+
110+
// map the existing element a new element
111+
IAtomParity mapped = original.map(mapping, Collections.EMPTY_MAP);
112+
113+
Assert.assertThat("mapped chiral atom was the same as the original",
114+
mapped.getAtom(), is(not(sameInstance(original.getAtom()))));
115+
Assert.assertThat("mapped chiral atom was not the clone",
116+
mapped.getAtom(), is(sameInstance(c1clone)));
117+
118+
IAtom[] originalLigands = original.getSurroundingAtoms();
119+
IAtom[] mappedLigands = mapped.getSurroundingAtoms();
120+
121+
Assert.assertThat("first ligand was te same as the original",
122+
mappedLigands[0], is(not(sameInstance(originalLigands[0]))));
123+
Assert.assertThat("first mapped ligand was not the clone",
124+
mappedLigands[0], is(sameInstance(o2clone)));
125+
Assert.assertThat("second ligand was te same as the original",
126+
mappedLigands[1], is(not(sameInstance(originalLigands[1]))));
127+
Assert.assertThat("second mapped ligand was not the clone",
128+
mappedLigands[1], is(sameInstance(n3clone)));
129+
Assert.assertThat("third ligand was te same as the original",
130+
mappedLigands[2], is(not(sameInstance(originalLigands[2]))));
131+
Assert.assertThat("third mapped ligand was not the clone",
132+
mappedLigands[2], is(sameInstance(c4clone)));
133+
Assert.assertThat("forth ligand was te same as the original",
134+
mappedLigands[3], is(not(sameInstance(originalLigands[3]))));
135+
Assert.assertThat("forth mapped ligand was not the clone",
136+
mappedLigands[3], is(sameInstance(h5clone)));
137+
138+
Assert.assertThat("stereo was not mapped",
139+
mapped.getParity(), is(original.getParity()));
140+
141+
}
142+
143+
@Test(expected = IllegalArgumentException.class)
144+
public void testMap_Null_Map() throws CloneNotSupportedException {
145+
146+
IChemObjectBuilder builder = NoNotificationChemObjectBuilder.getInstance();
147+
148+
IAtom c1 = builder.newInstance(IAtom.class, "C");
149+
IAtom o2 = builder.newInstance(IAtom.class, "O");
150+
IAtom n3 = builder.newInstance(IAtom.class, "N");
151+
IAtom c4 = builder.newInstance(IAtom.class, "C");
152+
IAtom h5 = builder.newInstance(IAtom.class, "H");
153+
154+
// new stereo element
155+
IAtomParity original = new AtomParity(c1,
156+
o2,n3,c4,h5,
157+
2);
158+
159+
160+
// map the existing element a new element - should through an IllegalArgumentException
161+
IAtomParity mapped = original.map(null, Collections.EMPTY_MAP);
162+
163+
}
164+
165+
@Test
166+
public void testMap_Map_Map_NullElement() throws CloneNotSupportedException {
167+
168+
IChemObjectBuilder builder = NoNotificationChemObjectBuilder.getInstance();
169+
170+
IAtom c1 = builder.newInstance(IAtom.class, "C");
171+
IAtom o2 = builder.newInstance(IAtom.class, "O");
172+
IAtom n3 = builder.newInstance(IAtom.class, "N");
173+
IAtom c4 = builder.newInstance(IAtom.class, "C");
174+
IAtom h5 = builder.newInstance(IAtom.class, "H");
175+
176+
// new stereo element
177+
IAtomParity original = new NNAtomParity(null,
178+
null, null, null, null,
179+
0);
180+
181+
182+
// map the existing element a new element
183+
IAtomParity mapped = original.map(Collections.EMPTY_MAP, Collections.EMPTY_MAP);
184+
185+
Assert.assertNull(mapped.getAtom());
186+
Assert.assertNull(mapped.getSurroundingAtoms()[0]);
187+
Assert.assertNull(mapped.getSurroundingAtoms()[1]);
188+
Assert.assertNull(mapped.getSurroundingAtoms()[2]);
189+
Assert.assertNull(mapped.getSurroundingAtoms()[3]);
190+
191+
}
192+
193+
@Test
194+
public void testMap_Map_Map_EmptyMapping() throws CloneNotSupportedException {
195+
196+
IChemObjectBuilder builder = NoNotificationChemObjectBuilder.getInstance();
197+
198+
IAtom c1 = builder.newInstance(IAtom.class, "C");
199+
IAtom o2 = builder.newInstance(IAtom.class, "O");
200+
IAtom n3 = builder.newInstance(IAtom.class, "N");
201+
IAtom c4 = builder.newInstance(IAtom.class, "C");
202+
IAtom h5 = builder.newInstance(IAtom.class, "H");
203+
204+
// new stereo element
205+
IAtomParity original = new NNAtomParity(c1,
206+
o2,n3,c4,h5,
207+
2);
208+
209+
210+
// map the existing element a new element - should through an IllegalArgumentException
211+
IAtomParity mapped = original.map(Collections.EMPTY_MAP, Collections.EMPTY_MAP);
212+
213+
Assert.assertNull(mapped.getAtom());
214+
Assert.assertNull(mapped.getSurroundingAtoms()[0]);
215+
Assert.assertNull(mapped.getSurroundingAtoms()[1]);
216+
Assert.assertNull(mapped.getSurroundingAtoms()[2]);
217+
Assert.assertNull(mapped.getSurroundingAtoms()[3]);
218+
Assert.assertNotNull(mapped.getParity());
219+
220+
}
77221
}

0 commit comments

Comments
 (0)