HI Dmitry,

On Tue, May 31, 2011 at 6:02 PM, Dmitry Katsubo
<dmitry.kats...@gmail.com> wrote:
> Currently InChIGeneratorFactory.getInChIToStructure() generates
> IAtomContainer from InChI. When it needs to be passed to
> StructureDiagramGenerator, a new Molecule needs to be created on the
> basis of this container:

Yes, the SDG does not handle disconnected structures.

> final StructureDiagramGenerator sdg = new StructureDiagramGenerator();
>
> sdg.setMolecule(new Molecule(InChIGeneratorFactory.getInstance()
> .getInChIToStructure(inchi, DefaultChemObjectBuilder.getInstance())
> .getAtomContainer()));

The proper way of doing is, is to check first of the IAtomContainer
you got from the getInChIToStructure() method is connected (using the
ConnectivityCehcker) and to partition is if not.

> Does it make sense for overall CDK API to have a preference for either
> IAtomContainer or IMolecule (which are basically the same objects by
> implementation), so that different modules can be bind more smoothly?

I do not understand this question. And IAtomContainer and IMolecule to
be the same by implementation is just because we do not want the
IMolecule to check if an edit results in a disconnected structure
(computationally expensive). But the semantics are there.

> And second is that getInChIToStructure() needs to be passed a
> IChemObjectBuilder instance, which most cases is a CDK default one.

Yeah, I'm wondering why, because the nonotify classes are in fact faster.
I guess performance is not so important after all :)

> Perhaps the API should include the set of methods without
> IChemObjectBuilder argument.

Yes, I have been thinking about that. We could instantiate a
ChemObjectBuilderFactory, which will try to load one of the two main
implementations. This will most certainly not work for all situations
(e.g. not in Bioclipse or any other place with custom class loaders,
unless we start making the Factory instantiated with that custom
ClassLoader...)...

Something like:

/**
 * @cdk.module core
 */
public class ChemObjectFactory {

  public abstract IChemObjectBuilder getBuilder(ClassLoader loader)
  throws ClassNotFoundException, InstantiationException {
    String[] knownBuilders = new String[
      "org.openscience.cdk.nonotify.NoNotificationChemObjectBuilder",
      "org.openscience.cdk.nonotify.DefaultChemObjectBuilder"
    ];
    for (int i=0; i<knownBuilders.length; i++) {
      IChemObjectBuilder builder =
loader.loadClass(knownBuilders[i]).getInstance();
      return builder;
    }
  }
  return new SomeAppropriateException();
}

Finish the patch (full JavaDoc, unit testsing, etc, etc), two people
to approve of the patch, and I'll apply it to cdk-1.4.x.

Egon

-- 
Dr E.L. Willighagen
Postdoctoral Researcher
Institutet för miljömedicin
Karolinska Institutet (http://ki.se/imm)
Homepage: http://egonw.github.com/
LinkedIn: http://se.linkedin.com/in/egonw
Blog: http://chem-bla-ics.blogspot.com/
PubList: http://www.citeulike.org/user/egonw/tag/papers

------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger. 
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today. 
http://p.sf.net/sfu/quest-sfdev2dev
_______________________________________________
Cdk-user mailing list
Cdk-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/cdk-user

Reply via email to