Refactor AtomProperties into C++
authorPaul Bauer <paul.bauer.q@gmail.com>
Tue, 8 Jan 2019 08:59:29 +0000 (09:59 +0100)
committerMark Abraham <mark.j.abraham@gmail.com>
Mon, 14 Jan 2019 11:42:45 +0000 (12:42 +0100)
Replaced raw pointers with vectors.

Change-Id: Ib1bdfcda9b618a70b5ce06beae1d3e9240e59ef8

src/gromacs/gmxana/gmx_editconf.cpp
src/gromacs/gmxpreprocess/makeexclusiondistances.cpp
src/gromacs/gmxpreprocess/solvate.cpp
src/gromacs/topology/atomprop.cpp
src/gromacs/topology/atomprop.h

index 4aecb9e3c3f48fc64a997537c00bbb05b74ff271..933b718c65d581cddca883a5068a7226cba65acf 100644 (file)
@@ -40,6 +40,7 @@
 #include <cstring>
 
 #include <algorithm>
+#include <string>
 
 #include "gromacs/commandline/pargs.h"
 #include "gromacs/commandline/viewit.h"
@@ -77,8 +78,8 @@ static real calc_mass(t_atoms *atoms, gmx_bool bGetMass, AtomProperties *aps)
         if (bGetMass)
         {
             aps->setAtomProperty(epropMass,
-                                 *atoms->resinfo[atoms->atom[i].resind].name,
-                                 *atoms->atomname[i], &(atoms->atom[i].m));
+                                 std::string(*atoms->resinfo[atoms->atom[i].resind].name),
+                                 std::string(*atoms->atomname[i]), &(atoms->atom[i].m));
         }
         tmass += atoms->atom[i].m;
     }
index 31dcb187f59e599da18a8f0c2dfe5b014f6cbb5d..ad524735d83c51af9903f1610e5eb2fa1901a63f 100644 (file)
@@ -36,6 +36,7 @@
 
 #include "makeexclusiondistances.h"
 
+#include <string>
 #include <vector>
 
 #include "gromacs/topology/atomprop.h"
@@ -52,10 +53,10 @@ makeExclusionDistances(const t_atoms *a, AtomProperties *aps,
         exclusionDistances.reserve(a->nr);
         for (int i = 0; i < a->nr; ++i)
         {
-            real value;
+            real        value;
             if (!aps->setAtomProperty(epropVDW,
-                                      *(a->resinfo[a->atom[i].resind].name),
-                                      *(a->atomname[i]), &value))
+                                      std::string(*(a->resinfo[a->atom[i].resind].name)),
+                                      std::string(*(a->atomname[i])), &value))
             {
                 value = defaultDistance;
             }
index 5e077e116489a0aaa86c625d0df777f8dbfb3027..40405d567016736b2042e80d7ea692dba70527e5 100644 (file)
@@ -765,8 +765,8 @@ static void update_top(t_atoms        *atoms,
     for (i = 0; (i < atoms->nr); i++)
     {
         aps->setAtomProperty(epropMass,
-                             *atoms->resinfo[atoms->atom[i].resind].name,
-                             *atoms->atomname[i], &mm);
+                             std::string(*atoms->resinfo[atoms->atom[i].resind].name),
+                             std::string(*atoms->atomname[i]), &mm);
         mtot += mm;
     }
 
index 0dfdc29b7477636dfdca7bbc7b46a183bb74b941..61066878aa0e76f3baf95fd21824f63b9e90a689 100644 (file)
@@ -43,6 +43,8 @@
 #include <cstdio>
 #include <cstring>
 
+#include <algorithm>
+
 #include "gromacs/compat/make_unique.h"
 #include "gromacs/math/functions.h"
 #include "gromacs/math/utilities.h"
@@ -60,26 +62,32 @@ enum {
     NOTFOUND = -4, WILDCARD, WILDPROT, PROTEIN
 };
 
