Move string array concatenation to format.h.

- Moved code from Options::setDescription() that concatenated strings
  from an array into format.h to make it more generally usable.
- Changed the code such that instead of requiring the array to be
  NULL-terminated, it can also deduce the length of the array passed as
  an argument.
- Made Options::setDescription() to take a simple string to allow using
  the alternative overload, and also to help in writing test code
  (for #666).

Change-Id: I96f45577fc6a4258a1c993983cc1d1a1a0846bdc

diff --git a/share/template/template.cpp b/share/template/template.cpp
index 25a084e814e9b0b5f0f8be1225d81804d90154ba..c3c6eb3b81481e95adfb85596fb35d0518e2e4bf 100644 (file)
--- a/share/template/template.cpp
+++ b/share/template/template.cpp
@@ -127,11 +127,10 @@ AnalysisTemplate::initOptions(TrajectoryAnalysisSettings *settings)
         "follow the instructions in the README file provided.",
         "This template implements a simple analysis programs that calculates",
         "average distances from a reference group to one or more",
-        "analysis groups.",
-        NULL
+        "analysis groups."
     };
 
-    options_.setDescription(desc);
+    options_.setDescription(concatenateStrings(desc));
 
     options_.addOption(FileNameOption("o")
         .filetype(eftPlot).outputFile()

diff --git a/src/gromacs/options/options.cpp b/src/gromacs/options/options.cpp
index b8be800fc4be44ef4cd2bfca332d12e3aed33348..c01c8b4c24f980ff6816525a2651a9c8097699f1 100644 (file)
--- a/src/gromacs/options/options.cpp
+++ b/src/gromacs/options/options.cpp
@@ -37,12 +37,10 @@
  */
 #include "gromacs/options/options.h"
 
-#include <cctype>
-#include <cstring>
-
 #include "gromacs/options/abstractoption.h"
 #include "gromacs/options/abstractoptionstorage.h"
 #include "gromacs/utility/exceptions.h"
+#include "gromacs/utility/format.h"
 #include "gromacs/utility/gmxassert.h"
 #include "gromacs/utility/messagestringcollector.h"
 
@@ -51,27 +49,6 @@
 namespace gmx
 {
 
-// TODO: Move this into the utility module
-static std::string composeString(const char *const *sarray)
-{
-    std::string result;
-
-    for (int i = 0; sarray[i] != NULL; ++i)
-    {
-        if (sarray[i][0] != '\0')
-        {
-            result.append(sarray[i]);
-            char lastchar = sarray[i][std::strlen(sarray[i])-1];
-            if (!std::isspace(lastchar))
-            {
-                result.append(" ");
-            }
-        }
-    }
-    result.resize(result.find_last_not_of(" \n\r\t") + 1);
-    return result;
-}
-
 /********************************************************************
  * Options::Impl
  */
@@ -156,9 +133,9 @@ const std::string &Options::description() const
     return _impl->_description;
 }
 
-void Options::setDescription(const char *const *desc)
+void Options::setDescription(const std::string &desc)
 {
-    _impl->_description = composeString(desc);
+    _impl->_description = desc;
 }
 
 void Options::addSubSection(Options *section)

diff --git a/src/gromacs/options/options.h b/src/gromacs/options/options.h
index 3d42d3a6815e78238721cb5aa22f78b06629bceb..cc99f57d7d4b16d39207a64fbde7080355a42189 100644 (file)
--- a/src/gromacs/options/options.h
+++ b/src/gromacs/options/options.h
@@ -113,14 +113,11 @@ class Options
         /*! \brief
          * Sets the full description of the option collection.
          *
-         * \param[in] desc Array of strings to form the description, terminated
-         *      by a NULL pointer.
+         * \param[in] desc  String to set as the description.
          *
-         * The strings in the \p desc array are concatenated to form the
-         * description, adding a single space between the strings if there is
-         * no whitespace in the end of a string.
+         * concatenateStrings() is useful for forming the input string.
          */
-        void setDescription(const char *const *desc);
+        void setDescription(const std::string &desc);
         //int addBugs(int nbugs, const char *const *bugs);
 
         /*! \brief

diff --git a/src/gromacs/trajectoryanalysis.h b/src/gromacs/trajectoryanalysis.h
index f9dbf482e968ab61fedb4e3fbe186d0bd483aa1f..368a991e2d6d4669c9bb5810a62ddf18aa2f6a6a 100644 (file)
--- a/src/gromacs/trajectoryanalysis.h
+++ b/src/gromacs/trajectoryanalysis.h
@@ -83,5 +83,6 @@
 #include "trajectoryanalysis/analysissettings.h"
 #include "trajectoryanalysis/cmdlinerunner.h"
 #include "utility/exceptions.h"
+#include "utility/format.h"
 
 #endif

diff --git a/src/gromacs/trajectoryanalysis/modules/angle.cpp b/src/gromacs/trajectoryanalysis/modules/angle.cpp
index d7e0fa1cb476a0af055ab4d939fe93ea6f2b8bfe..8c708feb962a6c73bc9ae9d401045cf8cb3f70c3 100644 (file)
--- a/src/gromacs/trajectoryanalysis/modules/angle.cpp
+++ b/src/gromacs/trajectoryanalysis/modules/angle.cpp
@@ -117,15 +117,14 @@ Angle::initOptions(TrajectoryAnalysisSettings *settings)
         "supported for static selections).",
         "[TT]-od[tt] can be used to dump all the individual angles,",
         "each on a separate line. This format is better suited for",
-        "further processing, e.g., if angles from multiple runs are needed.",
-        NULL
+        "further processing, e.g., if angles from multiple runs are needed."
     };
     static const char *const cGroup1TypeEnum[] =
         { "angle", "dihedral", "vector", "plane", NULL };
     static const char *const cGroup2TypeEnum[] =
         { "none", "vector", "plane", "t0", "z", "sphnorm", NULL };
 
-    _options.setDescription(desc);
+    _options.setDescription(concatenateStrings(desc));
 
     _options.addOption(FileNameOption("o").filetype(eftPlot).outputFile()
                            .store(&_fnAngle).defaultValueIfSet("angle"));

diff --git a/src/gromacs/trajectoryanalysis/modules/distance.cpp b/src/gromacs/trajectoryanalysis/modules/distance.cpp
index 3049862469d8061c95672618e603c66800e87c2a..9662fa51502acba917b48faaa51b696e5b533cda 100644 (file)
--- a/src/gromacs/trajectoryanalysis/modules/distance.cpp
+++ b/src/gromacs/trajectoryanalysis/modules/distance.cpp
@@ -53,6 +53,7 @@
 #include "gromacs/selection/selectionoption.h"
 #include "gromacs/trajectoryanalysis/analysissettings.h"
 #include "gromacs/utility/exceptions.h"
+#include "gromacs/utility/format.h"
 
 namespace gmx
 {
@@ -77,11 +78,10 @@ Distance::initOptions(TrajectoryAnalysisSettings *settings)
     static const char *const desc[] = {
         "g_dist can calculate the distance between two positions as",
         "a function of time. The total distance and its",
-        "x, y and z components are plotted.",
-        NULL
+        "x, y and z components are plotted."
     };
 
-    _options.setDescription(desc);
+    _options.setDescription(concatenateStrings(desc));
 
     _options.addOption(FileNameOption("o").filetype(eftPlot).outputFile()
                            .store(&_fnDist).defaultValue("dist"));

diff --git a/src/gromacs/trajectoryanalysis/modules/select.cpp b/src/gromacs/trajectoryanalysis/modules/select.cpp
index 400279be6d414a9e392ed3edd64c0caaa5611161..cb649a938558f2dbe63d908676ff1b5b4a6a9e8e 100644 (file)
--- a/src/gromacs/trajectoryanalysis/modules/select.cpp
+++ b/src/gromacs/trajectoryanalysis/modules/select.cpp
@@ -298,11 +298,10 @@ Select::initOptions(TrajectoryAnalysisSettings *settings)
         "one frame, and contains either 0/1 for each atom/residue/molecule",
         "possibly selected. 1 stands for the atom/residue/molecule being",
         "selected for the current frame, 0 for not selected.",
-        "With [TT]-dump[tt], the frame time is omitted from the output.",
-        NULL
+        "With [TT]-dump[tt], the frame time is omitted from the output."
     };
 
-    _options.setDescription(desc);
+    _options.setDescription(concatenateStrings(desc));
 
     _options.addOption(FileNameOption("os").filetype(eftPlot).outputFile()
                            .store(&_fnSize).defaultValueIfSet("size"));

diff --git a/src/gromacs/trajectoryanalysis/tests/test_selection.cpp b/src/gromacs/trajectoryanalysis/tests/test_selection.cpp
index 6c8d3d0b3160193d5e564da3f56d3c83123070c0..61ceca062f6d313ec04c6814ef53483ae4aea2f9 100644 (file)
--- a/src/gromacs/trajectoryanalysis/tests/test_selection.cpp
+++ b/src/gromacs/trajectoryanalysis/tests/test_selection.cpp
@@ -45,6 +45,7 @@
 #include <gromacs/trajectoryanalysis/analysissettings.h>
 #include <gromacs/trajectoryanalysis/cmdlinerunner.h>
 #include <gromacs/utility/exceptions.h>
+#include <gromacs/utility/format.h>
 
 namespace gmx
 {
@@ -98,11 +99,10 @@ Options &
 SelectionTester::initOptions(TrajectoryAnalysisSettings * /*settings*/)
 {
     static const char *const desc[] = {
-        "This is a test program for selections.",
-        NULL
+        "This is a test program for selections."
     };
 
-    _options.setDescription(desc);
+    _options.setDescription(concatenateStrings(desc));
 
     _options.addOption(SelectionOption("select").storeVector(&_selections)
                            .required().multiValue().allowMultiple()

diff --git a/src/gromacs/utility/format.cpp b/src/gromacs/utility/format.cpp
index 0ee8026b8b742cdb1997defca151e606c00fa2ec..603539c036aab8571165f4c8f0bbdf8446fc4b66 100644 (file)
--- a/src/gromacs/utility/format.cpp
+++ b/src/gromacs/utility/format.cpp
@@ -37,8 +37,10 @@
  */
 #include "gromacs/utility/format.h"
 
+#include <cctype>
 #include <cstdio>
 #include <cstdarg>
+#include <cstring>
 
 #include <algorithm>
 #include <string>
@@ -82,6 +84,26 @@ std::string formatString(const char *fmt, ...)
     }
 }
 
