Add EnumOption with support for proper enum types
authorTeemu Murtola <teemu.murtola@gmail.com>
Fri, 21 Aug 2015 10:10:31 +0000 (13:10 +0300)
committerGerrit Code Review <gerrit@gerrit.gromacs.org>
Sun, 15 Nov 2015 19:05:41 +0000 (20:05 +0100)
Generalize EnumIntOption such that it can now store the output into a
proper enum type, making it easier to write code where the compiler can
check for correct usage of the enums (i.e., give an error for
unintentional mixing of enum types, or for implicit int-to-enum casts).

Change-Id: Idcdd671f555327a3e3dad521b92d50930cb6e281

15 files changed:
src/gromacs/commandline/tests/cmdlinehelpwriter.cpp
src/gromacs/gmxpreprocess/insert-molecules.cpp
src/gromacs/options/abstractoption.h
src/gromacs/options/basicoptions.cpp
src/gromacs/options/basicoptions.h
src/gromacs/options/basicoptionstorage.h
src/gromacs/options/optionstoragetemplate.h
src/gromacs/options/tests/optionsassigner.cpp
src/gromacs/options/timeunitmanager.cpp
src/gromacs/options/timeunitmanager.h
src/gromacs/trajectoryanalysis/modules/angle.cpp
src/gromacs/trajectoryanalysis/modules/pairdist.cpp
src/gromacs/trajectoryanalysis/modules/rdf.cpp
src/gromacs/trajectoryanalysis/modules/select.cpp
src/testutils/refdata.cpp

index 80b2d7dddbac46438f1064f570b2e12efa9ebad9..c92d8255e6f56d705389646f0739f09d3c2f138f 100644 (file)
@@ -183,9 +183,9 @@ TEST_F(CommandLineHelpWriterTest, HandlesDefaultValuesFromVariables)
     options.addOption(StringOption("str").description("String option")
                           .storeVector(&svalues).multiValue());
 
-    int               evalue    = eBar;
+    TestEnum          evalue    = eBar;
     const char *const allowed[] = { "foo", "bar" };