+//! Basic entries in AtomProperty.
+struct BaseEntry {
+    //! Default constructor.
+    BaseEntry(const std::string &aName, const std::string &rName)
+        : atomName(aName), residueName(rName), isAvailable(false), value(0.0)
+    {}
+    //! Name for atom.
+    std::string atomName;
+    //! Name for residue.
+    std::string residueName;
+    //! Is property available.
+    bool        isAvailable;
+    //! Value set for property.
+    real        value;
+};
+
 //! Conglomeration of atom property entries.
 struct AtomProperty {
     //! Has property been set.
-    bool   isSet = false;
-    //! Number of properties.
-    int    nprop = 0;
-    //! Max number of properties.
-    int    maxprop = 0;
+    bool                   isSet = false;
     //! Database the property is coming from.
-    char  *db = nullptr;
+    std::string            db;
     //! Default value for property.
-    double def = 0.0;
-    //! Array of names for atoms.
-    char **atomName = nullptr;
-    //! Array of names for residues.
-    char **residueName = nullptr;
-    //! Array of flags if property is available.
-    bool  *isAvailable = nullptr;
-    //! Array of values for property.
-    real  *value = nullptr;
+    double                 def = 0.0;
+    //! Basic entries for properties.
+    std::vector<BaseEntry> entry;
 };
 
 //! Implementation detail type for Atomproperties.
@@ -107,19 +115,28 @@ class AtomProperties::Impl
  * \param[in] database Name of the database entry to compare to.
  * \returns Number of matching characters or NOTFOUND.
  */
