diff --git a/Code/Demos/RDKit/GettingStarted/sample.cpp b/Code/Demos/RDKit/GettingStarted/sample.cpp index 87ccd3e7e..aaa6e888d 100644 --- a/Code/Demos/RDKit/GettingStarted/sample.cpp +++ b/Code/Demos/RDKit/GettingStarted/sample.cpp @@ -32,16 +32,18 @@ using namespace RDKit; void BuildSimpleMolecule() { // build the molecule: C/C=C\C - RWMol *mol = new RWMol(); + auto mol = std::make_unique(); // add atoms and bonds: - mol->addAtom(new Atom(6)); // atom 0 - mol->addAtom(new Atom(6)); // atom 1 - mol->addAtom(new Atom(6)); // atom 2 - mol->addAtom(new Atom(6)); // atom 3 - mol->addBond(0, 1, Bond::SINGLE); // bond 0 - mol->addBond(1, 2, Bond::DOUBLE); // bond 1 - mol->addBond(2, 3, Bond::SINGLE); // bond 2 + constexpr bool updateLabel = false; + constexpr bool takeOwnership = true; + mol->addAtom(new Atom(6), updateLabel, takeOwnership); // atom 0 + mol->addAtom(new Atom(6), updateLabel, takeOwnership); // atom 1 + mol->addAtom(new Atom(6), updateLabel, takeOwnership); // atom 2 + mol->addAtom(new Atom(6), updateLabel, takeOwnership); // atom 3 + mol->addBond(0, 1, Bond::SINGLE); // bond 0 + mol->addBond(1, 2, Bond::DOUBLE); // bond 1 + mol->addBond(2, 3, Bond::SINGLE); // bond 2 // setup the stereochem: mol->getBondWithIdx(0)->setBondDir(Bond::ENDUPRIGHT); mol->getBondWithIdx(2)->setBondDir(Bond::ENDDOWNRIGHT); @@ -51,7 +53,7 @@ void BuildSimpleMolecule() { // Get the canonical SMILES, include stereochemistry: std::string smiles; - smiles = MolToSmiles(*(static_cast(mol)), true); + smiles = MolToSmiles(*mol, true); BOOST_LOG(rdInfoLog) << " sample 1 SMILES: " << smiles << std::endl; } @@ -107,11 +109,9 @@ void WorkWithRingInfo() { // can be played, but we won't // count the number of rings of size 5: - unsigned int nRingsSize5 = 0; - for (VECT_INT_VECT_CI ringIt = atomRings.begin(); ringIt != atomRings.end(); - ++ringIt) { - if (ringIt->size() == 5) nRingsSize5++; - } + auto nRingsSize5 = + std::count_if(atomRings.begin(), atomRings.end(), + [](const INT_VECT &ring) { return ring.size() == 5; }); TEST_ASSERT(nRingsSize5 == 1); delete mol; @@ -122,15 +122,13 @@ void WorkWithRingInfo() { atomRings = ringInfo->atomRings(); unsigned int nMatchingAtoms = 0; - for (VECT_INT_VECT_CI ringIt = atomRings.begin(); ringIt != atomRings.end(); - ++ringIt) { - if (ringIt->size() != 5) { + for (const auto &ring : atomRings) { + if (ring.size() != 5) { continue; } bool isAromatic = true; - for (INT_VECT_CI atomIt = ringIt->begin(); atomIt != ringIt->end(); - ++atomIt) { - if (!mol->getAtomWithIdx(*atomIt)->getIsAromatic()) { + for (auto atom : ring) { + if (!mol->getAtomWithIdx(atom)->getIsAromatic()) { isAromatic = false; break; } @@ -149,17 +147,17 @@ void WorkWithRingInfo() { bondRings = ringInfo->bondRings(); unsigned int nAromaticRings = 0; - for (VECT_INT_VECT_CI ringIt = bondRings.begin(); ringIt != bondRings.end(); - ++ringIt) { + for (const auto &ring : bondRings) { bool isAromatic = true; - for (INT_VECT_CI bondIt = ringIt->begin(); bondIt != ringIt->end(); - ++bondIt) { - if (!mol->getBondWithIdx(*bondIt)->getIsAromatic()) { + for (auto bond : ring) { + if (!mol->getBondWithIdx(bond)->getIsAromatic()) { isAromatic = false; break; } } - if (isAromatic) nAromaticRings++; + if (isAromatic) { + ++nAromaticRings; + } } TEST_ASSERT(nAromaticRings == 1); delete mol; @@ -207,21 +205,23 @@ void CleanupMolecule() { // calling the sanitizeMol function() // build: C1CC1C(:O):O - RWMol *mol = new RWMol(); + auto mol = std::make_unique(); // add atoms and bonds: - mol->addAtom(new Atom(6)); // atom 0 - mol->addAtom(new Atom(6)); // atom 1 - mol->addAtom(new Atom(6)); // atom 2 - mol->addAtom(new Atom(6)); // atom 3 - mol->addAtom(new Atom(8)); // atom 4 - mol->addAtom(new Atom(8)); // atom 5 - mol->addBond(3, 4, Bond::AROMATIC); // bond 0 - mol->addBond(3, 5, Bond::AROMATIC); // bond 1 - mol->addBond(3, 2, Bond::SINGLE); // bond 2 - mol->addBond(2, 1, Bond::SINGLE); // bond 3 - mol->addBond(1, 0, Bond::SINGLE); // bond 4 - mol->addBond(0, 2, Bond::SINGLE); // bond 5 + constexpr bool updateLabel = false; + constexpr bool takeOwnership = true; + mol->addAtom(new Atom(6), updateLabel, takeOwnership); // atom 0 + mol->addAtom(new Atom(6), updateLabel, takeOwnership); // atom 1 + mol->addAtom(new Atom(6), updateLabel, takeOwnership); // atom 2 + mol->addAtom(new Atom(6), updateLabel, takeOwnership); // atom 3 + mol->addAtom(new Atom(8), updateLabel, takeOwnership); // atom 4 + mol->addAtom(new Atom(8), updateLabel, takeOwnership); // atom 5 + mol->addBond(3, 4, Bond::AROMATIC); // bond 0 + mol->addBond(3, 5, Bond::AROMATIC); // bond 1 + mol->addBond(3, 2, Bond::SINGLE); // bond 2 + mol->addBond(2, 1, Bond::SINGLE); // bond 3 + mol->addBond(1, 0, Bond::SINGLE); // bond 4 + mol->addBond(0, 2, Bond::SINGLE); // bond 5 // instead of calling sanitize mol, which would generate an error, // we'll perceive the rings, then take care of aromatic bonds @@ -249,7 +249,7 @@ void CleanupMolecule() { // Get the canonical SMILES, include stereochemistry: std::string smiles; - smiles = MolToSmiles(*(static_cast(mol)), true); + smiles = MolToSmiles(*mol, true); BOOST_LOG(rdInfoLog) << " fixed SMILES: " << smiles << std::endl; } diff --git a/Code/GraphMol/Basement/FeatTrees/FeatTreeUtils.cpp b/Code/GraphMol/Basement/FeatTrees/FeatTreeUtils.cpp index 3280dda18..5a5bbe27c 100644 --- a/Code/GraphMol/Basement/FeatTrees/FeatTreeUtils.cpp +++ b/Code/GraphMol/Basement/FeatTrees/FeatTreeUtils.cpp @@ -125,10 +125,9 @@ void mergeRingCycle(FeatTreeGraph &featGraph, FeatTreeGraph &featGraphCopy, void addRingsAndConnectors(const ROMol &mol, FeatTreeGraph &resGraph) { RingInfo *ringInfo = mol.getRingInfo(); unsigned int ringIdxI = 0; - for (VECT_INT_VECT::const_iterator ringItI = ringInfo->atomRings().begin(); - ringItI != ringInfo->atomRings().end(); ++ringItI, ++ringIdxI) { - UINT_SET s; - s.insert(ringItI->begin(), ringItI->end()); + for (auto ringItI = ringInfo->atomRings().cbegin(); + ringItI != ringInfo->atomRings().cend(); ++ringItI, ++ringIdxI) { + UINT_SET s(ringItI->begin(), ringItI->end()); boost::add_vertex(FeatTreeNode(s), resGraph); // ------ ------ ------ ------ @@ -139,10 +138,10 @@ void addRingsAndConnectors(const ROMol &mol, FeatTreeGraph &resGraph) { INT_VECT ringI = *ringItI; std::sort(ringI.begin(), ringI.end()); unsigned int ringIdxJ = 0; - for (VECT_INT_VECT::const_iterator ringItJ = ringInfo->atomRings().begin(); - ringItJ != ringItI; ++ringItJ, ++ringIdxJ) { - for (INT_VECT::const_iterator ringJElem = ringItJ->begin(); - ringJElem != ringItJ->end(); ++ringJElem) { + for (auto ringItJ = ringInfo->atomRings().cbegin(); ringItJ != ringItI; + ++ringItJ, ++ringIdxJ) { + for (auto ringJElem = ringItJ->cbegin(); ringJElem != ringItJ->cend(); + ++ringJElem) { if (std::binary_search(ringI.begin(), ringI.end(), *ringJElem)) { // these two rings share a common atom, so set up an // edge between them in the feature tree: diff --git a/Code/GraphMol/ChemReactions/ReactionRunner.cpp b/Code/GraphMol/ChemReactions/ReactionRunner.cpp index 387492fbb..055b67c63 100644 --- a/Code/GraphMol/ChemReactions/ReactionRunner.cpp +++ b/Code/GraphMol/ChemReactions/ReactionRunner.cpp @@ -820,7 +820,7 @@ void checkProductChirality(Atom::ChiralType reactantChirality, void setReactantAtomPropertiesToProduct(Atom *productAtom, const Atom &reactantAtom, bool setImplicitProperties, - unsigned int reactantId) { + unsigned int reactantId) { // which properties need to be set from the reactant? if (productAtom->getAtomicNum() <= 0 || productAtom->hasProp(common_properties::_MolFileAtomQuery)) { @@ -849,7 +849,7 @@ void setReactantAtomPropertiesToProduct(Atom *productAtom, productAtom->setProp(common_properties::reactantAtomIdx, reactantAtom.getIdx()); productAtom->setProp(common_properties::reactantIdx, - reactantId); + reactantId); if (setImplicitProperties) { updateImplicitAtomProperties(productAtom, &reactantAtom); } @@ -879,7 +879,7 @@ Bond *addBondToProduct(const Bond &origB, RWMol &product, auto idx = product.addBond(begAtomIdx, endAtomIdx, origB.getBondType()); return product.getBondWithIdx(idx - 1); } else { - QueryBond *qbond = new QueryBond(origB.getBondType()); + auto *qbond = new QueryBond(origB.getBondType()); qbond->setBeginAtomIdx(begAtomIdx); qbond->setEndAtomIdx(endAtomIdx); qbond->setQuery(origB.getQuery()->copy()); @@ -906,7 +906,8 @@ void addMissingProductBonds(const Bond &origB, RWMOL_SPTR product, void addMissingProductAtom(const Atom &reactAtom, unsigned reactNeighborIdx, unsigned prodNeighborIdx, RWMOL_SPTR product, const ROMol &reactant, - ReactantProductAtomMapping *mapping, unsigned int reactantId) { + ReactantProductAtomMapping *mapping, + unsigned int reactantId) { Atom *newAtom = nullptr; if (!reactAtom.hasQuery()) { newAtom = new Atom(reactAtom); @@ -916,8 +917,7 @@ void addMissingProductAtom(const Atom &reactAtom, unsigned reactNeighborIdx, unsigned reactAtomIdx = reactAtom.getIdx(); newAtom->setProp(common_properties::reactantAtomIdx, reactAtomIdx); - newAtom->setProp(common_properties::reactantIdx, - reactantId); + newAtom->setProp(common_properties::reactantIdx, reactantId); unsigned productIdx = product->addAtom(newAtom, false, true); mapping->reactProdAtomMap[reactAtomIdx].push_back(productIdx); mapping->prodReactAtomMap[productIdx] = reactAtomIdx; @@ -1158,7 +1158,7 @@ void checkAndCorrectChiralityOfMatchingAtomsInProduct( if (reactantAtom.getDegree() > productAtom->getDegree()) { // we lost a bond from the reactant. // we can just remove the unmatched reactant bond from the list - INT_LIST::iterator rOrderIter = rOrder.begin(); + auto rOrderIter = rOrder.begin(); while (rOrderIter != rOrder.end() && rOrder.size() > pOrder.size()) { // we may invalidate the iterator so keep track of what comes next: auto thisOne = rOrderIter++; @@ -1307,17 +1307,14 @@ void generateProductConformers(Conformer *productConf, const ROMol &reactant, if (reactConf.is3D()) { productConf->set3D(true); } - for (std::map>::const_iterator pr = - mapping->reactProdAtomMap.begin(); - pr != mapping->reactProdAtomMap.end(); ++pr) { - std::vector prodIdxs = pr->second; + for (const auto &[pr, prodIdxs] : mapping->reactProdAtomMap) { if (prodIdxs.size() > 1) { BOOST_LOG(rdWarningLog) << "reactant atom match more than one product " "atom, coordinates need to be revised\n"; } // is this reliable when multiple product atom mapping occurs???? for (unsigned int prodIdx : prodIdxs) { - productConf->setAtomPos(prodIdx, reactConf.getAtomPos(pr->first)); + productConf->setAtomPos(prodIdx, reactConf.getAtomPos(pr)); } } } @@ -1326,8 +1323,7 @@ void addReactantAtomsAndBonds(const ChemicalReaction &rxn, RWMOL_SPTR product, const ROMOL_SPTR reactantSptr, const MatchVectType &match, const ROMOL_SPTR reactantTemplate, - Conformer *productConf, - unsigned int reactantId) { + Conformer *productConf, unsigned int reactantId) { // start by looping over all matches and marking the reactant atoms that // have already been "added" by virtue of being in the product. We'll also // mark "skipped" atoms: those that are in the match, but not in this @@ -1367,7 +1363,8 @@ void addReactantAtomsAndBonds(const ChemicalReaction &rxn, RWMOL_SPTR product, unsigned productAtomIdx = mapping->reactProdAtomMap[reactantAtomIdx][i]; Atom *productAtom = product->getAtomWithIdx(productAtomIdx); setReactantAtomPropertiesToProduct(productAtom, *reactantAtom, - rxn.getImplicitPropertiesFlag(), reactantId); + rxn.getImplicitPropertiesFlag(), + reactantId); if (reactantAtom->hasQuery()) { // finally: if the reactant atom is a query we should copy over the // query information. We need to replace the atom to do this @@ -1379,7 +1376,8 @@ void addReactantAtomsAndBonds(const ChemicalReaction &rxn, RWMOL_SPTR product, } // now traverse: addReactantNeighborsToProduct(*reactant, *reactantAtom, product, - visitedAtoms, chiralAtomsToCheck, mapping, reactantId); + visitedAtoms, chiralAtomsToCheck, mapping, + reactantId); // now that we've added all the reactant's neighbors, check to see if // it is chiral in the reactant but is not in the reaction. If so @@ -1529,7 +1527,8 @@ generateOneProductSet(const ChemicalReaction &rxn, for (auto iter = rxn.beginReactantTemplates(); iter != rxn.endReactantTemplates(); ++iter, reactantId++) { addReactantAtomsAndBonds(rxn, product, reactants.at(reactantId), - reactantsMatch.at(reactantId), *iter, conf, reactantId); + reactantsMatch.at(reactantId), *iter, conf, + reactantId); } if (doConfs) { @@ -1966,11 +1965,10 @@ std::vector run_Reactant(const ChemicalReaction &rxn, namespace { int getAtomMapNo(ROMol::ATOM_BOOKMARK_MAP *map, Atom *atom) { if (map) { - for (ROMol::ATOM_BOOKMARK_MAP::const_iterator it = map->begin(); - it != map->end(); ++it) { - for (auto ait = it->second.begin(); ait != it->second.end(); ++ait) { - if (*ait == atom) { - return it->first; + for (const auto &it : *map) { + for (auto ait : it.second) { + if (ait == atom) { + return it.first; } } } diff --git a/Code/GraphMol/Chirality.cpp b/Code/GraphMol/Chirality.cpp index 275583150..a611358b6 100644 --- a/Code/GraphMol/Chirality.cpp +++ b/Code/GraphMol/Chirality.cpp @@ -1612,8 +1612,8 @@ void findChiralAtomSpecialCases(ROMol &mol, ringAtomEntry < 0 ? -ringAtomEntry - 1 : ringAtomEntry - 1; same[ringAtomIdx] = ringAtomEntry; } - for (INT_VECT_CI rae = ringStereoAtoms.begin(); - rae != ringStereoAtoms.end(); ++rae) { + for (auto rae = ringStereoAtoms.begin(); rae != ringStereoAtoms.end(); + ++rae) { int ringAtomEntry = *rae; int ringAtomIdx = ringAtomEntry < 0 ? -ringAtomEntry - 1 : ringAtomEntry - 1; @@ -3022,10 +3022,10 @@ void findPotentialStereoBonds(ROMol &mol, bool cleanIt) { } } // end of check that beg and end atoms have at least 1 // neighbor: - } // end of 2 and 3 coordinated atoms only - } // end of we want it or CIP code is not set - } // end of double bond - } // end of for loop over all bonds + } // end of 2 and 3 coordinated atoms only + } // end of we want it or CIP code is not set + } // end of double bond + } // end of for loop over all bonds mol.setProp(common_properties::_BondsPotentialStereo, 1, true); } } diff --git a/Code/GraphMol/Depictor/RDDepictor.cpp b/Code/GraphMol/Depictor/RDDepictor.cpp index 167194fd5..b2fd2a9ab 100644 --- a/Code/GraphMol/Depictor/RDDepictor.cpp +++ b/Code/GraphMol/Depictor/RDDepictor.cpp @@ -466,8 +466,8 @@ void computeInitialCoords(RDKit::ROMol &mol, if (mri == efrags.end()) { // we are out of embedded fragments, if there are any // non embedded atoms use them to start a fragment - auto mrank = static_cast(RDKit::MAX_INT); - RDKit::INT_LIST_I mnri; + auto mrank = RDKit::MAX_INT; + auto mnri = nratms.end(); for (auto nri = nratms.begin(); nri != nratms.end(); ++nri) { auto rank = atomRanks.at(*nri); rank *= mol.getNumAtoms(); diff --git a/Code/GraphMol/DistGeomHelpers/testDgeomHelpers.cpp b/Code/GraphMol/DistGeomHelpers/testDgeomHelpers.cpp index de63be90a..5ec015574 100644 --- a/Code/GraphMol/DistGeomHelpers/testDgeomHelpers.cpp +++ b/Code/GraphMol/DistGeomHelpers/testDgeomHelpers.cpp @@ -513,16 +513,12 @@ void testMultipleConfs() { ROMol *m = SmilesToMol(smi, 0, 1); INT_VECT cids = DGeomHelpers::EmbedMultipleConfs(*m, 10, 30, 100, true, false, -1); - INT_VECT_CI ci; - // SDWriter writer("junk.sdf"); - double energy; - for (ci = cids.begin(); ci != cids.end(); ci++) { - // writer.write(*m, *ci); - ForceFields::ForceField *ff = UFF::constructForceField(*m, 10, *ci); + for (auto ci : cids) { + ForceFields::ForceField *ff = UFF::constructForceField(*m, 10, ci); ff->initialize(); - energy = ff->calcEnergy(); - // BOOST_LOG(rdInfoLog) << energy << std::endl; + auto energy = ff->calcEnergy(); + TEST_ASSERT(energy > 100.0); TEST_ASSERT(energy < 300.0); delete ff; @@ -539,16 +535,11 @@ void testMultipleConfsExpTors() { *m, 10, 30, 100, true, false, -1, true, 1, -1.0, nullptr, 1e-3, false, true, false, false, false, 5.0, false, 1, false, false); - INT_VECT_CI ci; - // SDWriter writer("junk.sdf"); - double energy; - - for (ci = cids.begin(); ci != cids.end(); ci++) { - // writer.write(*m, *ci); - ForceFields::ForceField *ff = UFF::constructForceField(*m, 10, *ci); + for (auto ci : cids) { + ForceFields::ForceField *ff = UFF::constructForceField(*m, 10, ci); ff->initialize(); - energy = ff->calcEnergy(); - // BOOST_LOG(rdInfoLog) << energy << std::endl; + auto energy = ff->calcEnergy(); + TEST_ASSERT(energy > 50.0); TEST_ASSERT(energy < 300.0); delete ff; @@ -751,11 +742,10 @@ void testIssue285() { TEST_ASSERT(cids.size() == tgtNumber); std::vector molBlocks; - for (INT_VECT_CI cid = cids.begin(); cid != cids.end(); ++cid) { - molBlocks.push_back(MolToMolBlock(*m, true, *cid)); + for (auto cid : cids) { + molBlocks.push_back(MolToMolBlock(*m, true, cid)); } - for (std::vector::const_iterator mbI = molBlocks.begin(); - mbI != molBlocks.end(); ++mbI) { + for (auto mbI = molBlocks.begin(); mbI != molBlocks.end(); ++mbI) { for (auto mbJ = mbI + 1; mbJ != molBlocks.end(); ++mbJ) { TEST_ASSERT((*mbI) != (*mbJ)); } @@ -941,7 +931,7 @@ void testConstrainedEmbedding() { } { - RWMol *test = static_cast(sdsup.next()); + auto test = static_cast(sdsup.next()); MolOps::addHs(*test); std::map coords; coords[4] = ref->getConformer().getAtomPos(0); @@ -1377,16 +1367,15 @@ void testMultiThreadMultiConf() { ROMol m2(*m); DGeomHelpers::EmbedMultipleConfs(*m, cids, 200, 1, 30, 100, true, false, -1); DGeomHelpers::EmbedMultipleConfs(m2, cids, 200, 0, 30, 100, true, false, -1); - INT_VECT_CI ci; - for (ci = cids.begin(); ci != cids.end(); ci++) { - ForceFields::ForceField *ff = UFF::constructForceField(*m, 100, *ci); + for (auto ci : cids) { + ForceFields::ForceField *ff = UFF::constructForceField(*m, 100, ci); ff->initialize(); double e1 = ff->calcEnergy(); const RDGeom::PointPtrVect &pVect = ff->positions(); TEST_ASSERT(e1 > 100.0); TEST_ASSERT(e1 < 300.0); - ForceFields::ForceField *ff2 = UFF::constructForceField(m2, 100, *ci); + ForceFields::ForceField *ff2 = UFF::constructForceField(m2, 100, ci); ff2->initialize(); double e2 = ff2->calcEnergy(); const RDGeom::PointPtrVect &p2Vect = ff2->positions(); diff --git a/Code/GraphMol/FileParsers/SDWriter.cpp b/Code/GraphMol/FileParsers/SDWriter.cpp index 2b907de5a..a2f3949be 100644 --- a/Code/GraphMol/FileParsers/SDWriter.cpp +++ b/Code/GraphMol/FileParsers/SDWriter.cpp @@ -125,13 +125,12 @@ void _MolToSDStream(std::ostream *dp_ostream, const ROMol &mol, int confId, (*dp_ostream) << MolToMolBlock(mol, true, confId, df_kekulize, df_forceV3000); // now write the properties - STR_VECT_CI pi; - if (props && props->size() > 0) { + if (props && !props->empty()) { // check if we have any properties the user specified to write out // in which loop over them and write them out - for (pi = props->begin(); pi != props->end(); pi++) { - if (mol.hasProp(*pi)) { - _writePropToStream(dp_ostream, mol, (*pi), d_molid); + for (const auto &pi : *props) { + if (mol.hasProp(pi)) { + _writePropToStream(dp_ostream, mol, pi, d_molid); } } } else { @@ -141,19 +140,18 @@ void _MolToSDStream(std::ostream *dp_ostream, const ROMol &mol, int confId, STR_VECT compLst; mol.getPropIfPresent(RDKit::detail::computedPropName, compLst); - STR_VECT_CI pi; - for (pi = properties.begin(); pi != properties.end(); pi++) { + for (const auto &pi : properties) { // ignore any of the following properties - if (((*pi) == RDKit::detail::computedPropName) || - ((*pi) == common_properties::_Name) || ((*pi) == "_MolFileInfo") || - ((*pi) == "_MolFileComments") || - ((*pi) == common_properties::_MolFileChiralFlag)) { + if (pi == RDKit::detail::computedPropName || + pi == common_properties::_Name || pi == "_MolFileInfo" || + pi == "_MolFileComments" || + pi == common_properties::_MolFileChiralFlag) { continue; } // check if this property is not computed - if (std::find(compLst.begin(), compLst.end(), (*pi)) == compLst.end()) { - _writePropToStream(dp_ostream, mol, (*pi), d_molid); + if (std::find(compLst.begin(), compLst.end(), pi) == compLst.end()) { + _writePropToStream(dp_ostream, mol, pi, d_molid); } } } diff --git a/Code/GraphMol/FileParsers/SmilesWriter.cpp b/Code/GraphMol/FileParsers/SmilesWriter.cpp index 6cc548f62..ef5021d27 100644 --- a/Code/GraphMol/FileParsers/SmilesWriter.cpp +++ b/Code/GraphMol/FileParsers/SmilesWriter.cpp @@ -93,7 +93,7 @@ void SmilesWriter::dumpHeader() const { (*dp_ostream) << d_nameHeader << d_delim; } - if (d_props.size() > 0) { + if (!d_props.empty()) { auto pi = d_props.begin(); (*dp_ostream) << (*pi); pi++; @@ -123,8 +123,7 @@ void SmilesWriter::write(const ROMol &mol, int) { std::string smi = MolToSmiles(mol, df_isomericSmiles, df_kekuleSmiles); (*dp_ostream) << smi; if (d_nameHeader != "") { - if (!mol.getPropIfPresent(common_properties::_Name, name) || - name.size() == 0) { + if (!mol.getPropIfPresent(common_properties::_Name, name) || name.empty()) { std::stringstream tstream; tstream << d_molid; name = tstream.str(); @@ -133,13 +132,12 @@ void SmilesWriter::write(const ROMol &mol, int) { (*dp_ostream) << d_delim << name; } - STR_VECT_CI pi; - for (pi = d_props.begin(); pi != d_props.end(); pi++) { + for (const auto &pi : d_props) { std::string pval; // FIX: we will assume that any property that the user requests is castable // to // a std::string - if (mol.getPropIfPresent(*pi, pval)) { + if (mol.getPropIfPresent(pi, pval)) { (*dp_ostream) << d_delim << pval; } else { (*dp_ostream) << d_delim << ""; diff --git a/Code/GraphMol/FileParsers/TDTWriter.cpp b/Code/GraphMol/FileParsers/TDTWriter.cpp index 60fa8a546..2fe1782e6 100644 --- a/Code/GraphMol/FileParsers/TDTWriter.cpp +++ b/Code/GraphMol/FileParsers/TDTWriter.cpp @@ -125,13 +125,12 @@ void TDTWriter::write(const ROMol &mol, int confId) { (*dp_ostream) << ";>\n"; } // now write the properties - STR_VECT_CI pi; - if (d_props.size() > 0) { + if (!d_props.empty()) { // check if we have any properties the user specified to write out // in which loop over them and write them out - for (pi = d_props.begin(); pi != d_props.end(); pi++) { - if (mol.hasProp(*pi)) { - writeProperty(mol, (*pi)); + for (const auto &pi : d_props) { + if (mol.hasProp(pi)) { + writeProperty(mol, pi); } } } else { @@ -141,19 +140,18 @@ void TDTWriter::write(const ROMol &mol, int confId) { STR_VECT compLst; mol.getPropIfPresent(RDKit::detail::computedPropName, compLst); - STR_VECT_CI pi; - for (pi = properties.begin(); pi != properties.end(); pi++) { + for (const auto &pi : properties) { // ignore any of the following properties - if (((*pi) == RDKit::detail::computedPropName) || - ((*pi) == common_properties::_Name) || ((*pi) == "_MolFileInfo") || - ((*pi) == "_MolFileComments") || - ((*pi) == common_properties::_MolFileChiralFlag)) { + if (pi == RDKit::detail::computedPropName || + pi == common_properties::_Name || pi == "_MolFileInfo" || + pi == "_MolFileComments" || + pi == common_properties::_MolFileChiralFlag) { continue; } // check if this property is not computed - if (std::find(compLst.begin(), compLst.end(), (*pi)) == compLst.end()) { - writeProperty(mol, (*pi)); + if (std::find(compLst.begin(), compLst.end(), pi) == compLst.end()) { + writeProperty(mol, pi); } } } diff --git a/Code/GraphMol/FragCatalog/FragCatGenerator.cpp b/Code/GraphMol/FragCatalog/FragCatGenerator.cpp index caf7a24eb..b067e479c 100644 --- a/Code/GraphMol/FragCatalog/FragCatGenerator.cpp +++ b/Code/GraphMol/FragCatalog/FragCatGenerator.cpp @@ -37,7 +37,6 @@ unsigned int addOrder1Paths(PATH_LIST &paths, const ROMol &mol, double tol = fparams->getTolerance(); PATH_LIST_CI pi; - INT_VECT_CI eti; double invar; int vid; for (pi = paths.begin(); pi != paths.end(); pi++) { @@ -45,13 +44,13 @@ unsigned int addOrder1Paths(PATH_LIST &paths, const ROMol &mol, // loop over each order 1 path found = false; const INT_VECT &o1entries = fcat->getEntriesOfOrder(1); - for (eti = o1entries.begin(); eti != o1entries.end(); eti++) { + for (auto eti : o1entries) { // loop over all the order 1 entries all ready present in the catalog - entry = fcat->getEntryWithIdx(*eti); + entry = fcat->getEntryWithIdx(eti); if (nent->match(entry, tol)) { found = true; invar = computeIntVectPrimesProduct(*pi); - mapkm1[invar] = (*eti); + mapkm1[invar] = eti; delete nent; nent = nullptr; break; @@ -130,10 +129,8 @@ unsigned int addHigherOrderPaths(const INT_PATH_LIST_MAP &allPaths, unsigned int scnt = 0; INT_VECT intersect, tmpVect; - INT_VECT_CI iti; invar = computeIntVectPrimesProduct(*pi); DOUBLE_VECT sinvarV; - DOUBLE_VECT_CI sci; // loop over the subpaths (order (k-1) ) (by ignoring one bond // at a time from consideration) and find out which entries int eh catalog @@ -170,13 +167,12 @@ unsigned int addHigherOrderPaths(const INT_PATH_LIST_MAP &allPaths, } // now search through the intersection list to check if we already have a - // isomorphic - // entry in the catalog - for (iti = intersect.begin(); iti != intersect.end(); iti++) { - entry = fcat->getEntryWithIdx(*iti); + // isomorphic entry in the catalog + for (auto iti : intersect) { + entry = fcat->getEntryWithIdx(iti); if (nent->match(entry, tol)) { found = true; - mEntId = (*iti); + mEntId = iti; delete nent; nent = nullptr; break; @@ -204,12 +200,12 @@ unsigned int addHigherOrderPaths(const INT_PATH_LIST_MAP &allPaths, nrem++; // increment the fragment counter // loop over the entries corresponding to the subpaths and // add connections to them - for (sci = sinvarV.begin(); sci != sinvarV.end(); sci++) { - entId = mapkm1[*sci]; + for (auto sci : sinvarV) { + entId = mapkm1[sci]; fcat->addEdge(entId, vid); } } // end of never seen this order k subgraph - } // end of loop over order k paths in mol + } // end of loop over order k paths in mol // overwrite mapkm1 with mapk before we move on to order k+1 mapkm1 = mapk; } // end of loop over path order diff --git a/Code/GraphMol/FragCatalog/FragCatParams.cpp b/Code/GraphMol/FragCatalog/FragCatParams.cpp index dcce284ee..c58f4248e 100644 --- a/Code/GraphMol/FragCatalog/FragCatParams.cpp +++ b/Code/GraphMol/FragCatalog/FragCatParams.cpp @@ -32,22 +32,16 @@ FragCatParams::FragCatParams(unsigned int lLen, unsigned int uLen, FragCatParams::FragCatParams(const FragCatParams &other) { d_funcGroups.clear(); - // copy consttructor + d_typeStr = other.getTypeStr(); d_lowerFragLen = other.getLowerFragLength(); d_upperFragLen = other.getUpperFragLength(); d_tolerance = other.getTolerance(); - // std::cout << "In param copier\n"; const MOL_SPTR_VECT &ofgrps = other.getFuncGroups(); - // const MOL_PTR_VECT &ofgrps = other.getFuncGroups(); - MOL_SPTR_VECT::const_iterator fgi; - // MOL_PTR_VECT_CI fgi; - for (fgi = ofgrps.begin(); fgi != ofgrps.end(); fgi++) { - auto *nmol = new ROMol(*(fgi->get())); - // ROMol *nmol = new ROMol(*(*fgi)); - d_funcGroups.push_back(ROMOL_SPTR(nmol)); - // d_funcGroups.push_back(nmol); + for (auto fgi : ofgrps) { + auto *nmol = new ROMol(*fgi); + d_funcGroups.emplace_back(nmol); } } @@ -64,7 +58,7 @@ const MOL_SPTR_VECT &FragCatParams::getFuncGroups() const { const ROMol *FragCatParams::getFuncGroup(unsigned int fid) const { URANGE_CHECK(fid, d_funcGroups.size()); - // return d_funcGroups[fid]; + return d_funcGroups[fid].get(); } diff --git a/Code/GraphMol/FragCatalog/FragCatalogEntry.cpp b/Code/GraphMol/FragCatalog/FragCatalogEntry.cpp index b1e881275..6ca791602 100644 --- a/Code/GraphMol/FragCatalog/FragCatalogEntry.cpp +++ b/Code/GraphMol/FragCatalog/FragCatalogEntry.cpp @@ -16,10 +16,12 @@ #include #include #include +#include + +#include +#include #include #include -#include -#include #include namespace RDKit { @@ -65,19 +67,17 @@ FragCatalogEntry::FragCatalogEntry(const std::string &pickle) { void FragCatalogEntry::setDescription(const FragCatParams *params) { PRECONDITION(params, ""); - INT_INT_VECT_MAP::const_iterator fMapIt; - for (fMapIt = d_aToFmap.begin(); fMapIt != d_aToFmap.end(); fMapIt++) { - int atIdx = fMapIt->first; - INT_VECT fGroups = fMapIt->second; - std::string label = "", temp; + for (auto &[atIdx, fGroups] : d_aToFmap) { + std::string label; + std::string temp; - INT_VECT::const_iterator fGroupIdx = fGroups.begin(); - const ROMol *fGroup; + const ROMol *fGroup = nullptr; + auto fGroupIdx = fGroups.cbegin(); for (unsigned int i = 0; i < fGroups.size() - 1; i++) { fGroup = params->getFuncGroup(*fGroupIdx); fGroup->getProp(common_properties::_Name, temp); label += "(<" + temp + ">)"; - fGroupIdx++; + ++fGroupIdx; } fGroup = params->getFuncGroup(*fGroupIdx); fGroup->getProp(common_properties::_Name, temp); @@ -85,37 +85,32 @@ void FragCatalogEntry::setDescription(const FragCatParams *params) { dp_mol->getAtomWithIdx(atIdx)->setProp( common_properties::_supplementalSmilesLabel, label); } - std::string smi = MolToSmiles(*dp_mol); - // std::cerr << "----" << smi << "----" << std::endl; - d_descrip = smi; + + d_descrip = MolToSmiles(*dp_mol); }; bool FragCatalogEntry::match(const FragCatalogEntry *other, double tol) const { PRECONDITION(other, "bad fragment to compare"); - // std::cerr << " MATCH: "<getOrder()<getOrder()) { return false; } // now check if both the entries have the same number of functional groups const INT_INT_VECT_MAP &oFgpMap = other->getFuncGroupMap(); - // std::cerr << " "<second[0]) << ":"; - for (ofi = oFgpMap.begin(); ofi != oFgpMap.end(); ofi++) { - // std::cerr << " "<< (ofi->second[0]); - if (tfi->second == ofi->second) { + for (const auto &ofi : oFgpMap) { + if (tfi.second == ofi.second) { found = true; break; } } - // std::cerr<getDiscrims(); // REVIEW: need an overload of feq that handles tuples in MolOps, or wherever // DiscrimTuple is defined - if (!(feq(std::get<0>(tdiscs), std::get<0>(odiscs), tol)) || - !(feq(std::get<1>(tdiscs), std::get<1>(odiscs), tol)) || - !(feq(std::get<2>(tdiscs), std::get<2>(odiscs), tol))) { - return false; - } - // FIX: this may not be enough // we may have to do the actual isomorphism mapping - return true; + return feq(std::get<0>(tdiscs), std::get<0>(odiscs), tol) && + feq(std::get<1>(tdiscs), std::get<1>(odiscs), tol) && + feq(std::get<2>(tdiscs), std::get<2>(odiscs), tol); } Subgraphs::DiscrimTuple FragCatalogEntry::getDiscrims() const { @@ -145,15 +136,13 @@ Subgraphs::DiscrimTuple FragCatalogEntry::getDiscrims() const { if (this->hasProp(common_properties::Discrims)) { this->getProp(common_properties::Discrims, res); } else { - PATH_TYPE path; - for (unsigned int i = 0; i < dp_mol->getNumBonds(); ++i) { - path.push_back(i); - } + PATH_TYPE path(dp_mol->getNumBonds(), 0); + std::iota(path.begin(), path.end(), 0); // create invariant additions to reflect the functional groups attached to // the atoms - std::vector funcGpInvars; gboost::hash vectHasher; + std::vector funcGpInvars(dp_mol->getNumAtoms(), 0); for (unsigned int aid = 0; aid < dp_mol->getNumAtoms(); ++aid) { std::uint32_t invar = 0; auto mapPos = d_aToFmap.find(aid); @@ -162,7 +151,7 @@ Subgraphs::DiscrimTuple FragCatalogEntry::getDiscrims() const { std::sort(fGroups.begin(), fGroups.end()); invar = vectHasher(fGroups); } - funcGpInvars.push_back(invar); + funcGpInvars[aid] = invar; } res = Subgraphs::calcPathDiscriminators(*dp_mol, path, true, &funcGpInvars); this->setProp(common_properties::Discrims, res); @@ -196,8 +185,7 @@ void FragCatalogEntry::toStream(std::ostream &ss) const { INT_VECT tmpVect = iivmci.second; tmpInt = tmpVect.size(); streamWrite(ss, tmpInt); - for (INT_VECT_CI ivci = tmpVect.begin(); ivci != tmpVect.end(); ivci++) { - tmpInt = *ivci; + for (auto tmpInt : tmpVect) { streamWrite(ss, tmpInt); } } diff --git a/Code/GraphMol/FragCatalog/FragCatalogUtils.cpp b/Code/GraphMol/FragCatalog/FragCatalogUtils.cpp index 8f82b085c..3dba949b5 100644 --- a/Code/GraphMol/FragCatalog/FragCatalogUtils.cpp +++ b/Code/GraphMol/FragCatalog/FragCatalogUtils.cpp @@ -112,7 +112,6 @@ MatchVectType findFuncGroupsOnMol(const ROMol &mol, const FragCatParams *params, int aid; // const ROMol *fgrp; - INT_VECT_CI bi; aidFgrps.clear(); int fid = 0; @@ -154,10 +153,10 @@ MatchVectType findFuncGroupsOnMol(const ROMol &mol, const FragCatParams *params, // FIX: obviously we assume here that the function groups in params // come in decreasing order of size. bool allDone = true; - for (bi = bondIds.begin(); bi != bondIds.end(); bi++) { - if (std::find(fgBonds.begin(), fgBonds.end(), (*bi)) == fgBonds.end()) { + for (auto bi : bondIds) { + if (std::find(fgBonds.begin(), fgBonds.end(), bi) == fgBonds.end()) { allDone = false; - fgBonds.push_back(*bi); + fgBonds.push_back(bi); } } diff --git a/Code/GraphMol/FragCatalog/FragFPGenerator.cpp b/Code/GraphMol/FragCatalog/FragFPGenerator.cpp index 5e9f17f5f..5a80389b0 100644 --- a/Code/GraphMol/FragCatalog/FragFPGenerator.cpp +++ b/Code/GraphMol/FragCatalog/FragFPGenerator.cpp @@ -57,7 +57,6 @@ void FragFPGenerator::computeFP(const ROMol &mol, const FragCatalog &fcat, // first deal with order 1 stuff PATH_LIST_CI pi; - INT_VECT_CI eti; const INT_VECT &o1entries = fcat.getEntriesOfOrder(1); const FragCatalogEntry *entry; int bitId; @@ -73,14 +72,14 @@ void FragFPGenerator::computeFP(const ROMol &mol, const FragCatalog &fcat, // a match. This -1 initialization will be useful when we move onto higher // order stuff mapkm1[invar] = -1; - for (eti = o1entries.begin(); eti != o1entries.end(); eti++) { - entry = fcat.getEntryWithIdx(*eti); + for (auto eti : o1entries) { + entry = fcat.getEntryWithIdx(eti); if (nent->match(entry, tol)) { bitId = entry->getBitId(); if (bitId >= 0) { fp->setBit(bitId); } - mapkm1[invar] = (*eti); + mapkm1[invar] = eti; break; } } @@ -118,7 +117,6 @@ void FragFPGenerator::computeFP(const ROMol &mol, const FragCatalog &fcat, // << std::endl; int scnt = 0; INT_VECT intersect, tmpVect; - INT_VECT_CI iti; PATH_TYPE::const_iterator pii; // loop over the subpaths (order (k-1) ) (by ignoring one bond @@ -159,12 +157,11 @@ void FragFPGenerator::computeFP(const ROMol &mol, const FragCatalog &fcat, } } // now search through the intersection list to check if we already have a - // isomorphic - // entry in the catalog - for (iti = intersect.begin(); iti != intersect.end(); iti++) { - entry = fcat.getEntryWithIdx(*iti); + // isomorphic entry in the catalog + for (auto iti : intersect) { + entry = fcat.getEntryWithIdx(iti); if (nent->match(entry, tol)) { - mapk[invar] = (*iti); + mapk[invar] = iti; bitId = entry->getBitId(); if (bitId >= 0) { fp->setBit(bitId); @@ -178,6 +175,5 @@ void FragFPGenerator::computeFP(const ROMol &mol, const FragCatalog &fcat, // overwrite mapkm1 with mapk before we move on to order k+1 mapkm1 = mapk; } // end of loop over path order - } } // namespace RDKit diff --git a/Code/GraphMol/Kekulize.cpp b/Code/GraphMol/Kekulize.cpp index d85d4d79d..64c383e05 100644 --- a/Code/GraphMol/Kekulize.cpp +++ b/Code/GraphMol/Kekulize.cpp @@ -28,13 +28,12 @@ void backTrack(RWMol &mol, INT_INT_DEQ_MAP &, int lastOpt, INT_VECT &done, // remove on done list that comes after the lastOpt including itself auto ei = std::find(done.begin(), done.end(), lastOpt); - INT_VECT tdone; - tdone.insert(tdone.end(), done.begin(), ei); + INT_VECT tdone(done.begin(), ei); - INT_VECT_CRI eri = std::find(done.rbegin(), done.rend(), lastOpt); + auto eri = std::find(done.rbegin(), done.rend(), lastOpt); ++eri; // and push them back onto the stack - for (INT_VECT_CRI ri = done.rbegin(); ri != eri; ++ri) { + for (auto ri = done.rbegin(); ri != eri; ++ri) { aqueue.push_front(*ri); } @@ -368,7 +367,7 @@ bool kekulizeWorker(RWMol &mol, const INT_VECT &allAtms, optsV.push_back(nbrIdx); } } // end of curr atoms can have a double bond - } // end of looping over neighbors + } // end of looping over neighbors // Non-wedged options first, then wedged — both already in rank order // because nbrs was pre-sorted by lessByRank above. @@ -449,15 +448,15 @@ bool kekulizeWorker(RWMol &mol, const INT_VECT &allAtms, return false; } } // end of else try to backtrack - } // end of curr atom atom being a cand for double bond - } // end of while we are not done with all atoms + } // end of curr atom atom being a cand for double bond + } // end of while we are not done with all atoms return true; } class QuestionEnumerator { public: QuestionEnumerator(INT_VECT questions) - : d_questions(std::move(questions)), d_pos(1) {}; + : d_questions(std::move(questions)), d_pos(1){}; INT_VECT next() { INT_VECT res; if (d_pos >= (0x1u << d_questions.size())) { @@ -479,8 +478,7 @@ class QuestionEnumerator { bool permuteDummiesAndKekulize(RWMol &mol, const INT_VECT &allAtms, boost::dynamic_bitset<> dBndCands, - INT_VECT &questions, - const UINT_VECT &atomRanks, + INT_VECT &questions, const UINT_VECT &atomRanks, unsigned int maxBackTracks) { boost::dynamic_bitset<> atomsInPlay(mol.getNumAtoms()); for (int allAtm : allAtms) { @@ -509,9 +507,8 @@ bool permuteDummiesAndKekulize(RWMol &mol, const INT_VECT &allAtms, tCands[it] = 0; } // try kekulizing again: - kekulized = - kekulizeWorker(mol, allAtms, tCands, dBndAdds, done, atomRanks, - maxBackTracks); + kekulized = kekulizeWorker(mol, allAtms, tCands, dBndAdds, done, atomRanks, + maxBackTracks); } return kekulized; } @@ -533,9 +530,8 @@ void kekulizeFused(RWMol &mol, const VECT_INT_VECT ås, boost::dynamic_bitset<> dBndAdds(nbnds); markDbondCands(mol, allAtms, dBndCands, questions, done); - auto kekulized = - kekulizeWorker(mol, allAtms, dBndCands, dBndAdds, done, atomRanks, - maxBackTracks); + auto kekulized = kekulizeWorker(mol, allAtms, dBndCands, dBndAdds, done, + atomRanks, maxBackTracks); if (!kekulized && questions.size()) { // we failed, but there are some dummy atoms we can try permuting. kekulized = permuteDummiesAndKekulize(mol, allAtms, dBndCands, questions, diff --git a/Code/GraphMol/MolOps.cpp b/Code/GraphMol/MolOps.cpp index 1f8f28926..7f6fe5455 100644 --- a/Code/GraphMol/MolOps.cpp +++ b/Code/GraphMol/MolOps.cpp @@ -871,8 +871,8 @@ unsigned int getMolFrags(const ROMol &mol, VECT_INT_VECT &frags) { comMap[mi].push_back(i); } - for (INT_INT_VECT_MAP_CI mci = comMap.begin(); mci != comMap.end(); mci++) { - frags.push_back((*mci).second); + for (auto &mci : comMap) { + frags.push_back(std::move(mci.second)); } return rdcast(frags.size()); } diff --git a/Code/GraphMol/PeriodicTable.h b/Code/GraphMol/PeriodicTable.h index eb46740c1..02e3c762d 100644 --- a/Code/GraphMol/PeriodicTable.h +++ b/Code/GraphMol/PeriodicTable.h @@ -85,7 +85,7 @@ class RDKIT_GRAPHMOL_EXPORT PeriodicTable { } else if (elementSymbol == "O") { anum = 8; } else { - STR_UINT_MAP::const_iterator iter = byname.find(elementSymbol); + auto iter = byname.find(elementSymbol); if (iter != byname.end()) { anum = iter->second; } diff --git a/Code/GraphMol/Subgraphs/Subgraphs.cpp b/Code/GraphMol/Subgraphs/Subgraphs.cpp index caac0012b..448d0b878 100644 --- a/Code/GraphMol/Subgraphs/Subgraphs.cpp +++ b/Code/GraphMol/Subgraphs/Subgraphs.cpp @@ -175,17 +175,6 @@ void recurseWalkRange( } } -void dumpVIV(VECT_INT_VECT v) { - VECT_INT_VECT::iterator i; - INT_VECT::iterator j; - for (i = v.begin(); i != v.end(); i++) { - for (j = i->begin(); j != i->end(); j++) { - std::cout << *j << " "; - } - std::cout << std::endl; - } -} - PATH_LIST extendPaths(int *adjMat, unsigned int dim, const PATH_LIST &paths, int allowRingClosures = -1, double *distMat = nullptr) { diff --git a/Code/GraphMol/molopstest.cpp b/Code/GraphMol/molopstest.cpp index c5107d219..c482b08e7 100644 --- a/Code/GraphMol/molopstest.cpp +++ b/Code/GraphMol/molopstest.cpp @@ -10,6 +10,7 @@ #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #include +#include #include #include #include @@ -310,14 +311,12 @@ TEST_CASE("test3") { bfs = MolOps::symmetrizeSSSR(*m, bfrs); REQUIRE(bfs == 6); BOOST_LOG(rdInfoLog) << "BFSR: " << bfs << "\n"; - // VECT_INT_VECT_I ri; - // for (ri == bfrs.begin(); ri != bfrs.end(); ri++) { + for (auto bring : bfrs) { - INT_VECT_I mi; BOOST_LOG(rdInfoLog) << "( "; - // for (mi = (*ri).begin(); mi != (*ri).end(); mi++) { - for (mi = bring.begin(); mi != bring.end(); mi++) { - BOOST_LOG(rdInfoLog) << " " << (*mi); + + for (const auto &mi : bring) { + BOOST_LOG(rdInfoLog) << " " << mi; } BOOST_LOG(rdInfoLog) << ")\n"; } @@ -1813,21 +1812,21 @@ TEST_CASE("Testing shortest path code") { INT_LIST path = MolOps::getShortestPath(*m, 1, 20); REQUIRE(path.size() == 7); - INT_LIST_CI pi = path.begin(); + auto pi = path.begin(); REQUIRE((*pi) == 1); - pi++; + ++pi; REQUIRE((*pi) == 2); - pi++; + ++pi; REQUIRE((*pi) == 3); - pi++; + ++pi; REQUIRE((*pi) == 16); - pi++; + ++pi; REQUIRE((*pi) == 17); - pi++; + ++pi; REQUIRE((*pi) == 18); - pi++; + ++pi; REQUIRE((*pi) == 20); - pi++; + ++pi; delete m; } { @@ -1838,9 +1837,9 @@ TEST_CASE("Testing shortest path code") { INT_LIST path = MolOps::getShortestPath(*m, 0, 1); std::cerr << "path: " << path.size() << std::endl; REQUIRE(path.size() == 2); - INT_LIST_CI pi = path.begin(); + auto pi = path.begin(); REQUIRE((*pi) == 0); - pi++; + ++pi; REQUIRE((*pi) == 1); path = MolOps::getShortestPath(*m, 1, 2); @@ -1857,19 +1856,19 @@ TEST_CASE("Testing shortest path code") { INT_LIST path = MolOps::getShortestPath(*m, 8, 11); REQUIRE(path.size() == 7); - INT_LIST_CI pi = path.begin(); + auto pi = path.begin(); REQUIRE((*pi) == 8); - pi++; + ++pi; REQUIRE((*pi) == 7); - pi++; + ++pi; REQUIRE((*pi) == 2); - pi++; - pi++; // two equally long routes here - pi++; // two equally long routes here + ++pi; + ++pi; // two equally long routes here + ++pi; // two equally long routes here REQUIRE((*pi) == 10); - pi++; + ++pi; REQUIRE((*pi) == 11); - pi++; + ++pi; delete m; } } @@ -3891,9 +3890,8 @@ TEST_CASE("Testing canonicalization basics") { MatchVectType mv; REQUIRE(SubstructMatch(*m, *m2, mv)); std::map mmap; - for (MatchVectType::const_iterator mvit = mv.begin(); mvit != mv.end(); - ++mvit) { - mmap[mvit->second] = mvit->first; + for (auto &mvit : mv) { + mmap[mvit.second] = mvit.first; } REQUIRE(m2->getBondBetweenAtoms(mmap[2], mmap[3])->getBondType() == Bond::DOUBLE); @@ -3927,9 +3925,8 @@ TEST_CASE("Testing canonicalization basics") { MatchVectType mv; REQUIRE(SubstructMatch(*m, *m2, mv)); std::map mmap; - for (MatchVectType::const_iterator mvit = mv.begin(); mvit != mv.end(); - ++mvit) { - mmap[mvit->second] = mvit->first; + for (auto &mvit : mv) { + mmap[mvit.second] = mvit.first; } REQUIRE(m2->getBondBetweenAtoms(mmap[10], mmap[11])->getBondType() == Bond::DOUBLE); @@ -4057,9 +4054,8 @@ TEST_CASE("Testing canonicalization basics") { MatchVectType mv; REQUIRE(SubstructMatch(*m, *m2, mv)); std::map mmap; - for (MatchVectType::const_iterator mvit = mv.begin(); mvit != mv.end(); - ++mvit) { - mmap[mvit->second] = mvit->first; + for (auto &mvit : mv) { + mmap[mvit.second] = mvit.first; } REQUIRE(m2->getBondBetweenAtoms(mmap[1], mmap[2])->getBondType() == Bond::DOUBLE); @@ -4086,9 +4082,8 @@ TEST_CASE("Testing canonicalization basics") { MatchVectType mv; REQUIRE(SubstructMatch(*m, *m2, mv)); std::map mmap; - for (MatchVectType::const_iterator mvit = mv.begin(); mvit != mv.end(); - ++mvit) { - mmap[mvit->second] = mvit->first; + for (auto &mvit : mv) { + mmap[mvit.second] = mvit.first; } REQUIRE(m2->getBondBetweenAtoms(mmap[4], mmap[5])->getBondType() == Bond::DOUBLE); @@ -4133,9 +4128,8 @@ TEST_CASE("Testing canonicalization basics") { MatchVectType mv; REQUIRE(SubstructMatch(*m, *m2, mv)); std::map mmap; - for (MatchVectType::const_iterator mvit = mv.begin(); mvit != mv.end(); - ++mvit) { - mmap[mvit->second] = mvit->first; + for (auto &mvit : mv) { + mmap[mvit.second] = mvit.first; } REQUIRE(m2->getBondBetweenAtoms(mmap[1], mmap[2])->getBondType() == @@ -4204,9 +4198,8 @@ TEST_CASE("Testing canonicalization basics") { MatchVectType mv; REQUIRE(SubstructMatch(*m, *m2, mv)); std::map mmap; - for (MatchVectType::const_iterator mvit = mv.begin(); mvit != mv.end(); - ++mvit) { - mmap[mvit->second] = mvit->first; + for (auto &mvit : mv) { + mmap[mvit.second] = mvit.first; } REQUIRE(m2->getBondBetweenAtoms(mmap[21], mmap[13])->getBondType() == @@ -4228,9 +4221,8 @@ TEST_CASE("Testing canonicalization basics") { m2 = SmilesToMol(tsmi); REQUIRE(SubstructMatch(*m, *m2, mv)); mmap.clear(); - for (MatchVectType::const_iterator mvit = mv.begin(); mvit != mv.end(); - ++mvit) { - mmap[mvit->second] = mvit->first; + for (auto &mvit : mv) { + mmap[mvit.second] = mvit.first; } REQUIRE(m2->getBondBetweenAtoms(mmap[21], mmap[13])->getBondType() == Bond::DOUBLE); @@ -4265,9 +4257,8 @@ TEST_CASE("Testing canonicalization basics") { REQUIRE(SubstructMatch(*m, *m2, mv)); std::map mmap; mmap.clear(); - for (MatchVectType::const_iterator mvit = mv.begin(); mvit != mv.end(); - ++mvit) { - mmap[mvit->second] = mvit->first; + for (auto &mvit : mv) { + mmap[mvit.second] = mvit.first; } REQUIRE(m2->getBondBetweenAtoms(mmap[1], mmap[2])->getBondType() == Bond::DOUBLE); @@ -5256,14 +5247,15 @@ TEST_CASE( delete m; } { - std::vector smilesVec; - smilesVec.emplace_back("C1=C[CH+]1"); - smilesVec.emplace_back("C1=CC=C[CH+]C=C1"); - smilesVec.emplace_back("c1c[cH+]1"); - smilesVec.emplace_back("c1ccc[cH+]cc1"); - for (std::vector::const_iterator smiles = smilesVec.begin(); - smiles != smilesVec.end(); ++smiles) { - RWMol *m = SmilesToMol(*smiles); + constexpr std::array testSmiles = { + "C1=C[CH+]1", + "C1=CC=C[CH+]C=C1", + "c1c[cH+]1", + "c1ccc[cH+]cc1", + }; + for (const auto &smiles : testSmiles) { + CAPTURE(smiles); + auto m = SmilesToMol(smiles); REQUIRE(m); bool allConjugated = true; for (unsigned int i = 0; allConjugated && i < m->getNumBonds(); ++i) { @@ -6700,7 +6692,7 @@ TEST_CASE( sstrm.str(""); REQUIRE(sstrm.str() == ""); RWMol m; - QueryAtom *qa = new QueryAtom(); + auto qa = new QueryAtom(); qa->setQuery(makeAtomTypeQuery(1, aromatic)); qa->expandQuery(makeAtomNumQuery(6), Queries::CompositeQueryType::COMPOSITE_OR); diff --git a/Code/ML/InfoTheory/InfoBitRanker.cpp b/Code/ML/InfoTheory/InfoBitRanker.cpp index 3cf1bc86d..a033fe039 100644 --- a/Code/ML/InfoTheory/InfoBitRanker.cpp +++ b/Code/ML/InfoTheory/InfoBitRanker.cpp @@ -35,7 +35,7 @@ void InfoBitRanker::setBiasList(RDKit::INT_VECT &classList) { d_biasList = classList; // make sure we don't have any duplicates std::sort(d_biasList.begin(), d_biasList.end()); - RDKit::INT_VECT_CI bi = std::unique(d_biasList.begin(), d_biasList.end()); + auto bi = std::unique(d_biasList.begin(), d_biasList.end()); CHECK_INVARIANT(bi == d_biasList.end(), "There are duplicates in the class bias list"); @@ -48,8 +48,8 @@ void InfoBitRanker::setBiasList(RDKit::INT_VECT &classList) { void InfoBitRanker::setMaskBits(RDKit::INT_VECT &maskBits) { delete dp_maskBits; dp_maskBits = new ExplicitBitVect(d_dims); - for (RDKit::INT_VECT_CI bi = maskBits.begin(); bi != maskBits.end(); ++bi) { - dp_maskBits->setBit(*bi); + for (auto bi : maskBits) { + dp_maskBits->setBit(bi); } } diff --git a/Code/RDGeneral/types.cpp b/Code/RDGeneral/types.cpp index 67637b665..801620e08 100644 --- a/Code/RDGeneral/types.cpp +++ b/Code/RDGeneral/types.cpp @@ -13,55 +13,40 @@ namespace RDKit { -const double MAX_DOUBLE = std::numeric_limits::max(); -const double EPS_DOUBLE = std::numeric_limits::epsilon(); -const double SMALL_DOUBLE = 1.0e-8; -const double MAX_INT = static_cast(std::numeric_limits::max()); -const double MAX_LONGINT = - static_cast(std::numeric_limits::max()); - // template // T larger_of(T arg1,T arg2) { return arg1>arg2 ? arg1 : arg2; }; void Union(const INT_VECT &r1, const INT_VECT &r2, INT_VECT &res) { - res.resize(0); + res.clear(); res = r1; - INT_VECT_CI ri; - for (ri = r2.begin(); ri != r2.end(); ri++) { - if (std::find(res.begin(), res.end(), (*ri)) == res.end()) { - res.push_back(*ri); + for (auto ri : r2) { + if (std::find(res.begin(), res.end(), ri) == res.end()) { + res.push_back(ri); } } } void Intersect(const INT_VECT &r1, const INT_VECT &r2, INT_VECT &res) { - res.resize(0); - INT_VECT_CI ri; - for (ri = r1.begin(); ri != r1.end(); ri++) { - if (std::find(r2.begin(), r2.end(), (*ri)) != r2.end()) { - res.push_back(*ri); + res.clear(); + for (auto ri : r1) { + if (std::find(r2.begin(), r2.end(), ri) != r2.end()) { + res.push_back(ri); } } } void Union(const VECT_INT_VECT &rings, INT_VECT &res, const INT_VECT *exclude) { - res.resize(0); - INT_VECT ring; - unsigned int id; - auto nrings = static_cast(rings.size()); - INT_VECT_CI ri; - - for (id = 0; id < nrings; id++) { + res.clear(); + auto nrings = static_cast(rings.size()); + for (int id = 0; id < nrings; ++id) { if (exclude) { - if (std::find(exclude->begin(), exclude->end(), static_cast(id)) != - exclude->end()) { + if (std::find(exclude->begin(), exclude->end(), id) != exclude->end()) { continue; } } - ring = rings[id]; - for (ri = ring.begin(); ri != ring.end(); ri++) { - if (std::find(res.begin(), res.end(), (*ri)) == res.end()) { - res.push_back(*ri); + for (const auto &ri : rings[id]) { + if (std::find(res.begin(), res.end(), ri) == res.end()) { + res.push_back(ri); } } } diff --git a/Code/RDGeneral/types.h b/Code/RDGeneral/types.h index e220fb291..a83008776 100644 --- a/Code/RDGeneral/types.h +++ b/Code/RDGeneral/types.h @@ -213,11 +213,9 @@ typedef __int64 LONGINT; #undef min // FUCK I hate this nonsense #endif -RDKIT_RDGENERAL_EXPORT extern const double MAX_DOUBLE; -RDKIT_RDGENERAL_EXPORT extern const double EPS_DOUBLE; -RDKIT_RDGENERAL_EXPORT extern const double SMALL_DOUBLE; -RDKIT_RDGENERAL_EXPORT extern const double MAX_INT; -RDKIT_RDGENERAL_EXPORT extern const double MAX_LONGINT; +inline constexpr double MAX_DOUBLE = std::numeric_limits::max(); +inline constexpr double EPS_DOUBLE = std::numeric_limits::epsilon(); +inline constexpr int MAX_INT = std::numeric_limits::max(); typedef unsigned int UINT; typedef unsigned short USHORT; diff --git a/Code/SimDivPickers/HierarchicalClusterPicker.cpp b/Code/SimDivPickers/HierarchicalClusterPicker.cpp index 47a51b26d..f09f7c140 100644 --- a/Code/SimDivPickers/HierarchicalClusterPicker.cpp +++ b/Code/SimDivPickers/HierarchicalClusterPicker.cpp @@ -70,9 +70,8 @@ RDKit::VECT_INT_VECT HierarchicalClusterPicker::cluster( // add the items from cluster cx2 to cx1 // REVIEW: merge function??? - for (RDKit::INT_VECT_CI cx2i = clusters[cx2].begin(); - cx2i != clusters[cx2].end(); cx2i++) { - clusters[cx1].push_back(*cx2i); + for (const auto &cx2i : clusters[cx2]) { + clusters[cx1].push_back(cx2i); } // mark the second cluster as removed @@ -88,7 +87,7 @@ RDKit::VECT_INT_VECT HierarchicalClusterPicker::cluster( // some error checking here, uniqueify removed and the vector should not // changed // REVIEW can we put this inside a #ifdef DEBUG? - RDKit::INT_VECT_CI nEnd = std::unique(removed.begin(), removed.end()); + auto nEnd = std::unique(removed.begin(), removed.end()); CHECK_INVARIANT( nEnd == removed.end(), "Somehow there are duplicates in the list of removed clusters"); @@ -118,12 +117,10 @@ RDKit::INT_VECT HierarchicalClusterPicker::pick(const double *distMat, for (unsigned int i = 0; i < pickSize; i++) { int pick; double minSumD2 = RDKit::MAX_DOUBLE; - for (RDKit::INT_VECT_CI cxi1 = clusters[i].begin(); - cxi1 != clusters[i].end(); ++cxi1) { + for (auto cxi1 = clusters[i].begin(); cxi1 != clusters[i].end(); ++cxi1) { int curPick = (*cxi1); double d2sum = 0.0; - for (RDKit::INT_VECT_CI cxi2 = clusters[i].begin(); - cxi2 != clusters[i].end(); ++cxi2) { + for (auto cxi2 = clusters[i].begin(); cxi2 != clusters[i].end(); ++cxi2) { if (cxi1 == cxi2) { continue; }