-    options.addOption(EnumIntOption("enum").description("Enum option")
+    options.addOption(EnumOption<TestEnum>("enum").description("Enum option")
                           .enumValue(allowed).store(&evalue));
 
     CommandLineHelpWriter writer(options);
index 88e1e8d80a0e47346763ec63c9b22dad463d5439..ebfec94fb1d62411e57bd7d8378fbbbd7095df9d 100644 (file)
 using gmx::RVec;
 
 /* enum for random rotations of inserted solutes */
-enum {
+enum RotationType {
     en_rotXYZ, en_rotZ, en_rotNone
 };
+const char *const cRotationEnum[] = {"xyz", "z", "none"};
 
 static void center_molecule(std::vector<RVec> *x)
 {
@@ -89,8 +90,8 @@ static void center_molecule(std::vector<RVec> *x)
 }
 
 static void generate_trial_conf(const std::vector<RVec> &xin,
-                                const rvec offset, int enum_rot, gmx_rng_t rng,
-                                std::vector<RVec> *xout)
+                                const rvec offset, RotationType enum_rot,
+                                gmx_rng_t rng, std::vector<RVec> *xout)
 {
     *xout = xin;
     real alfa = 0.0, beta = 0.0, gamma = 0.0;
@@ -109,7 +110,7 @@ static void generate_trial_conf(const std::vector<RVec> &xin,
             alfa = beta = gamma = 0.;
             break;
     }
-    if (enum_rot == en_rotXYZ || (enum_rot == en_rotZ))
+    if (enum_rot == en_rotXYZ || enum_rot == en_rotZ)
     {
         rotate_conf(xout->size(), as_rvec_array(xout->data()), NULL, alfa, beta, gamma);
     }
@@ -144,7 +145,8 @@ static void insert_mols(int nmol_insrt, int ntry, int seed,
                         t_topology *top, std::vector<RVec> *x,
                         const t_atoms &atoms_insrt, const std::vector<RVec> &x_insrt,
                         int ePBC, matrix box,
-                        const std::string &posfn, const rvec deltaR, int enum_rot)
+                        const std::string &posfn, const rvec deltaR,
+                        RotationType enum_rot)
 {
     fprintf(stderr, "Initialising inter-atomic distances...\n");
     gmx_atomprop_t          aps = gmx_atomprop_init();
@@ -290,19 +292,19 @@ class InsertMolecules : public ICommandLineOptionsModule
         virtual int run();
 
     private:
-        std::string inputConfFile_;
-        std::string insertConfFile_;
-        std::string positionFile_;
-        std::string outputConfFile_;
-        rvec        newBox_;
-        bool        bBox_;
-        int         nmolIns_;
-        int         nmolTry_;
-        int         seed_;
-        real        defaultDistance_;
-        real        scaleFactor_;
-        rvec        deltaR_;
-        int         enumRot_;
+        std::string   inputConfFile_;
+        std::string   insertConfFile_;
+        std::string   positionFile_;
+        std::string   outputConfFile_;
+        rvec          newBox_;
+        bool          bBox_;
+        int           nmolIns_;
+        int           nmolTry_;
+        int           seed_;
+        real          defaultDistance_;
+        real          scaleFactor_;
+        rvec          deltaR_;
+        RotationType  enumRot_;
 };
 
 void InsertMolecules::initOptions(IOptionsContainer                 *options,
@@ -388,8 +390,7 @@ void InsertMolecules::initOptions(IOptionsContainer                 *options,
     options->addOption(RealOption("dr").vector()
                            .store(deltaR_)
                            .description("Allowed displacement in x/y/z from positions in [TT]-ip[tt] file"));
-    const char *const cRotationEnum[] = {"xyz", "z", "none"};
-    options->addOption(EnumIntOption("rot").enumValue(cRotationEnum)
+    options->addOption(EnumOption<RotationType>("rot").enumValue(cRotationEnum)
                            .store(&enumRot_)
                            .description("Rotate inserted molecules randomly"));
 }
index 47276f214b459a6ee20e595ddbfec4f9fb9fe5dc..ff8a2596865374f82d1ba577d3ca34e934f16c09 100644 (file)
@@ -373,6 +373,14 @@ class OptionTemplate : public AbstractOption
          * is none.
          */
         const T *defaultValueIfSet() const { return defaultValueIfSet_; }
+        /*! \brief
+         * Returns a pointer to the storage location, or NULL if none specified.
+         */
+        T *store() const { return store_; }
+        /*! \brief
+         * Returns a pointer to the storage vector, or NULL if none specified.
+         */
+        std::vector<T> *storeVector() const { return storeVector_; }
         //! Returns \p *this casted into MyClass to reduce typing.
         MyClass &me() { return static_cast<MyClass &>(*this); }
         //! \endcond
index 3c2e6d310cb09279d3b550d4d4e4ab237623cf61..1a09d40569fbd53474e2d370ef55982a5956613f 100644 (file)
@@ -47,6 +47,7 @@
 #include <cstdio>
 #include <cstdlib>
 
+#include <algorithm>
 #include <limits>
 #include <string>
 #include <vector>
@@ -613,42 +614,51 @@ StringOption::createStorage(const OptionManagerContainer & /*managers*/) const
  * EnumOptionStorage
  */
 
-EnumOptionStorage::EnumOptionStorage(const EnumIntOption &settings)
-    : MyBase(settings), info_(this)
+EnumOptionStorage::EnumOptionStorage(const AbstractOption &settings,
+                                     const char *const *enumValues, int count,
+                                     int defaultValue, int defaultValueIfSet,
+                                     EnumIndexStorePointer store)
+    : MyBase(settings), info_(this), store_(move(store))
 {
-    if (settings.enumValues_ == NULL)
+    if (enumValues == NULL)
     {
-        GMX_THROW(APIError("Allowed values must be provided to EnumIntOption"));
+        GMX_THROW(APIError("Allowed values must be provided to EnumOption"));
     }
 
-    int count = settings.enumValuesCount_;
     if (count < 0)
     {
         count = 0;
-        while (settings.enumValues_[count] != NULL)
+        while (enumValues[count] != NULL)
         {
             ++count;
         }
     }
     for (int i = 0; i < count; ++i)
     {
-        if (settings.enumValues_[i] == NULL)
+        if (enumValues[i] == NULL)
         {
             GMX_THROW(APIError("Enumeration value cannot be NULL"));
         }
-        allowed_.push_back(settings.enumValues_[i]);
+        allowed_.push_back(enumValues[i]);
     }
 
-    const int *const defaultValue = settings.defaultValue();
-    if (defaultValue != NULL)
+    GMX_ASSERT(defaultValue < count, "Default enumeration value is out of range");
+    GMX_ASSERT(defaultValueIfSet < count, "Default enumeration value is out of range");
+    setFlag(efOption_HasDefaultValue);
+    if (defaultValue >= 0)
     {
-        GMX_ASSERT(*defaultValue < count, "Default enumeration value is out of range");
+        setDefaultValue(defaultValue);
     }
-    const int *const defaultValueIfSet = settings.defaultValueIfSet();
-    if (defaultValueIfSet != NULL)
+    if (defaultValueIfSet >= 0)
     {
-        GMX_ASSERT(*defaultValueIfSet < count, "Default enumeration value is out of range");
+        setDefaultValueIfSet(defaultValueIfSet);
     }
+
+    if (values().empty())
+    {
+        values() = store_->initialValues();
+    }
+    refreshEnumIndexStore();
 }
 
 std::string EnumOptionStorage::formatExtraDescription() const
@@ -674,6 +684,24 @@ void EnumOptionStorage::convertValue(const std::string &value)
     addValue(match - allowed_.begin());
 }
 
+void EnumOptionStorage::processSetValues(ValueList *values)
+{
+    const size_t newSize = (hasFlag(efOption_ClearOnNextSet) ? 0 : valueCount())
+        + std::max<size_t>(values->size(), 1);
+    store_->reserveSpace(newSize);
+}
+
+void EnumOptionStorage::refreshValues()
+{
+    MyBase::refreshValues();
+    refreshEnumIndexStore();
+}
+
+void EnumOptionStorage::refreshEnumIndexStore()
+{
+    store_->refreshValues(values());
+}
+
 /********************************************************************
  * EnumOptionInfo
  */
@@ -694,13 +722,29 @@ const std::vector<std::string> &EnumOptionInfo::allowedValues() const
 }
 
 /********************************************************************
- * EnumIntOption
+ * EnumOption helpers
  */
 
+namespace internal
+{
+
+EnumIndexStoreInterface::~EnumIndexStoreInterface()
+{
+}
+
+//! \cond internal
 AbstractOptionStorage *
-EnumIntOption::createStorage(const OptionManagerContainer & /*managers*/) const
+createEnumOptionStorage(const AbstractOption &option,
+                        const char *const *enumValues, int count,
+                        int defaultValue, int defaultValueIfSet,
+                        EnumIndexStoreInterface *store)
 {
-    return new EnumOptionStorage(*this);
+    EnumOptionStorage::EnumIndexStorePointer storePtr(store);
+    return new EnumOptionStorage(option, enumValues, count, defaultValue,
+                                 defaultValueIfSet, move(storePtr));
 }
+//! \endcond
+
+} // namespace internal
 
 } // namespace gmx