-static int findNumberOfMatches(const char *search, const char *database)
+static int compareToDatabase(const std::string &search, const std::string &database)
 {
-    int i = 0;
-    while (search[i] && database[i] && (search[i] == database[i]) )
+    if (database.length() > search.length())
     {
-        i++;
+        return NOTFOUND;
     }
-
-    if (database[i])
+    size_t matches = 0;
+    for (size_t i = 0; i < database.length(); i++)
+    {
+        if (search[i] == database[i])
+        {
+            matches++;
+        }
+    }
+    if (matches == database.length())
+    {
+        return matches;
+    }
+    else
     {
-        i = NOTFOUND;
+        return NOTFOUND;
     }
-    return i;
 }
 
 /*! \brief
@@ -133,31 +150,31 @@ static int findNumberOfMatches(const char *search, const char *database)
  * \returns The index for the property.
  */
 static int findPropertyIndex(AtomProperty *ap, gmx_residuetype_t *restype,
-                             char *residueName, char *atomName,
+                             const std::string &residueName, const std::string &atomName,
                              gmx_bool *bExact)
 {
     int      j = NOTFOUND;
 
-    bool     bProtein  = gmx_residuetype_is_protein(restype, residueName);
-    bool     bProtWild = (strcmp(residueName, "AAA") == 0);
+    bool     bProtein  = gmx_residuetype_is_protein(restype, residueName.c_str());
+    bool     bProtWild = residueName == "AAA";
     int      malen     = NOTFOUND;
     int      mrlen     = NOTFOUND;
-    for (int i = 0; (i < ap->nprop); i++)
+    for (size_t i = 0; (i < ap->entry.size()); i++)
     {
-        int rlen = findNumberOfMatches(residueName, ap->residueName[i]);
+        int rlen = compareToDatabase(residueName, ap->entry[i].residueName);
         if (rlen == NOTFOUND)
         {
-            if ( (strcmp(ap->residueName[i], "*") == 0) ||
-                 (strcmp(ap->residueName[i], "???") == 0) )
+            if ( (ap->entry[i].residueName == "*") ||
+                 (ap->entry[i].residueName == "???")  )
             {
                 rlen = WILDCARD;
             }
-            else if (strcmp(ap->residueName[i], "AAA") == 0)
+            else if (ap->entry[i].residueName == "AAA")
             {
                 rlen = WILDPROT;
             }
         }
-        int alen = findNumberOfMatches(atomName, ap->atomName[i]);
+        int alen = compareToDatabase(atomName, ap->entry[i].atomName);
         if ( (alen > NOTFOUND) && (rlen > NOTFOUND))
         {
             if ( ( (alen > malen) && (rlen >= mrlen)) ||
@@ -170,21 +187,23 @@ static int findPropertyIndex(AtomProperty *ap, gmx_residuetype_t *restype,
         }
     }
 
-    *bExact = ((malen == static_cast<long int>(strlen(atomName))) &&
-               ((mrlen == static_cast<long int>(strlen(residueName))) ||
+    *bExact = ((malen == static_cast<long int>(atomName.length())) &&
+               ((mrlen == static_cast<long int>(residueName.length())) ||
                 ((mrlen == WILDPROT) && bProtWild) ||
                 ((mrlen == WILDCARD) && !bProtein && !bProtWild)));
 
     if (debug)
     {
-        fprintf(debug, "searching residue: %4s atom: %4s\n", residueName, atomName);
+        fprintf(debug, "searching residue: %4s atom: %4s\n", residueName.c_str(),
+                atomName.c_str());
         if (j == NOTFOUND)
         {
             fprintf(debug, " not successful\n");
         }
         else
         {
-            fprintf(debug, " match: %4s %4s\n", ap->residueName[j], ap->atomName[j]);
+            fprintf(debug, " match: %4s %4s\n",
+                    ap->entry[j].residueName.c_str(), ap->entry[j].atomName.c_str());
         }
     }
     return j;
@@ -201,7 +220,7 @@ static int findPropertyIndex(AtomProperty *ap, gmx_residuetype_t *restype,
  * \param[in] line Where to add property.
  */
 static void addProperty(AtomProperty *ap, gmx_residuetype_t *restype,
-                        char *residueName, char *atomName,
+                        const std::string &residueName, const std::string &atomName,
                         real propValue, int line)
 {
     bool bExact;
@@ -209,45 +228,30 @@ static void addProperty(AtomProperty *ap, gmx_residuetype_t *restype,
 
     if (!bExact)
     {
-        if (ap->nprop >= ap->maxprop)
-        {
-            ap->maxprop += 10;
-            srenew(ap->residueName, ap->maxprop);
-            srenew(ap->atomName, ap->maxprop);
-            srenew(ap->value, ap->maxprop);
-            srenew(ap->isAvailable, ap->maxprop);
-            for (int i = ap->nprop; (i < ap->maxprop); i++)
-            {
-                ap->atomName[i]     = nullptr;
-                ap->residueName[i]  = nullptr;
-                ap->value[i]        = 0;
-                ap->isAvailable[i]  = FALSE;
-            }
-        }
-        ap->atomName[ap->nprop]     = gmx_strdup(atomName);
-        ap->residueName[ap->nprop]  = gmx_strdup(residueName);
-        j                           = ap->nprop;
-        ap->nprop++;
+        ap->entry.emplace_back(BaseEntry(atomName, residueName));
+
+        j = ap->entry.size() - 1;
     }
-    if (ap->isAvailable[j])
+    if (ap->entry[j].isAvailable)
     {
-        if (ap->value[j] == propValue)
+        if (ap->entry[j].value == propValue)
         {
             fprintf(stderr, "Warning double identical entries for %s %s %g on line %d in file %s\n",
-                    residueName, atomName, propValue, line, ap->db);
+                    residueName.c_str(), atomName.c_str(), propValue, line, ap->db.c_str());
         }
         else
         {
             fprintf(stderr, "Warning double different entries %s %s %g and %g on line %d in file %s\n"
                     "Using last entry (%g)\n",
-                    residueName, atomName, propValue, ap->value[j], line, ap->db, propValue);
-            ap->value[j] = propValue;
+                    residueName.c_str(), atomName.c_str(),
+                    propValue, ap->entry[j].value, line, ap->db.c_str(), propValue);
+            ap->entry[j].value = propValue;
         }
     }
     else
     {
-        ap->isAvailable[j] = TRUE;
-        ap->value[j]       = propValue;
+        ap->entry[j].isAvailable = TRUE;
+        ap->entry[j].value       = propValue;
     }
 }
 
@@ -276,7 +280,7 @@ static void readProperty(AtomProperty *ap, gmx_residuetype_t *restype, double fa
         else
         {
             fprintf(stderr, "WARNING: Error in file %s at line %d ignored\n",
-                    ap->db, line_no);
+                    ap->db.c_str(), line_no);
         }
     }
     ap->isSet = TRUE;
@@ -300,13 +304,13 @@ static bool setProperties(AtomProperty *ap, gmx_residuetype_t *restype, int epro
     bool              printWarning = false;
     if (!ap->isSet)
     {
-        ap->db  = gmx_strdup(fns[eprop]);
+        ap->db  = fns[eprop];
         ap->def = def[eprop];
         readProperty(ap, restype, fac[eprop]);
 
         if (debug)
         {
-            fprintf(debug, "Entries in %s: %d\n", ap->db, ap->nprop);
+            fprintf(debug, "Entries in %s: %zu\n", ap->db.c_str(), ap->entry.size());
         }
 
         if ( (!haveBeenWarned && (eprop == epropMass) ) || (eprop == epropVDW))
@@ -324,30 +328,8 @@ AtomProperties::AtomProperties()
     gmx_residuetype_init(&impl_->restype);
 }
 
-static void destroy_prop(AtomProperty *ap)
-{
-    if (ap->isSet)
-    {
-        sfree(ap->db);
-
-        for (int i = 0; i < ap->nprop; i++)
-        {
-            sfree(ap->atomName[i]);
-            sfree(ap->residueName[i]);
-        }
-        sfree(ap->atomName);
-        sfree(ap->residueName);
-        sfree(ap->isAvailable);
-        sfree(ap->value);
-    }
-}
-
 AtomProperties::~AtomProperties()
 {
-    for (int p = 0; p < epropNR; p++)
-    {
-        destroy_prop(&impl_->prop[p]);
-    }
     gmx_residuetype_destroy(impl_->restype);
 }
 
@@ -361,7 +343,7 @@ gmx_residuetype_t *AtomProperties::restype()
     return impl_->restype;
 }
 
-//! Print warning about vdW to terminal.
+//! Print warning that vdW radii and masses are guessed.
 static void printWarning()
 {
     printf("\n"
@@ -385,48 +367,32 @@ static void printvdwWarning(FILE *fp)
     }
 }
 
-bool AtomProperties::setAtomProperty(int         eprop,
-                                     const char *residueName,
-                                     const char *atomName,
-                                     real       *value)
+bool AtomProperties::setAtomProperty(int                eprop,
+                                     const std::string &residueName,
+                                     const std::string &atomName,
+                                     real              *value)
 {
-    int  j;
-#define MAXQ 32
-    char atomname[MAXQ], resname[MAXQ];
-    bool bExact;
+    int           j;
+    std::string   tmpAtomName, tmpResidueName;
+    gmx_bool      bExact;
 
     if (setProperties(prop(eprop), restype(), eprop, impl_->bWarned))
     {
         printWarning();
         impl_->bWarned = true;
     }
-    if ((strlen(atomName) > MAXQ-1) || (strlen(residueName) > MAXQ-1))
-    {
-        if (debug)
-        {
-            fprintf(debug, "WARNING: will only compare first %d characters\n",
-                    MAXQ-1);
-        }
-    }
     if (isdigit(atomName[0]))
     {
-        int i;
         /* put digit after atomname */
-        for (i = 1; i < MAXQ-1 && atomName[i] != '\0'; i++)
-        {
-            atomname[i-1] = atomName[i];
-        }
-        atomname[i-1] = atomName[0];
-        atomname[i]   = '\0';
+        tmpAtomName.append(atomName.substr(1));
+        tmpAtomName.append(1, atomName[0]);
     }
     else
     {
-        strncpy(atomname, atomName, MAXQ-1);
+        tmpAtomName = atomName;
     }
-    strncpy(resname, residueName, MAXQ-1);
-
-    j = findPropertyIndex(&(impl_->prop[eprop]), impl_->restype, resname,
-                          atomname, &bExact);
+    j = findPropertyIndex(&(impl_->prop[eprop]), impl_->restype, residueName,
+                          tmpAtomName, &bExact);
 
     if (eprop == epropVDW && !impl_->bWarnVDW)
     {
@@ -435,7 +401,7 @@ bool AtomProperties::setAtomProperty(int         eprop,
     }
     if (j >= 0)
     {
-        *value = impl_->prop[eprop].value[j];
+        *value = impl_->prop[eprop].entry[j].value;
         return true;
     }
     else
@@ -446,21 +412,21 @@ bool AtomProperties::setAtomProperty(int         eprop,
 }
 
 
-const char *AtomProperties::elementFromAtomNumber(int atomNumber)
+const std::string AtomProperties::elementFromAtomNumber(int atomNumber)
 {
     if (setProperties(prop(epropElement), restype(), epropElement, impl_->bWarned))
     {
         printWarning();
         impl_->bWarned = true;
     }
-    for (int i = 0; (i < impl_->prop[epropElement].nprop); i++)
+    for (const auto &e : prop(epropElement)->entry)
     {
-        if (std::round(impl_->prop[epropElement].value[i]) == atomNumber)
+        if (std::round(e.value) == atomNumber)
         {
-            return impl_->prop[epropElement].atomName[i];
+            return e.atomName;
         }
     }
-    return nullptr;
+    return "";
 }
 
 int AtomProperties::atomNumberFromElement(const char *element)
@@ -470,11 +436,11 @@ int AtomProperties::atomNumberFromElement(const char *element)
         printWarning();
         impl_->bWarned = true;
     }
-    for (int i = 0; (i < impl_->prop[epropElement].nprop); i++)
+    for (const auto &e : prop(epropElement)->entry)
     {
-        if (gmx_strcasecmp(impl_->prop[epropElement].atomName[i], element) == 0)
+        if (gmx_strcasecmp(e.atomName.c_str(), element) == 0)
         {
-            return gmx::roundToInt(impl_->prop[epropElement].value[i]);
+            return gmx::roundToInt(e.value);
         }
     }
     return -1;
index 6b3cd081e61aaeb43864894a6d02996579347a6b..5eba35b3d73f6bb3281b558e11fb70d7b0cb5584 100644 (file)
@@ -1,7 +1,9 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
+ * Copyright (c) 2001-2004, The GROMACS development team.
+ * Copyright (c) 2010,2014,2018,2019, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -35,6 +37,8 @@
 #ifndef GMX_TOPOLOGY_ATOMPROP_H
 #define GMX_TOPOLOGY_ATOMPROP_H
 
+#include <string>
+
 #include "gromacs/utility/basedefinitions.h"
 #include "gromacs/utility/classhelpers.h"
 #include "gromacs/utility/real.h"
@@ -66,7 +70,7 @@ class AtomProperties
          * \todo This should be made const once the lazy
          * implementation is done properly for the class.
          */
-        const char *elementFromAtomNumber(int atomNumber);
+        const std::string elementFromAtomNumber(int atomNumber);
         /*! \brief
          * Get atom number from element string.
          *
@@ -91,10 +95,10 @@ class AtomProperties
          * \param[out] value New value to set or default.
          * \returns If the operation has been succesful.
          */
-        bool setAtomProperty(int         eprop,
-                             const char* residueName,
-                             const char* atomName,
-                             real       *value);
+        bool setAtomProperty(int                eprop,
+                             const std::string &residueName,
+                             const std::string &atomName,
+                             real              *value);
         /*! \brief
          * Get handle to property.
          *