+std::string concatenateStrings(const char *const *sarray, size_t count)
+{
+    std::string result;
+
+    for (size_t i = 0; i < count && sarray[i] != NULL; ++i)
+    {
+        if (sarray[i][0] != '\0')
+        {
+            result.append(sarray[i]);
+            char lastchar = sarray[i][std::strlen(sarray[i])-1];
+            if (!std::isspace(lastchar))
+            {
+                result.append(" ");
+            }
+        }
+    }
+    result.resize(result.find_last_not_of(" \n\r\t") + 1);
+    return result;
+}
+
 /********************************************************************
  * TextLineWrapper::Impl
  */

diff --git a/src/gromacs/utility/format.h b/src/gromacs/utility/format.h
index cd3464ac09ad4b0363fe318b64841698c5cdb24c..a6650a7dd3084901973d372092bd1d3292ba5cd1 100644 (file)
--- a/src/gromacs/utility/format.h
+++ b/src/gromacs/utility/format.h
@@ -60,6 +60,41 @@ namespace gmx
  */
 std::string formatString(const char *fmt, ...);
 
+/*! \brief
+ * Joins strings in an array to a single string.
+ *
+ * \param[in] sarray  Array of strings to concatenate.
+ * \param[in] count   Number of elements in \p sarray to concatenate.
+ * \returns   All strings in \p sarray joined, ensuring at least one space
+ *      between the strings.
+ * \throws    std::bad_alloc if out of memory.
+ *
+ * The strings in the \p sarray array are concatenated, adding a single space
+ * between the strings if there is no whitespace in the end of a string.
+ * Terminal whitespace is removed.
+ *
+ * \inpublicapi
+ */
+std::string concatenateStrings(const char * const *sarray, size_t count);
+/*! \brief
+ * Convenience overload for joining strings in a C array (static data).
+ *
+ * \param[in] sarray  Array of strings to concatenate.
+ * \tparam    count   Deduced number of elements in \p sarray.
+ * \returns   All strings in \p sarray joined, ensuring at least one space
+ *      between the strings.
+ * \throws    std::bad_alloc if out of memory.
+ *
+ * \see concatenateStrings(const char * const *, size_t)
+ *
+ * \inpublicapi
+ */
+template <size_t count>
+std::string concatenateStrings(const char * const (&sarray)[count])
+{
+    return concatenateStrings(sarray, count);
+}
+
 /*! \brief
  * Wraps lines to a predefined length.
  *

diff --git a/src/gromacs/utility/tests/format.cpp b/src/gromacs/utility/tests/format.cpp
index e717211b4b15b6680023dfcbb92f3a0de69eab33..cb881fd1fb0bae4ecaffe2933fd13089d9085e5f 100644 (file)
--- a/src/gromacs/utility/tests/format.cpp
+++ b/src/gromacs/utility/tests/format.cpp
@@ -121,6 +121,24 @@ TEST(FormatStringTest, HandlesLongStrings)
     EXPECT_EQ("x10", longString.substr(1999));
 }
 
+/********************************************************************
+ * Tests for concatenateStrings()
+ */
+
+typedef FormatTestBase ConcatenateStringsTest;
+
+TEST_F(ConcatenateStringsTest, HandlesDifferentStringEndings)
+{
+    static const char * const strings[] = {
+        "First string",
+        "Second string ",
+        "Third string\n",
+        "Fourth string",
+        ""
+    };
+    checkFormatting(gmx::concatenateStrings(strings), "CombinedStrings");
+}
+
 /********************************************************************
  * Tests for TextLineWrapper
  */

diff --git a/src/gromacs/utility/tests/refdata/ConcatenateStringsTest_HandlesDifferentStringEndings.xml b/src/gromacs/utility/tests/refdata/ConcatenateStringsTest_HandlesDifferentStringEndings.xml
new file mode 100644 (file)
index 0000000..1d9ba80
--- /dev/null
+++ b/src/gromacs/utility/tests/refdata/ConcatenateStringsTest_HandlesDifferentStringEndings.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="CombinedStrings"><![CDATA[
+First string Second string Third string
+Fourth string]]></String>
+</ReferenceData>