Revert "A variety of clang-tidy fixes"

Signed-off-by: Geoff Hutchison <geoff.hutchison@gmail.com>

avogadro/calc/chargemanager.cpp

@@ -101,7 +101,7 @@ std::set<std::string> ChargeManager::identifiersForMolecule(
   std::set<std::string> identifiers = molecule.partialChargeTypes();
 
   // check our models for compatibility
-  for (auto *m_model : m_models) {
+  for (auto m_model : m_models) {
 
     // We check that every element in the molecule
     // is handled by the model

avogadro/calc/defaultmodel.cpp

@@ -8,12 +8,14 @@
 #include <avogadro/core/array.h>
 #include <avogadro/core/molecule.h>
 
-#include <utility>
+namespace Avogadro {
 
-namespace Avogadro::Calc {
+using Core::Molecule;
 
-DefaultModel::DefaultModel(std::string  id)
-  : ChargeModel(), m_identifier(std::move(id))
+namespace Calc {
+
+DefaultModel::DefaultModel(const std::string& id)
+  : ChargeModel(), m_identifier(id)
 {
   // we don't know which elements are in the molecule
   // but we can just say all of them are okay
@@ -28,4 +30,5 @@ MatrixX DefaultModel::partialCharges(Core::Molecule& mol) const
   return mol.partialCharges(m_identifier);
 }
 
+} // namespace Calc
 } // namespace Avogadro

avogadro/calc/defaultmodel.h

@@ -32,7 +32,7 @@ namespace Calc {
 class AVOGADROCALC_EXPORT DefaultModel : public ChargeModel
 {
 public:
-  DefaultModel(std::string  identifier = "");
+  DefaultModel(const std::string& identifier = "");
   virtual ~DefaultModel();
 
   /**

avogadro/core/atomutilities.cpp

@@ -75,7 +75,7 @@ Vector3 AtomUtilities::generateNewBondVector(
     return newPos;
   } else if (currentValence == 1) {
     // One bonded atom
-    const Vector3& bond1 = allVectors[0];
+    Vector3 bond1 = allVectors[0];
 
     // Check what's attached to our neighbor -- we want to set trans to the
     // neighbor
@@ -147,8 +147,8 @@ Vector3 AtomUtilities::generateNewBondVector(
     return -1.0 * newPos.normalized();
   } // end one bond
   else if (currentValence == 2) {
-    const Vector3& bond1 = allVectors[0];
-    const Vector3& bond2 = allVectors[1];
+    Vector3 bond1 = allVectors[0];
+    Vector3 bond2 = allVectors[1];
 
     Vector3 v1 = bond1 + bond2;
     v1.normalize();
@@ -170,9 +170,9 @@ Vector3 AtomUtilities::generateNewBondVector(
     return -1.0 * newPos.normalized();
   } // end two bonds
   else if (currentValence == 3) {
-    const Vector3& bond1 = allVectors[0];
-    const Vector3& bond2 = allVectors[1];
-    const Vector3& bond3 = allVectors[2];
+    Vector3 bond1 = allVectors[0];
+    Vector3 bond2 = allVectors[1];
+    Vector3 bond3 = allVectors[2];
 
     // need to handle different hybridizations here
 

avogadro/core/avospglib.cpp

@@ -36,7 +36,7 @@ unsigned short AvoSpglib::getHallNumber(Molecule& mol, double cartTol)
   }
 
   Index numAtoms = mol.atomCount();
-  auto *positions = new double[numAtoms][3];
+  auto positions = new double[numAtoms][3];
   int* types = new int[numAtoms];
 
   const Array<unsigned char>& atomicNums = mol.atomicNumbers();
@@ -111,7 +111,7 @@ bool AvoSpglib::standardizeCell(Molecule& mol, double cartTol, bool toPrimitive,
   // If toPrimitive is true, then we will just use the number of atoms.
   // See http://atztogo.github.io/spglib/api.html#spg-standardize-cell
   int numAtomsMultiplier = toPrimitive ? 1 : 4;
-  auto *positions = new double[numAtoms * numAtomsMultiplier][3];
+  auto positions = new double[numAtoms * numAtomsMultiplier][3];
   int* types = new int[numAtoms * numAtomsMultiplier];
 
   const Array<unsigned char>& atomicNums = mol.atomicNumbers();

avogadro/core/cube.cpp

@@ -118,7 +118,7 @@ const std::vector<float>* Cube::data() const
 
 bool Cube::setData(const std::vector<float>& values)
 {
-  if (values.empty())
+  if (!values.size())
     return false;
 
   if (static_cast<int>(values.size()) ==
@@ -141,9 +141,9 @@ bool Cube::setData(const std::vector<float>& values)
 bool Cube::addData(const std::vector<float>& values)
 {
   // Initialise the cube to zero if necessary
-  if (m_data.empty())
+  if (!m_data.size())
     m_data.resize(m_points.x() * m_points.y() * m_points.z());
-  if (values.size() != m_data.size() || values.empty())
+  if (values.size() != m_data.size() || !values.size())
     return false;
   for (unsigned int i = 0; i < m_data.size(); i++) {
     m_data[i] += values[i];

avogadro/core/elements.cpp

@@ -13,6 +13,7 @@
 #include <cctype>
 #include <vector>
 
+using Avogadro::Core::isCustomElement;
 
 namespace Avogadro::Core {
 

avogadro/core/gaussiansettools.cpp

@@ -120,12 +120,18 @@ double GaussianSetTools::calculateSpinDensity(const Vector3& position) const
 
 bool GaussianSetTools::isValid() const
 {
-  return m_molecule && dynamic_cast<GaussianSet*>(m_molecule->basisSet());
+  if (m_molecule && dynamic_cast<GaussianSet*>(m_molecule->basisSet()))
+    return true;
+  else
+    return false;
 }
 
 inline bool GaussianSetTools::isSmall(double val) const
 {
-  return val > -1e-20 && val < 1e-20;
+  if (val > -1e-20 && val < 1e-20)
+    return true;
+  else
+    return false;
 }
 
 inline vector<double> GaussianSetTools::calculateValues(

avogadro/core/layermanager.cpp

@@ -73,7 +73,7 @@ void LayerManager::deleteMolecule(const Molecule* mol)
 
   auto aux = m_molToInfo.find(mol);
   if (aux != m_molToInfo.end()) {
-    const auto *id = aux->second->mol;
+    auto id = aux->second->mol;
     if (id == mol) {
       auto it = m_molToInfo.begin();
       while (it != m_molToInfo.end()) {

avogadro/core/mesh.cpp

@@ -121,7 +121,7 @@ const Color3f* Mesh::color(int n) const
 {
   // If there is only one color return that, otherwise colored by vertex.
   if (m_colors.size() == 1)
-    return m_colors.data();
+    return &(m_colors[0]);
   else
     return &(m_colors[n * 3]);
 }
@@ -149,7 +149,10 @@ bool Mesh::addColors(const Core::Array<Color3f>& values)
 bool Mesh::valid() const
 {
   if (m_vertices.size() == m_normals.size()) {
-    return m_colors.size() == 1 || m_colors.size() == m_vertices.size();
+    if (m_colors.size() == 1 || m_colors.size() == m_vertices.size())
+      return true;
+    else
+      return false;
   } else {
     return false;
   }
@@ -176,7 +179,7 @@ Mesh& Mesh::operator=(const Mesh& other)
 
 void Mesh::smooth(int iterationCount)
 {
-  if (m_vertices.empty())
+  if (m_vertices.size() == 0)
     return;
   if (iterationCount <= 0)
     return;

avogadro/core/molecule.cpp

@@ -262,7 +262,7 @@ MatrixX Molecule::partialCharges(const std::string& type) const
 std::set<std::string> Molecule::partialChargeTypes() const
 {
   std::set<std::string> types;
-  for (const auto& it : m_partialCharges)
+  for (auto& it : m_partialCharges)
     types.insert(it.first);
   return types;
 }
@@ -324,9 +324,9 @@ signed char Molecule::totalCharge() const
   // check the data map first
   if (m_data.hasValue("totalCharge")) {
     charge = m_data.value("totalCharge").toInt();
-  } else if (!m_formalCharges.empty()) {
-    for (signed char m_formalCharge : m_formalCharges)
-      charge += m_formalCharge;
+  } else if (m_formalCharges.size() > 0) {
+    for (Index i = 0; i < m_formalCharges.size(); ++i)
+      charge += m_formalCharges[i];
     return charge;
   }
   return charge; // should be zero
@@ -342,8 +342,8 @@ char Molecule::totalSpinMultiplicity() const
   } else {
     // add up the electrons
     unsigned long electrons = 0;
-    for (unsigned char m_atomicNumber : m_atomicNumbers)
-      electrons += m_atomicNumber;
+    for (Index i = 0; i < m_atomicNumbers.size(); ++i)
+      electrons += m_atomicNumbers[i];
 
     // adjust by the total charge
     electrons -= totalCharge();
@@ -1274,7 +1274,7 @@ bool Molecule::removeBonds(Index atom)
 
   while (true) {
     const std::vector<size_t>& bondList = m_graph.edges(atom);
-    if (bondList.empty())
+    if (!bondList.size())
       break;
     size_t bond = bondList[0];
     removeBond(bond);

avogadro/core/neighborperceiver.cpp

@@ -10,7 +10,7 @@ namespace Avogadro::Core {
 NeighborPerceiver::NeighborPerceiver(const Array<Vector3> points, float maxDistance)
  : m_maxDistance(maxDistance), m_cachedArray(nullptr)
 {
-  if (points.empty()) return;
+  if (!points.size()) return;
 
   // find bounding box
   m_minPos = points[0];

avogadro/core/ringperceiver.cpp

@@ -384,7 +384,7 @@ std::vector<std::vector<size_t>> perceiveRings(const Graph& graph)
   std::vector<RingCandidate> candidates;
   for (size_t i = 0; i < n; i++) {
     for (size_t j = i + 1; j < n; j++) {
-      if (P(i, j).size() == 1 && Pt(i, j).empty()) {
+      if (P(i, j).size() == 1 && Pt(i, j).size() == 0) {
         continue;
       } else {
         size_t size;

avogadro/core/secondarystructure.cpp

@@ -23,7 +23,7 @@ SecondaryStructureAssigner::SecondaryStructureAssigner(Molecule* mol)
 
 SecondaryStructureAssigner::~SecondaryStructureAssigner()
 {
-  for (auto *hBond : m_hBonds)
+  for (auto hBond : m_hBonds)
     delete hBond;
   m_hBonds.clear();
 }
@@ -44,7 +44,7 @@ void SecondaryStructureAssigner::assign(Molecule* mol)
 
   float infinity = std::numeric_limits<float>::max();
   // Then assign the alpha helix by going through the hBond records
-  for (auto *hBond : m_hBonds) {
+  for (auto hBond : m_hBonds) {
     if (hBond->distSquared < infinity) {
       // check to see how far apart the residues are
       int separation = std::abs(int(hBond->residue - hBond->residuePair));
@@ -79,7 +79,7 @@ void SecondaryStructureAssigner::assign(Molecule* mol)
   }
 
   // Then assign the beta sheet - but only if a residue isn't assigned
-  for (auto *hBond : m_hBonds) {
+  for (auto hBond : m_hBonds) {
     if (hBond->distSquared < infinity) {
       if (m_molecule->residue(hBond->residue).secondaryStructure() ==
           Residue::SecondaryStructure::undefined)
@@ -89,7 +89,7 @@ void SecondaryStructureAssigner::assign(Molecule* mol)
   }
 
   // Check that sheets bond to other sheets
-  for (auto *hBond : m_hBonds) {
+  for (auto hBond : m_hBonds) {
     if (hBond->distSquared < infinity) {
       // find the match
       auto current = m_molecule->residue(hBond->residue);
@@ -169,7 +169,7 @@ void SecondaryStructureAssigner::assignBackboneHydrogenBonds()
   const float maxDistSq = maxDist * maxDist; // 10.24
 
   // delete any previous records
-  for (auto *hBond : m_hBonds)
+  for (auto hBond : m_hBonds)
     delete hBond;
   m_hBonds.clear();
 
@@ -204,7 +204,7 @@ void SecondaryStructureAssigner::assignBackboneHydrogenBonds()
     }
   }
 
-  if (m_hBonds.empty())
+  if (m_hBonds.size() == 0)
     return;
 
   // sort by z-coordinate
@@ -217,11 +217,11 @@ void SecondaryStructureAssigner::assignBackboneHydrogenBonds()
   // now loop through the sorted list (so we can exit quickly)
   int n = m_hBonds.size();
   for (int i = 0; i < n; ++i) {
-    auto *recordI = m_hBonds[i];
+    auto recordI = m_hBonds[i];
     const Residue& residueI = m_molecule->residue(recordI->residue);
 
     for (int j = i + 1; j < n; ++j) {
-      auto *recordJ = m_hBonds[j];
+      auto recordJ = m_hBonds[j];
       const Residue& residueJ = m_molecule->residue(recordJ->residue);
 
       // skip if we're not on the same chain

avogadro/core/slatersettools.cpp

@@ -74,12 +74,18 @@ double SlaterSetTools::calculateSpinDensity(const Vector3&) const
 
 bool SlaterSetTools::isValid() const
 {
-  return m_molecule && dynamic_cast<SlaterSet*>(m_molecule->basisSet());
+  if (m_molecule && dynamic_cast<SlaterSet*>(m_molecule->basisSet()))
+    return true;
+  else
+    return false;
 }
 
 inline bool SlaterSetTools::isSmall(double val) const
 {
-  return val > -1e-20 && val < 1e-20;
+  if (val > -1e-20 && val < 1e-20)
+    return true;
+  else
+    return false;
 }
 
 vector<double> SlaterSetTools::calculateValues(const Vector3& position) const