index 6a733b924053394b984e006ab2c2e53f6cdcc65c..a2b3360624e2ff6bc221061f0c1b396600f4c505 100644 (file)
@@ -369,19 +369,150 @@ class StringOption : public OptionTemplate<std::string, StringOption>
         friend class StringOptionStorage;
 };
 
+//! \}
+
+namespace internal
+{
+
+/*! \internal
+ * \brief
+ * Interface for handling storage of the enum indexes.
+ *
+ * This interface acts as a proxy between the EnumOptionStorage class (that
+ * operates on `int` values), and the actual enum variable that receives the
+ * values.  The implementation of this interface takes care of conversion of
+ * the values and writing them out into the actual enum variables.
+ *
+ * \ingroup module_options
+ */
+class EnumIndexStoreInterface
+{
+    public:
+        virtual ~EnumIndexStoreInterface();
+
+        //! Returns initial values from the actual enum variables.
+        virtual std::vector<int> initialValues() const = 0;
+        //! Reserves space for storage in the actual enum variables.
+        virtual void reserveSpace(size_t count) = 0;
+        //! Updates values in the actual enum variables based on option values.
+        virtual void refreshValues(const std::vector<int> &values) = 0;
+};
+
+/*! \internal
+ * \brief
+ * Type-specific implementation for EnumIndexStoreInterface.
+ *
+ * This class is instantiated for each enum type for which EnumOption is used,
+ * and takes care of managing the `int`-to-`enum` conversions as described in
+ * EnumIndexStoreInterface.  Having this as a template in the header allows the
+ * actual storage implementation to not be in the header, which would require
+ * exposing all the internals through this one header...
+ *
+ * \ingroup module_options
+ */
+template <typename EnumType>
+class EnumIndexStore : public EnumIndexStoreInterface
+{
+    public:
+        //! Initializes the storage for the given actual enum variables.
+        EnumIndexStore(EnumType *store, std::vector<EnumType> *storeVector)
+            : store_(store), storeVector_(storeVector)
+        {
+        }
+
+        virtual std::vector<int> initialValues() const
+        {
+            std::vector<int> result;
+            if (storeVector_ != NULL)
+            {
+                typename std::vector<EnumType>::const_iterator i;
+                for (i = storeVector_->begin(); i != storeVector_->end(); ++i)
+                {
+                    result.push_back(*i);
+                }
+            }
+            else if (store_ != NULL)
+            {
+                // TODO: Copy more than one value if that would make sense.
+                result.push_back(store_[0]);
+            }
+            return result;
+        }
+        virtual void reserveSpace(size_t count)
+        {
+            if (storeVector_ != NULL)
+            {
+                storeVector_->reserve(count);
+            }
+        }
+        virtual void refreshValues(const std::vector<int> &values)
+        {
+            if (store_ != NULL)
+            {
+                for (size_t i = 0; i < values.size(); ++i)
+                {
+                    store_[i] = static_cast<EnumType>(values[i]);
+                }
+            }
+            if (storeVector_ != NULL)
+            {
+                GMX_ASSERT(storeVector_->capacity() >= values.size(),
+                           "reserveSpace() should have been called earlier");
+                storeVector_->resize(values.size());
+                for (size_t i = 0; i < values.size(); ++i)
+                {
+                    (*storeVector_)[i] = static_cast<EnumType>(values[i]);
+                }
+            }
+        }
+
+    private:
+        EnumType              *store_;
+        std::vector<EnumType> *storeVector_;
+};
+
+//! \cond internal
+/*! \internal
+ * \brief
+ * Helper to create EnumOptionStorage instances.
+ *
+ * This function works as a proxy between EnumOption::createStorage() and the
+ * EnumOptionStorage constructor, such that the latter does not need to be
+ * exposed in the header.
+ *
+ * \ingroup module_options
+ */
+AbstractOptionStorage *
+createEnumOptionStorage(const AbstractOption &option,
+                        const char *const *enumValues, int count,
+                        int defaultValue, int defaultValueIfSet,
+                        EnumIndexStoreInterface *store);
+//! \endcond
+
+}   // namespace internal
+
+//! \addtogroup module_options
+//! \{
+
 /*! \brief
  * Specifies an option that accepts enumerated string values and writes the
- * selected index into an integer variable.
+ * selected index into an `enum` variable.
+ *
+ * \tparam EnumType  Type of the variable that receives the values
+ *     (can also be `int`).
  *
  * Examples:
  * \code
    enum MyEnum { eAtom, eRes, eMol };
-   using gmx::EnumIntOption;
+   using gmx::EnumOption;
    const char * const  allowed[] = { "atom", "residue", "molecule" };
-   int          value = eAtom; // default value
-   options.addOption(EnumIntOption("type").enumValue(allowed).store(&value));
+   MyEnum       value = eAtom; // default value
+   options.addOption(EnumOption<MyEnum>("type").enumValue(allowed).store(&value));
  * \endcode
  *
+ * storeCount() is not currently implemented for this option type, and
+ * providing multiple default values through an array passed to store() does
+ * not work consistently in all cases.
  * In the current implementation, the values of the enum type should correspond
  * to indices in the array passed to enumValue(), i.e., be consencutive
  * starting from zero.  Only values corresponding to valid indices are accepted
@@ -391,19 +522,22 @@ class StringOption : public OptionTemplate<std::string, StringOption>
  *
  * Public methods in this class do not throw.
  *
- * \todo
- * Implement a variant that accepts proper enum types.
- *
  * \inpublicapi
  */
