Hi all,
a recently reported bug involves code added to the CDK library over 3 years ago:
IChemObject instances that are properties of the IChemObject are
cloned for the cloned IChemObject. Example:
IAtomContainer container1 = builder.newAtomContainer();
IAtom atom1 = builder.newAtom();
container1.addAtom(atom1);
atom1.setProperty("myParent", atomcontainer1);
A situation like this is used, and not that obscure.
Now, the bug report is a StackOverflow when clone() is called on this
object. Which is obvious.
Indeed, there is a JUnit test which looks like:
IChemObject chemObject1 = builder.newChemObject();
Hashtable props1 = new Hashtable();
IAtom atom = builder.newAtom("C");
props1.put("atom", atom);
chemObject1.setProperties(props1);
IChemObject chemObject2 = (IChemObject)chemObject1.clone();
// test cloning of properties field
Hashtable props2 = new Hashtable();
chemObject2.setProperties(props2);
Assert.assertEquals(props1, chemObject1.getProperties());
Assert.assertEquals(1, chemObject2.getProperties().size());
Assert.assertEquals(1, chemObject1.getProperties().size());
// ok, copied hashtable item, but this item should be cloned
Assert.assertNotSame(atom, chemObject2.getProperties().get("atom"));
The last line requires the "atom" property of the cloned atom not to
be the same instance as the original, that is a clone.
So, there needs to be a API change issued to solve this bug. I have
run all JUnit tests for the library, and the not cloning
of properties does not give any failing unit tests other than the one
just discussed.
However, the user community should indicate what the expected behavior
of clone() with respect to IChemObject type properties should be.
I cannot remember why I (commit rev 3087) added the cloning of
IChemObjects, and would actually propose to not clone these
properties.
Alternatively, IChemObject type properties can be dropped... one could
question of the usefulness of cloning in "myParent" property use
cases.
So, there at least three options, and we need to decide which one the
CDK should adopt:
1. clone IChemObject type properties, using some cycle detection and max depth
2. not clone IChemObject type properties
3. drop any IChemObject type properties
Comments, please.
Egon
--
----
http://chem-bla-ics.blogspot.com/
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Cdk-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/cdk-user
