Clean up option (default) value access

Currently, the only user for valueCount() and formatValue() in
OptionInfo was actually interested in the default values, and limiting
access to the default values only makes the responsibilities clearer.
So replace both methods with a single method that returns just the
default values.  The internal implementation currently places some
limitations on when this can actually be called, but that should not be
a problem for current or future uses.

Change-Id: I3e68ce397ec3f40f62ececad4ee6b87d0ecc790e

diff --git a/src/gromacs/commandline/cmdlinehelpwriter.cpp b/src/gromacs/commandline/cmdlinehelpwriter.cpp
index 1ecd452d7e5d7933d40297aaf268805f1178d129..4756ebe4a8b2524ba2b24ef553214c5568bec63c 100644 (file)
--- a/src/gromacs/commandline/cmdlinehelpwriter.cpp
+++ b/src/gromacs/commandline/cmdlinehelpwriter.cpp
@@ -248,27 +248,9 @@ void formatOptionNameAndValue(const OptionInfo &option, std::string *name,
 }
 
 //! Formats the default option value as a string.
-std::string
-defaultOptionValue(const OptionInfo &option)
+std::string defaultOptionValue(const OptionInfo &option)
 {
-    if (option.valueCount() == 0
-        || (option.valueCount() == 1 && option.formatValue(0).empty()))
-    {
-        return option.formatDefaultValueIfSet();
-    }
-    else
-    {
-        std::string result;
-        for (int i = 0; i < option.valueCount(); ++i)
-        {
-            if (i != 0)
-            {
-                result.append(" ");
-            }
-            result.append(option.formatValue(i));
-        }
-        return result;
-    }
+    return joinStrings(option.defaultValuesAsStrings(), " ");
 }
 
 //! Formats the flags for a file option as a string.

diff --git a/src/gromacs/options/abstractoption.cpp b/src/gromacs/options/abstractoption.cpp
index b35eafd1cca8fb68ffa55f5823998872c2e402cf..d7f38d1d86d566f731b8108e3e9b8a0ebaea9232 100644 (file)
--- a/src/gromacs/options/abstractoption.cpp
+++ b/src/gromacs/options/abstractoption.cpp
@@ -261,19 +261,9 @@ std::string OptionInfo::formatDescription() const
     return description;
 }
 
-std::string OptionInfo::formatDefaultValueIfSet() const
+std::vector<std::string> OptionInfo::defaultValuesAsStrings() const
 {
-    return option().formatDefaultValueIfSet();
-}
-
-int OptionInfo::valueCount() const
-{
-    return option().valueCount();
-}
-
-std::string OptionInfo::formatValue(int i) const
-{
-    return option().formatValue(i);
+    return option().defaultValuesAsStrings();
 }
 
 } // namespace gmx

diff --git a/src/gromacs/options/abstractoption.h b/src/gromacs/options/abstractoption.h
index fc1723cea59d86f884d7ad0e317209ac5c117d1b..8d3ffcc686531e34de649efffc12868d09797ce0 100644 (file)
--- a/src/gromacs/options/abstractoption.h
+++ b/src/gromacs/options/abstractoption.h
@@ -485,17 +485,17 @@ class OptionInfo
         std::string type() const;
         //! Returns the description of the option.
         std::string formatDescription() const;
+
         /*! \brief
-         * Returns the default value if set for the option as a string.
+         * Returns the default value(s) of the option as strings.
+         *
+         * If there is no default value, but defaultValueIfSet() is set, that
+         * is returned instead.
          *
-         * \see OptionTemplate::defaultValueIfSet()
+         * Currently, this can only be called before option values have been
+         * assigned.
          */
-        std::string formatDefaultValueIfSet() const;
-
-        //! Returns the number of values given for the option.
-        int valueCount() const;
-        //! Returns the i'th value of the option as a string.
-        std::string formatValue(int i) const;
+        std::vector<std::string> defaultValuesAsStrings() const;
 
     protected:
         /*! \cond libapi */

diff --git a/src/gromacs/options/abstractoptionstorage.h b/src/gromacs/options/abstractoptionstorage.h
index 889614542662b3821df0984df5dd76a2ab27d3d6..84a5985208798181b625532b99163d4bbb3da344 100644 (file)
--- a/src/gromacs/options/abstractoptionstorage.h
+++ b/src/gromacs/options/abstractoptionstorage.h
@@ -44,6 +44,7 @@
 #define GMX_OPTIONS_ABSTRACTOPTIONSTORAGE_H
 
 #include <string>
+#include <vector>
 
 #include "gromacs/options/optionflags.h"
 #include "gromacs/utility/classhelpers.h"
@@ -138,16 +139,8 @@ class AbstractOptionStorage
          * Returns the number of option values added so far.
          */
         virtual int valueCount() const = 0;