-class EnumIntOption : public OptionTemplate<int, EnumIntOption>
+template <typename EnumType>
+class EnumOption : public OptionTemplate<EnumType, EnumOption<EnumType> >
 {
     public:
         //! OptionInfo subclass corresponding to this option type.
         typedef EnumOptionInfo InfoType;
 
+        // This needs to be duplicated from OptionTemplate because this class
+        // is a template.
+        //! Short-hand for the base class.
+        typedef OptionTemplate<EnumType, EnumOption<EnumType> > MyBase;
+
         //! Initializes an option with the given name.
-        explicit EnumIntOption(const char *name)
+        explicit EnumOption(const char *name)
             : MyBase(name), enumValues_(NULL), enumValuesCount_(0)
         {
         }
@@ -420,7 +554,7 @@ class EnumIntOption : public OptionTemplate<int, EnumIntOption>
          * The strings are copied once the option is created.
          */
         template <size_t count>
-        EnumIntOption &enumValue(const char *const (&values)[count])
+        EnumOption &enumValue(const char *const (&values)[count])
         {
             GMX_ASSERT(enumValues_ == NULL,
                        "Multiple sets of enumerated values specified");
@@ -440,7 +574,7 @@ class EnumIntOption : public OptionTemplate<int, EnumIntOption>
          *
          * \see enumValue()
          */
-        EnumIntOption &enumValueFromNullTerminatedArray(const char *const *values)
+        EnumOption &enumValueFromNullTerminatedArray(const char *const *values)
         {
             GMX_ASSERT(enumValues_ == NULL,
                        "Multiple sets of enumerated values specified");
@@ -450,9 +584,24 @@ class EnumIntOption : public OptionTemplate<int, EnumIntOption>
         }
 
     private:
+        //! Helper function to convert default values for storate initialization.
+        static int convertToInt(const EnumType *defaultValue)
+        {
+            return defaultValue != NULL ? static_cast<int>(*defaultValue) : -1;
+        }
+
         //! Creates a EnumOptionStorage object.
         virtual AbstractOptionStorage *createStorage(
-            const OptionManagerContainer & /*managers*/) const;
+            const OptionManagerContainer & /*managers*/) const
+        {
+            // TODO: Implement storeCount() if necessary.
+            return internal::createEnumOptionStorage(
+                    *this, enumValues_, enumValuesCount_,
+                    convertToInt(MyBase::defaultValue()),
+                    convertToInt(MyBase::defaultValueIfSet()),
+                    new internal::EnumIndexStore<EnumType>(
+                            MyBase::store(), MyBase::storeVector()));
+        }
 
         const char *const      *enumValues_;
         int                     enumValuesCount_;
@@ -464,6 +613,9 @@ class EnumIntOption : public OptionTemplate<int, EnumIntOption>
         friend class EnumOptionStorage;
 };
 
