From bf40264a472dc2fb7e8c36ff506db47ebbc841db Mon Sep 17 00:00:00 2001 From: Sreejith Nair Date: Tue, 4 Apr 2023 21:38:15 -0300 Subject: [PATCH 1/4] Refactoring : Extract method changes to reduce the cyclomatic complexity of getBackboneAtomArray --- .../nbio/structure/StructureTools.java | 68 ++++++------------- 1 file changed, 21 insertions(+), 47 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/StructureTools.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/StructureTools.java index 3dad14a2a0..51e8b880de 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/StructureTools.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/StructureTools.java @@ -1128,65 +1128,39 @@ public static Atom[] getRepresentativeAtomArray(Structure s) { * @return an Atom[] array */ public static Atom[] getBackboneAtomArray(Structure s) { - List atoms = new ArrayList<>(); + Set nucleotideAtomNames = new HashSet<>(Arrays.asList(C1_ATOM_NAME, C2_ATOM_NAME, C3_ATOM_NAME, C4_ATOM_NAME, O2_ATOM_NAME, O3_ATOM_NAME, O4_ATOM_NAME, O5_ATOM_NAME, OP1_ATOM_NAME, OP2_ATOM_NAME, P_ATOM_NAME)); + Set aminoAcidAtomNames = new HashSet<>(Arrays.asList(CA_ATOM_NAME, C_ATOM_NAME, N_ATOM_NAME, O_ATOM_NAME)); for (Chain c : s.getChains()) { for (Group g : c.getAtomGroups()) { if (g.hasAminoAtoms()) { - // this means we will only take atoms grom groups that have - // complete backbones - for (Atom a : g.getAtoms()) { - switch (g.getType()) { - case NUCLEOTIDE: - // Nucleotide backbone - if (a.getName().equals(C1_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(C2_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(C3_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(C4_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(O2_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(O3_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(O4_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(O5_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(OP1_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(OP2_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(P_ATOM_NAME)) - atoms.add(a); - // TODO Allow C4* names as well as C4'? -SB 3/2015 - break; - case AMINOACID: - default: - // we do it this way instead of with g.getAtom() to - // be sure we always use the same order as original - if (a.getName().equals(CA_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(C_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(N_ATOM_NAME)) - atoms.add(a); - if (a.getName().equals(O_ATOM_NAME)) - atoms.add(a); - break; - } + if (g.getType() == GroupType.NUCLEOTIDE) { + addNucleotideAndAminoAtoms(atoms, g, nucleotideAtomNames); + } else { + addNucleotideAndAminoAtoms(atoms, g, aminoAcidAtomNames); } } } - } - return atoms.toArray(new Atom[0]); } + /** + * This method will be used to add the Nucleotide and Amino atoms to the backbone Atom arrays based on the pre-defined Atom names. + * @param atoms + * @param g + * @param atomNames + */ + private static void addNucleotideAndAminoAtoms(List atoms, Group g, Set atomNames) { + for (Atom a : g.getAtoms()) { + if (atomNames.contains(a.getName())) { + atoms.add(a); + } + } + } + + /** * Convert three character amino acid codes into single character e.g. * convert CYS to C. Valid 3-letter codes will be those of the standard 20 From 2ddf8b09c2b81bb799424a1f7c8cc215750f069a Mon Sep 17 00:00:00 2001 From: Sreejith Nair Date: Wed, 5 Apr 2023 01:04:23 -0300 Subject: [PATCH 2/4] Refactoring Decompose conditional : Changes to refactor getContactOverlapScore method to eliminate complex conditional smell --- .../structure/contact/StructureInterface.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/StructureInterface.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/StructureInterface.java index 44da65e31b..b26275cde8 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/StructureInterface.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/StructureInterface.java @@ -616,10 +616,7 @@ public double getContactOverlapScore(StructureInterface other, boolean invert) { Pair thisCompounds = new Pair(thisChains.getFirst().getEntityInfo(), thisChains.getSecond().getEntityInfo()); Pair otherCompounds = new Pair(otherChains.getFirst().getEntityInfo(), otherChains.getSecond().getEntityInfo()); - if ( ( (thisCompounds.getFirst().getMolId() == otherCompounds.getFirst().getMolId()) && - (thisCompounds.getSecond().getMolId() == otherCompounds.getSecond().getMolId()) ) || - ( (thisCompounds.getFirst().getMolId() == otherCompounds.getSecond().getMolId()) && - (thisCompounds.getSecond().getMolId() == otherCompounds.getFirst().getMolId()) ) ) { + if (checkMolIdMatch(thisCompounds,otherCompounds)) { int common = 0; GroupContactSet thisContacts = getGroupContacts(); @@ -653,6 +650,17 @@ public double getContactOverlapScore(StructureInterface other, boolean invert) { } } + /** + * This method check if two compounds have same MolIds or not. + * @param thisCompounds + * @param otherCompounds + * @return + */ + private boolean checkMolIdMatch(Pair thisCompounds, Pair otherCompounds){ + boolean firstMatch = thisCompounds.getFirst().getMolId() == otherCompounds.getFirst().getMolId() && thisCompounds.getSecond().getMolId() == otherCompounds.getSecond().getMolId(); + boolean secondMatch = thisCompounds.getFirst().getMolId() == otherCompounds.getSecond().getMolId() && thisCompounds.getSecond().getMolId() == otherCompounds.getFirst().getMolId(); + return firstMatch || secondMatch; + } public GroupContactSet getGroupContacts() { if (groupContacts==null) { this.groupContacts = new GroupContactSet(contacts); From 4676fe7c7e28c8004b2739472594533739109c5f Mon Sep 17 00:00:00 2001 From: Sreejith Nair Date: Wed, 5 Apr 2023 01:05:00 -0300 Subject: [PATCH 3/4] Refactoring : Changes to refactor isPureTranslation method to eliminate complex conditional smell --- .../nbio/structure/xtal/CrystalTransform.java | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/xtal/CrystalTransform.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/xtal/CrystalTransform.java index 3926c9223a..06251cbb31 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/xtal/CrystalTransform.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/xtal/CrystalTransform.java @@ -180,26 +180,29 @@ public boolean isIdentity() { */ public boolean isPureTranslation() { if (isPureCrystalTranslation()) return true; - if (SpaceGroup.deltaComp(matTransform.m00,1,SpaceGroup.DELTA) && - SpaceGroup.deltaComp(matTransform.m01,0,SpaceGroup.DELTA) && - SpaceGroup.deltaComp(matTransform.m02,0,SpaceGroup.DELTA) && - - SpaceGroup.deltaComp(matTransform.m10,0,SpaceGroup.DELTA) && - SpaceGroup.deltaComp(matTransform.m11,1,SpaceGroup.DELTA) && - SpaceGroup.deltaComp(matTransform.m12,0,SpaceGroup.DELTA) && - - SpaceGroup.deltaComp(matTransform.m20,0,SpaceGroup.DELTA) && - SpaceGroup.deltaComp(matTransform.m21,0,SpaceGroup.DELTA) && - SpaceGroup.deltaComp(matTransform.m22,1,SpaceGroup.DELTA) && - ( Math.abs(matTransform.m03-0.0)>SpaceGroup.DELTA || - Math.abs(matTransform.m13-0.0)>SpaceGroup.DELTA || - Math.abs(matTransform.m23-0.0)>SpaceGroup.DELTA)) { - return true; - } - + if (isPureMatrixTranslation()) return true; return false; } + /** + * This method will help check if the matrix translation is pure or not. + * @return boolean + */ + private boolean isPureMatrixTranslation(){ + return SpaceGroup.deltaComp(matTransform.m00,1,SpaceGroup.DELTA) && + SpaceGroup.deltaComp(matTransform.m01,0,SpaceGroup.DELTA) && + SpaceGroup.deltaComp(matTransform.m02,0,SpaceGroup.DELTA) && + + SpaceGroup.deltaComp(matTransform.m10,0,SpaceGroup.DELTA) && + SpaceGroup.deltaComp(matTransform.m11,1,SpaceGroup.DELTA) && + SpaceGroup.deltaComp(matTransform.m12,0,SpaceGroup.DELTA) && + + SpaceGroup.deltaComp(matTransform.m20,0,SpaceGroup.DELTA) && + SpaceGroup.deltaComp(matTransform.m21,0,SpaceGroup.DELTA) && + SpaceGroup.deltaComp(matTransform.m22,1,SpaceGroup.DELTA) && + (Math.abs(matTransform.m03-0.0)>SpaceGroup.DELTA || Math.abs(matTransform.m13-0.0)>SpaceGroup.DELTA || Math.abs(matTransform.m23-0.0)>SpaceGroup.DELTA); + } + /** * Tells whether this transformation contains a fractional translational * component (whatever its rotational component). A fractional translation From a135683257d0f709e19255cf44566fb67d12411d Mon Sep 17 00:00:00 2001 From: Sreejith Nair Date: Thu, 6 Apr 2023 02:45:06 -0300 Subject: [PATCH 4/4] Refactoring : Adding Static constants for NUCLEOTIDE_BACKBONE_ATOMS and AMINOACID_BACKBONE_ATOMS --- .../org/biojava/nbio/structure/StructureTools.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/StructureTools.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/StructureTools.java index 51e8b880de..af5f7ab80f 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/StructureTools.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/StructureTools.java @@ -167,6 +167,9 @@ public class StructureTools { private static final Set hBondDonorAcceptors; + private static final Set NUCLEOTIDE_BACKBONE_ATOMS; + private static final Set AMINOACID_BACKBONE_ATOMS; + static { nucleotides30 = new HashMap(); nucleotides30.put("DA", 'A'); @@ -256,6 +259,9 @@ public class StructureTools { hBondDonorAcceptors.add(Element.O); hBondDonorAcceptors.add(Element.S); + NUCLEOTIDE_BACKBONE_ATOMS = new HashSet<>(Arrays.asList(C1_ATOM_NAME, C2_ATOM_NAME, C3_ATOM_NAME, C4_ATOM_NAME, O2_ATOM_NAME, O3_ATOM_NAME, O4_ATOM_NAME, O5_ATOM_NAME, OP1_ATOM_NAME, OP2_ATOM_NAME, P_ATOM_NAME)); + AMINOACID_BACKBONE_ATOMS = new HashSet<>(Arrays.asList(CA_ATOM_NAME, C_ATOM_NAME, N_ATOM_NAME, O_ATOM_NAME)); + } /** @@ -1129,16 +1135,13 @@ public static Atom[] getRepresentativeAtomArray(Structure s) { */ public static Atom[] getBackboneAtomArray(Structure s) { List atoms = new ArrayList<>(); - Set nucleotideAtomNames = new HashSet<>(Arrays.asList(C1_ATOM_NAME, C2_ATOM_NAME, C3_ATOM_NAME, C4_ATOM_NAME, O2_ATOM_NAME, O3_ATOM_NAME, O4_ATOM_NAME, O5_ATOM_NAME, OP1_ATOM_NAME, OP2_ATOM_NAME, P_ATOM_NAME)); - Set aminoAcidAtomNames = new HashSet<>(Arrays.asList(CA_ATOM_NAME, C_ATOM_NAME, N_ATOM_NAME, O_ATOM_NAME)); - for (Chain c : s.getChains()) { for (Group g : c.getAtomGroups()) { if (g.hasAminoAtoms()) { if (g.getType() == GroupType.NUCLEOTIDE) { - addNucleotideAndAminoAtoms(atoms, g, nucleotideAtomNames); + addNucleotideAndAminoAtoms(atoms, g, NUCLEOTIDE_BACKBONE_ATOMS); } else { - addNucleotideAndAminoAtoms(atoms, g, aminoAcidAtomNames); + addNucleotideAndAminoAtoms(atoms, g, AMINOACID_BACKBONE_ATOMS); } } }