-        /*! \brief
-         * Returns the i'th value formatted as a string.
-         *
-         * If \p i is DefaultValueIfSetIndex, should format the default value
-         * if set (see OptionTemplate::defaultValueIfSet()).
-         */
-        virtual std::string formatValue(int i) const = 0;
-        //! \copydoc OptionInfo::formatDefaultValueIfSet()
-        std::string formatDefaultValueIfSet() const
-        { return formatValue(DefaultValueIfSetIndex); }
+        //! \copydoc OptionInfo::defaultValuesAsStrings()
+        virtual std::vector<std::string> defaultValuesAsStrings() const = 0;
 
         /*! \brief
          * Starts adding values from a new source for the option.
@@ -206,9 +199,6 @@ class AbstractOptionStorage
         void finish();
 
     protected:
-        //! Index used with formatValue() for formatting default value if set.
-        static const int DefaultValueIfSetIndex = -1;
-
         /*! \brief
          * Initializes the storage object from the settings object.
          *

diff --git a/src/gromacs/options/optionstoragetemplate.h b/src/gromacs/options/optionstoragetemplate.h
index 796484ef4aecaf63f62cb9da1b0f4295855d8053..f3750c6bcf5f33cfff50b477c013bd143c7b4d92 100644 (file)
--- a/src/gromacs/options/optionstoragetemplate.h
+++ b/src/gromacs/options/optionstoragetemplate.h
@@ -67,13 +67,13 @@ class Options;
  *
  * \tparam T Assignable type that stores a single option value.
  *
- * Provides an implementation of the clearSet(), valueCount(), and processSet()
- * methods of AbstractOptionStorage, as well as a basic no-action
- * implementation of processAll().  One new virtual method is added:
- * processSetValues().  The default implementation does nothing.
- * This leaves typeString(), formatValue(), and convertValue() to be
- * implemented in derived classes.  processSetValues() and processAll() can
- * also be implemented if necessary.
+ * Provides an implementation of the clearSet(), valueCount(), processSet(),
+ * and defaultValuesAsStrings() methods of AbstractOptionStorage, as well as a
+ * basic no-action implementation of processAll().  Two new virtual methods are
+ * added: processSetValues() and formatSingleValue().
+ * This leaves typeString(), convertValue() and formatStringValue() to be
+ * implemented in derived classes.
+ * processSetValues() and processAll() can also be implemented if necessary.
  *
  * Implements transaction support for adding values within a set: all calls to
  * addValue() add the value to a temporary storage, processSetValues() operates
@@ -100,14 +100,14 @@ class OptionStorageTemplate : public AbstractOptionStorage
         virtual std::string typeString() const = 0;
         //! \copydoc gmx::AbstractOptionStorage::valueCount()
         virtual int valueCount() const { return store_->valueCount(); }
-        /*! \copydoc gmx::AbstractOptionStorage::formatValue()
+        /*! \copydoc gmx::AbstractOptionStorage::defaultValuesAsStrings()
          *
-         * OptionStorageTemplate implements handling of DefaultValueIfSetIndex
-         * in this method, as well as checking that \p i is a valid index.
+         * OptionStorageTemplate implements handling of defaultValueIfSet()
+         * cases and composing the vector.
          * Derived classes must implement formatSingleValue() to provide the
          * actual formatting for a value of type \p T.
          */
-        virtual std::string formatValue(int i) const;
+        virtual std::vector<std::string> defaultValuesAsStrings() const;
 
     protected:
         //! Smart pointer for managing the final storage interface.
@@ -189,7 +189,8 @@ class OptionStorageTemplate : public AbstractOptionStorage
          * \returns   \p value formatted as a string.
          *
          * The derived class must provide this method to format values a
-         * strings.  Called by formatValue() to do the actual formatting.
+         * strings.  Called by defaultValuesAsStrings() to do the actual
+         * formatting.
          */
         virtual std::string formatSingleValue(const T &value) const = 0;
 
@@ -436,19 +437,28 @@ std::unique_ptr<IOptionValueStore<T> > OptionStorageTemplate<T>::createStore(
 
 
 template <typename T>
-std::string OptionStorageTemplate<T>::formatValue(int i) const
+std::vector<std::string> OptionStorageTemplate<T>::defaultValuesAsStrings() const
 {
-    GMX_RELEASE_ASSERT(i == DefaultValueIfSetIndex || (i >= 0 && i < valueCount()),
-                       "Invalid value index");
-    if (i == DefaultValueIfSetIndex)
+    std::vector<std::string> result;
+    if (hasFlag(efOption_NoDefaultValue))
+    {
+        return result;
+    }
+    GMX_RELEASE_ASSERT(hasFlag(efOption_HasDefaultValue),
+                       "Current option implementation can only provide default values before assignment");
+    for (const auto &value : values())
+    {
+        result.push_back(formatSingleValue(value));
+    }
+    if (result.empty() || (result.size() == 1 && result[0].empty()))
     {
+        result.clear();
         if (defaultValueIfSet_.get() != NULL)
         {
-            return formatSingleValue(*defaultValueIfSet_);
+            result.push_back(formatSingleValue(*defaultValueIfSet_));
         }
-        return std::string();
     }
-    return formatSingleValue(values()[i]);
+    return result;
 }
 
 

diff --git a/src/gromacs/selection/selectionfileoptionstorage.h b/src/gromacs/selection/selectionfileoptionstorage.h
index 99c96accac2cf0a01bc7e6fa4f35d0c67e03a7e6..16fa84964e38f287869543d6aa0c33a951f83e67 100644 (file)
--- a/src/gromacs/selection/selectionfileoptionstorage.h
+++ b/src/gromacs/selection/selectionfileoptionstorage.h
@@ -71,7 +71,7 @@ class SelectionFileOptionStorage : public AbstractOptionStorage
         virtual OptionInfo &optionInfo() { return info_; }
         virtual std::string typeString() const { return "file"; }
         virtual int valueCount() const { return 0; }
-        virtual std::string formatValue(int /*i*/) const { return ""; }
+        virtual std::vector<std::string> defaultValuesAsStrings() const { return {}; }
 
     private:
         virtual void clearSet();