+//! Shorthand for an enumerated option that stores into an `int` variable.
+typedef EnumOption<int> EnumIntOption;
+
 /*! \brief
  * Wrapper class for accessing boolean option information.
  *
index 41c2a2849eb14b3a252fe48b6ea8b141f7fd990f..a4aa6ead9f3c448bf6e9ac86f786dd171d4b5f21 100644 (file)
@@ -42,6 +42,7 @@
 #ifndef GMX_OPTIONS_BASICOPTIONSTORAGE_H
 #define GMX_OPTIONS_BASICOPTIONSTORAGE_H
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -215,8 +216,30 @@ class StringOptionStorage : public OptionStorageTemplate<std::string>
 class EnumOptionStorage : public OptionStorageTemplate<int>
 {
     public:
-        //! \copydoc DoubleOptionStorage::DoubleOptionStorage()
-        explicit EnumOptionStorage(const EnumIntOption &settings);
+        //! Shorthand for the enum index storage interface.
+        typedef std::unique_ptr<internal::EnumIndexStoreInterface>
+            EnumIndexStorePointer;
+
+        /*! \brief
+         * Initializes the storage from option settings.
+         *
+         * \param[in] settings      Basic storage settings.
+         * \param[in] enumValues    Allowed values.
+         * \param[in] count         Number of elements in \p enumValues,
+         *     or -1 if \p enumValues is `NULL`-terminated.
+         * \param[in] defaultValue  Default value, or -1 if no default.
+         * \param[in] defaultValueIfSet  Default value if set, or -1 if none.
+         * \param[in] store         Storage to convert the values to/from `int`.
+         *
+         * This constructor takes more parameters than other storage parameters
+         * because the front-end option type is a template, and as such cannot
+         * be passed here without exposing also this header as an installed
+         * header.
+         */
+        EnumOptionStorage(const AbstractOption &settings,
+                          const char *const *enumValues, int count,
+                          int defaultValue, int defaultValueIfSet,
+                          EnumIndexStorePointer store);
 
         virtual OptionInfo &optionInfo() { return info_; }
         virtual std::string typeString() const { return "enum"; }
@@ -228,9 +251,14 @@ class EnumOptionStorage : public OptionStorageTemplate<int>
 
     private:
         virtual void convertValue(const std::string &value);
+        virtual void processSetValues(ValueList *values);
+        virtual void refreshValues();
+
+        void refreshEnumIndexStore();
 
         EnumOptionInfo            info_;
         std::vector<std::string>  allowed_;
+        EnumIndexStorePointer     store_;
 };
 
 /*!\}*/
index 324731646633e9327b33a000df8368f3b5929312..0cb0ab322490c76cb85b4b5ad43475ff8461bcd2 100644 (file)
@@ -118,7 +118,16 @@ class OptionStorageTemplate : public AbstractOptionStorage
         template <class U>
         explicit OptionStorageTemplate(const OptionTemplate<T, U> &settings,
                                        OptionFlags staticFlags = OptionFlags());
-
+        /*! \brief
+         * Initializes the storage from base option settings.
+         *
+         * \param[in] settings  Option settings.
+         * \throws  APIError if invalid settings have been provided.
+         *
+         * This constructor works for cases where there is no matching
+         * OptionTemplate (e.g., EnumOption).
+         */
+        explicit OptionStorageTemplate(const AbstractOption &settings);
 
         virtual void clearSet();
         /*! \copydoc gmx::AbstractOptionStorage::convertValue()
@@ -207,7 +216,8 @@ class OptionStorageTemplate : public AbstractOptionStorage
          * If this function succeeds, values added with addValue() since the
          * previous clearSet() are added to the storage for the option.
          * Only throws in out-of-memory conditions, and provides the strong
-         * exception safety guarantee.
+         * exception safety guarantee as long as the copy constructor of `T`
+         * does not throw.
          *
          * See addValue() for cases where this method should be used in derived
          * classes.
@@ -346,6 +356,16 @@ OptionStorageTemplate<T>::OptionStorageTemplate(const OptionTemplate<T, U> &sett
 }
 
 
+template <typename T>
+OptionStorageTemplate<T>::OptionStorageTemplate(const AbstractOption &settings)
+    : AbstractOptionStorage(settings, OptionFlags()),
+      store_(NULL), countptr_(NULL),
+      ownedValues_(new std::vector<T>())
+{
+    values_ = ownedValues_.get();
+}
+
+
 template <typename T>
 OptionStorageTemplate<T>::~OptionStorageTemplate()
 {
@@ -419,6 +439,7 @@ void OptionStorageTemplate<T>::commitValues()
     }
     else
     {
+        values_->reserve(values_->size() + setValues_.size());
         values_->insert(values_->end(), setValues_.begin(), setValues_.end());
     }
     clearSet();
index 1a415cbafd5ee9c8acb9aa965124adf88c2b728c..b9dd8eb5f8f77a7581388e1ad97bb74d29fd0c33 100644 (file)
@@ -849,7 +849,7 @@ TEST(OptionsAssignerStringTest, HandlesEnumDefaultValueFromVector)
  * Tests for enum options
  */
 
