When deciding which side to place a double bond also consider if a ring contains a metal.

johnmay · johnmay · commit 473846460b1e · 2019-07-28T13:50:25.000+01:00
diff --git a/display/renderbasic/src/main/java/org/openscience/cdk/renderer/generators/standard/StandardBondGenerator.java b/display/renderbasic/src/main/java/org/openscience/cdk/renderer/generators/standard/StandardBondGenerator.java
@@ -1599,7 +1599,7 @@ static Map<IBond, IAtomContainer> ringPreferenceMap(IAtomContainer container,
 
         final List<IAtomContainer> rings = AtomContainerSetManipulator.getAllAtomContainers(smallest);
 
-        Collections.sort(rings, new RingBondOffsetComparator());
+        Collections.sort(rings, new RingBondOffsetComparator(container));
 
         final Map<IBond, IAtomContainer> ringMap = new HashMap<IBond, IAtomContainer>();
 
@@ -1689,29 +1689,52 @@ static final class RingBondOffsetComparator implements Comparator<IAtomContainer
             }
         }
 
+        private final boolean hasMetal;
+
         /**
          * Create a new comparator.
          */
-        RingBondOffsetComparator() {}
+        RingBondOffsetComparator(IAtomContainer mol) {
+            hasMetal = hasMetal(mol);
+        }
+
+        RingBondOffsetComparator() {
+            hasMetal = false;
+        }
+
+        private static boolean hasMetal(IAtomContainer mol) {
+            for (IAtom atom : mol.atoms())
+                if (Elements.isMetal(atom))
+                    return true;
+            return false;
+        }
 
         /**
          *{@inheritDoc}
          */
         @Override
-        public int compare(IAtomContainer containerA, IAtomContainer containerB) {
+        public int compare(IAtomContainer ringa, IAtomContainer ringb) {
+
+            // non-metal rings (e.g. carbo/hetro cycles first)
+            if (hasMetal) {
+                int cmp = Boolean.compare(hasMetal(ringa),
+                                          hasMetal(ringb));
+                if (cmp != 0)
+                    return cmp;
+            }
 
-            // first order by size
-            int sizeCmp = Ints.compare(sizePreference(containerA.getAtomCount()),
-                    sizePreference(containerB.getAtomCount()));
+            // order by size 6,5,7,4,3,rest
+            int sizeCmp = Ints.compare(sizePreference(ringa.getAtomCount()),
+                    sizePreference(ringb.getAtomCount()));
             if (sizeCmp != 0) return sizeCmp;
 
             // now order by number of double bonds
-            int piBondCmp = Ints.compare(nDoubleBonds(containerA), nDoubleBonds(containerB));
+            int piBondCmp = Ints.compare(nDoubleBonds(ringa), nDoubleBonds(ringb));
             if (piBondCmp != 0) return -piBondCmp;
 
             // order by element frequencies, all carbon rings are preferred
-            int[] freqA = countLightElements(containerA);
-            int[] freqB = countLightElements(containerB);
+            int[] freqA = countLightElements(ringa);
+            int[] freqB = countLightElements(ringb);
 
             for (Elements element : Arrays.asList(Elements.Carbon, Elements.Nitrogen, Elements.Oxygen, Elements.Sulfur,
                     Elements.Phosphorus)) {
diff --git a/display/renderbasic/src/test/java/org/openscience/cdk/renderer/generators/standard/StandardBondGeneratorTest.java b/display/renderbasic/src/test/java/org/openscience/cdk/renderer/generators/standard/StandardBondGeneratorTest.java
@@ -25,10 +25,14 @@
 package org.openscience.cdk.renderer.generators.standard;
 
 import org.junit.Test;
+import org.openscience.cdk.interfaces.IAtom;
 import org.openscience.cdk.interfaces.IAtomContainer;
 import org.openscience.cdk.interfaces.IBond;
+import org.openscience.cdk.silent.SilentChemObjectBuilder;
+import org.openscience.cdk.smiles.SmilesParser;
 import org.openscience.cdk.templates.TestMoleculeFactory;
 
+import javax.vecmath.Point2d;
 import java.util.Map;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -59,6 +63,31 @@ public void adenineRingPreference() throws Exception {
         assertThat(nSize6, is(6));
     }
 
+    @Test
+    public void metalRingPreference() throws Exception {
+
+        SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance());
+        IAtomContainer mol = smipar.parseSmiles("C1[Fe]C=CC2=C1C=CN2");
+        for (IAtom atom : mol.atoms())
+            atom.setPoint2d(new Point2d(0,0));
+        Map<IBond, IAtomContainer> ringMap = StandardBondGenerator.ringPreferenceMap(mol);
+
+        int nSize5 = 0, nSize6 = 0;
+        for (IBond bond : mol.bonds()) {
+            IAtomContainer ring = ringMap.get(bond);
+            // exocyclic bond
+            if (ring == null) continue;
+            int size = ring.getAtomCount();
+            if (size == 5) nSize5++;
+            if (size == 6) nSize6++;
+        }
+
+        // 5 bonds should point to the six member ring
+        // 5 bonds should point to the five member ring
+        assertThat(nSize5, is(5));
+        assertThat(nSize6, is(5));
+    }
+
     @Test
     public void ringSizePriority() {
         assertThat(RingBondOffsetComparator.sizePreference(6), is(0));
