methods to manipulate atom types in ReactionManipulator

[uli-f]

• added methods to perceive and unset the atom type configurations in ReactionManipulator
• fixed a javadoc errors wrt content in AtomContainerManipulator#percieveAtomTypesAndConfigureUnsetProperties

[egonw]

Please add test methods for all new methods. And thanx for the extensive documentation!

[uli-f]

Please add test methods for all new methods.

I decided against adding tests as the methods merely combine two (already tested) method calls. There is not logic in those methods other than a for loop.

I am happy to add tests if that is what you prefer (e.g., to achieve good test coverage numbers).
Just let me know and I am on it.

[egonw]

I decided against adding tests as the methods merely combine two (already tested) method calls.

Understood. But... we write unit tests as much as now as for the future. The implementation is now simple, but in the future may not. I found it much more beneficial for maintainability if all assumption are explicit. It fixed a lot of code in the 2003-2008 period, even the silliest unit tests.

[uli-f]

Understood. But... we write unit tests as much as now as for the future. The implementation is now simple, but in the future may not. I found it much more beneficial for maintainability if all assumption are explicit. It fixed a lot of code in the 2003-2008 period, even the silliest unit tests.

Okay, I see your point.
Will add (simple) tests now.

[uli-f]

Added tests now.
Looking forward to feedback 😃

[egonw]

Thanks for adding the tests.

[johnmay]

Looks fine, personally I don't think we use wildcard imports anywhere else mainly for convention. In the restricted case of org.openscience.cdk.interfaces.* I guess it is fine. It would be marginally more efficient to loop the component types rather than use the less efficient getAllMolecules. The fastest toolkits represent reactions as molecules anyways and so technically it's already possible to do it like this:

java
IReaction rxn = ...;
IAtomContainer mol = ReactionManipulator.toMolecule(rxn);
AtomContainerManipulator.percieveAtomTypesAndConfigureAtoms(mol);
rxn = ReactionManipulator.toReaction(mol);

Although that would be pretty silly :-).