-//! Enum type for EnumIntOption tests.
+//! Enum type for EnumOption tests.
 enum TestEnum
 {
     etestNone,
@@ -861,10 +861,10 @@ enum TestEnum
 TEST(OptionsAssignerEnumTest, StoresSingleValue)
 {
     gmx::Options           options(NULL, NULL);
-    int                    value     = etestNone;
-    using gmx::EnumIntOption;
+    TestEnum               value     = etestNone;
+    using gmx::EnumOption;
     ASSERT_NO_THROW(options.addOption(
-                            EnumIntOption("p").store(&value)
+                            EnumOption<TestEnum>("p").store(&value)
                                 .enumValue(c_allowed)));
     EXPECT_EQ(etestNone, value);
 
@@ -882,10 +882,10 @@ TEST(OptionsAssignerEnumTest, StoresSingleValue)
 TEST(OptionsAssignerEnumTest, StoresVectorValues)
 {
     gmx::Options           options(NULL, NULL);
-    std::vector<int>       values;
-    using gmx::EnumIntOption;
+    std::vector<TestEnum>  values;
+    using gmx::EnumOption;
     ASSERT_NO_THROW(options.addOption(
-                            EnumIntOption("p").storeVector(&values)
+                            EnumOption<TestEnum>("p").storeVector(&values)
                                 .multiValue().enumValue(c_allowed)));
     EXPECT_TRUE(values.empty());
 
@@ -906,10 +906,10 @@ TEST(OptionsAssignerEnumTest, StoresVectorValues)
 TEST(OptionsAssignerEnumTest, HandlesInitialValueOutOfRange)
 {
     gmx::Options           options(NULL, NULL);
-    int                    value     = etestNR;
-    using gmx::EnumIntOption;
+    TestEnum               value     = etestNR;
+    using gmx::EnumOption;
     ASSERT_NO_THROW(options.addOption(
-                            EnumIntOption("p").store(&value)
+                            EnumOption<TestEnum>("p").store(&value)
                                 .enumValue(c_allowed)));
     EXPECT_EQ(etestNR, value);
 
@@ -924,10 +924,10 @@ TEST(OptionsAssignerEnumTest, HandlesInitialValueOutOfRange)
 TEST(OptionsAssignerEnumTest, HandlesEnumDefaultValue)
 {
     gmx::Options           options(NULL, NULL);
-    int                    value     = etestNone;
-    using gmx::EnumIntOption;
+    TestEnum               value     = etestNone;
+    using gmx::EnumOption;
     ASSERT_NO_THROW(options.addOption(
-                            EnumIntOption("p").store(&value)
+                            EnumOption<TestEnum>("p").store(&value)
                                 .enumValue(c_allowed).defaultValue(etestTest)));
     EXPECT_EQ(etestTest, value);
 
@@ -942,10 +942,10 @@ TEST(OptionsAssignerEnumTest, HandlesEnumDefaultValue)
 TEST(OptionsAssignerEnumTest, HandlesEnumDefaultValueFromVariable)
 {
     gmx::Options           options(NULL, NULL);
-    int                    value     = etestTest;
-    using gmx::EnumIntOption;
+    TestEnum               value     = etestTest;
+    using gmx::EnumOption;
     ASSERT_NO_THROW(options.addOption(
-                            EnumIntOption("p").store(&value)
+                            EnumOption<TestEnum>("p").store(&value)
                                 .enumValue(c_allowed)));
     EXPECT_EQ(etestTest, value);
 
@@ -960,12 +960,12 @@ TEST(OptionsAssignerEnumTest, HandlesEnumDefaultValueFromVariable)
 TEST(OptionsAssignerEnumTest, HandlesEnumDefaultValueFromVector)
 {
     gmx::Options             options(NULL, NULL);
-    std::vector<int>         value;
+    std::vector<TestEnum>    value;
     value.push_back(etestNone);
     value.push_back(etestTest);
-    using gmx::EnumIntOption;
+    using gmx::EnumOption;
     ASSERT_NO_THROW(options.addOption(
-                            EnumIntOption("p").storeVector(&value).valueCount(2)
+                            EnumOption<TestEnum>("p").storeVector(&value).valueCount(2)
                                 .enumValue(c_allowed)));
     ASSERT_EQ(2U, value.size());
     EXPECT_EQ(etestNone, value[0]);
index 55f4303e6d00fc2080bbbc257bfa82f80135feea..73f8fc329b081311b76b770a174fb6dbee5ce157 100644 (file)
@@ -167,7 +167,7 @@ void TimeUnitBehavior::setTimeUnitFromEnvironment()
 
 void TimeUnitBehavior::addTimeUnitOption(IOptionsContainer *options, const char *name)
 {
-    options->addOption(EnumIntOption(name).enumValue(g_timeUnits)
+    options->addOption(EnumOption<TimeUnit>(name).enumValue(g_timeUnits)
                            .store(&timeUnit_)
                            .description("Unit for time values"));
 }
index a5ac0129a0546446042e9904436e7c35e9aaed8f..78bcaf5851af9171bd710f5d6952b97a51d5da9c 100644 (file)
@@ -188,10 +188,7 @@ class TimeUnitBehavior : public IOptionsBehavior
         virtual void optionsFinished() {}
 
     private:
-        // Type is int to make it possible to use it with
-        // StringOption::storeEnumIndex(), but it should always one of the
-        // allowed values for TimeUnit.
-        int              timeUnit_;
+        TimeUnit         timeUnit_;
         TimeUnit        *timeUnitStore_;
 
         GMX_DISALLOW_COPY_AND_ASSIGN(TimeUnitBehavior);
index 4a7b510c7a549935b488f04492d6c52588dd0483..d5fe0e590dabbf9999b959e51ca8e9784aa34bfa 100644 (file)
@@ -296,8 +296,8 @@ class Angle : public TrajectoryAnalysisModule
         std::string                              fnAll_;
         std::string                              fnHistogram_;
 
-        int                                      g1type_;
-        int                                      g2type_;
+        Group1Type                               g1type_;
+        Group2Type                               g2type_;
         double                                   binWidth_;
 
         AnalysisData                             angles_;
@@ -392,10 +392,10 @@ Angle::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *setti
                            .store(&fnHistogram_).defaultBasename("anghist")
                            .description("Histogram of the angles"));
 
-    options->addOption(EnumIntOption("g1").enumValue(cGroup1TypeEnum)
+    options->addOption(EnumOption<Group1Type>("g1").enumValue(cGroup1TypeEnum)
                            .store(&g1type_)
                            .description("Type of analysis/first vector group"));
-    options->addOption(EnumIntOption("g2").enumValue(cGroup2TypeEnum)
+    options->addOption(EnumOption<Group2Type>("g2").enumValue(cGroup2TypeEnum)
                            .store(&g2type_)
                            .description("Type of second vector group"));
     options->addOption(DoubleOption("binw").store(&binWidth_)
index 92709aecd890ce3fd9b434776c69d94bd700f35e..73a248c3404167857308d570fa511798f175cff5 100644 (file)
@@ -147,9 +147,9 @@ class PairDistance : public TrajectoryAnalysisModule
         std::string             fnDist_;
 
         double                  cutoff_;
-        int                     distanceType_;
-        int                     refGroupType_;
-        int                     selGroupType_;
+        DistanceType            distanceType_;
+        GroupType               refGroupType_;
+        GroupType               selGroupType_;
 
         //! Number of groups in `refSel_`.
         int                     refGroupCount_;
@@ -221,13 +221,13 @@ PairDistance::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings
 
     options->addOption(DoubleOption("cutoff").store(&cutoff_)
                            .description("Maximum distance to consider"));
-    options->addOption(EnumIntOption("type").store(&distanceType_)
+    options->addOption(EnumOption<DistanceType>("type").store(&distanceType_)
                            .enumValue(c_distanceTypes)
                            .description("Type of distances to calculate"));
-    options->addOption(EnumIntOption("refgrouping").store(&refGroupType_)
+    options->addOption(EnumOption<GroupType>("refgrouping").store(&refGroupType_)
                            .enumValue(c_groupTypes)
                            .description("Grouping of -ref positions to compute the min/max over"));
-    options->addOption(EnumIntOption("selgrouping").store(&selGroupType_)
+    options->addOption(EnumOption<GroupType>("selgrouping").store(&selGroupType_)
                            .enumValue(c_groupTypes)
                            .description("Grouping of -sel positions to compute the min/max over"));
 
index 3712bffae021f90e12dae76bf444ffbe3d31f68f..841f5f85dce651fea7501ccf51c0114859525135 100644 (file)
@@ -135,7 +135,7 @@ class Rdf : public TrajectoryAnalysisModule
     private:
         std::string                               fnRdf_;
         std::string                               fnCumulative_;
-        int                                       surface_;
+        SurfaceType                               surface_;
         AnalysisDataPlotSettings                  plotSettings_;
 
         /*! \brief
@@ -188,7 +188,7 @@ class Rdf : public TrajectoryAnalysisModule
         double                                    binwidth_;
         double                                    cutoff_;
         double                                    rmax_;
-        int                                       normalization_;
+        Normalization                             normalization_;
         bool                                      bNormalizationSet_;
         bool                                      bXY_;
         bool                                      bExclusions_;
@@ -284,7 +284,7 @@ Rdf::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *setting
 
     options->addOption(DoubleOption("bin").store(&binwidth_)
                            .description("Bin width (nm)"));
-    options->addOption(EnumIntOption("norm").enumValue(c_NormalizationEnum)
+    options->addOption(EnumOption<Normalization>("norm").enumValue(c_NormalizationEnum)
                            .store(&normalization_)
                            .storeIsSet(&bNormalizationSet_)
                            .description("Normalization"));
@@ -297,7 +297,7 @@ Rdf::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *setting
     options->addOption(DoubleOption("rmax").store(&rmax_)
                            .description("Largest distance (nm) to calculate"));
 
-    options->addOption(EnumIntOption("surf").enumValue(c_SurfaceEnum)
+    options->addOption(EnumOption<SurfaceType>("surf").enumValue(c_SurfaceEnum)
                            .store(&surface_)
                            .description("RDF with respect to the surface of the reference"));
 
index 7cacecd8910a44aafc06c7aebb37f6d8d1402802..b43d09ff5b8b348404e663b1b6bd9eee3ac4c1f2 100644 (file)
@@ -307,8 +307,8 @@ class Select : public TrajectoryAnalysisModule
         bool                                bFracNorm_;
         bool                                bResInd_;
         bool                                bCumulativeLifetimes_;
-        int                                 resNumberType_;
-        int                                 pdbAtoms_;
+        ResidueNumbering                    resNumberType_;
+        PdbAtomsSelection                   pdbAtoms_;
 
         const TopologyInformation          *top_;
         std::vector<int>                    totsize_;
@@ -444,10 +444,10 @@ Select::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *sett
                            .description("Normalize by total number of positions with -os"));
     options->addOption(BooleanOption("cfnorm").store(&bFracNorm_)
                            .description("Normalize by covered fraction with -os"));
-    options->addOption(EnumIntOption("resnr").store(&resNumberType_)
+    options->addOption(EnumOption<ResidueNumbering>("resnr").store(&resNumberType_)
                            .enumValue(cResNumberEnum)
                            .description("Residue number output type with -oi and -on"));
-    options->addOption(EnumIntOption("pdbatoms").store(&pdbAtoms_)
+    options->addOption(EnumOption<PdbAtomsSelection>("pdbatoms").store(&pdbAtoms_)
                            .enumValue(cPDBAtomsEnum)
                            .description("Atoms to write with -ofpdb"));
     options->addOption(BooleanOption("cumlt").store(&bCumulativeLifetimes_)
index 53a84fa8482c509d7131aaab7b5714664dbd368b..c709685e56d510772a3b700748d5fc8172bc3f4a 100644 (file)
@@ -150,13 +150,12 @@ typedef std::shared_ptr<internal::TestReferenceDataImpl>
  */
 TestReferenceDataImplPointer g_referenceData;
 //! Global reference data mode set with setReferenceDataMode().
-// TODO: Make this a real enum (requires solving a TODO in EnumIntOption).
-int                          g_referenceDataMode = erefdataCompare;
+ReferenceDataMode            g_referenceDataMode = erefdataCompare;
 
 //! Returns the global reference data mode.
 ReferenceDataMode getReferenceDataMode()
 {
-    return static_cast<ReferenceDataMode>(g_referenceDataMode);
+    return g_referenceDataMode;
 }
 
 //! Returns a reference to the global reference data object.
@@ -211,7 +210,7 @@ void initReferenceData(IOptionsContainer *options)
     const char *const refDataEnum[] =
     { "check", "create", "update-changed", "update-all" };
     options->addOption(
-            EnumIntOption("ref-data")
+            EnumOption<ReferenceDataMode>("ref-data")
                 .enumValue(refDataEnum).store(&g_referenceDataMode)
                 .description("Operation mode for tests that use reference data"));
     ::testing::UnitTest::GetInstance()->listeners().Append(