Remove SelectionOptionAdjuster.
authorTeemu Murtola <teemu.murtola@gmail.com>
Sun, 12 Feb 2012 12:57:15 +0000 (14:57 +0200)
committerTeemu Murtola <teemu.murtola@gmail.com>
Mon, 20 Feb 2012 18:26:42 +0000 (20:26 +0200)
There is no reason to have a separate SelectionOptionAdjuster now that
SelectionOptionInfo provides a more general mechanism to achieve the
same thing.  Kept the SelectionOption::getAdjuster() method as the way
to obtain a SelectionOptionInfo object, but this can be replaced by a
more general mechanism if this functionality is needed for other option
types.

Related to issue #839.

Change-Id: Ief109c61e346cdfac2c665d7ed9a3002f80cc57f

src/gromacs/selection/selectionoption.cpp
src/gromacs/selection/selectionoption.h
src/gromacs/selection/selectionoptioninfo.h
src/gromacs/selection/selectionoptionstorage.h
src/gromacs/selection/tests/selectionoption.cpp
src/gromacs/trajectoryanalysis/modules/angle.cpp
src/gromacs/trajectoryanalysis/modules/angle.h

index e97129e5738d0823c844f97c82a89bce42322de8..367a3db9eaa82747fadefeb4e9625d187c213554 100644 (file)
@@ -63,24 +63,16 @@ SelectionOptionStorage::SelectionOptionStorage(const SelectionOption &settings,
                                                Options *options)
     : MyBase(settings, options,
              OptionFlags() | efNoDefaultValue | efDontCheckMinimumCount),
-      _info(this), _selectionFlags(settings._selectionFlags),
-      _adjuster(NULL)
+      _info(this), _selectionFlags(settings._selectionFlags)
 {
     options->globalProperties().request(eogpSelectionCollection);
-    if (settings._adjuster != NULL)
+    if (settings._infoPtr != NULL)
     {
-        _adjuster = new SelectionOptionAdjuster(this);
-        *settings._adjuster = _adjuster;
+        *settings._infoPtr = &_info;
     }
 }
 
 
-SelectionOptionStorage::~SelectionOptionStorage()
-{
-    delete _adjuster;
-}
-
-
 std::string SelectionOptionStorage::formatValue(int i) const
 {
     Selection *sel = values().at(i);
@@ -208,49 +200,49 @@ SelectionOptionInfo::SelectionOptionInfo(SelectionOptionStorage *option)
 {
 }
 
+SelectionOptionStorage &SelectionOptionInfo::option()
+{
+    return static_cast<SelectionOptionStorage &>(OptionInfo::option());
+}
 
-/********************************************************************
- * SelectionOptionAdjuster
- */
-
-SelectionOptionAdjuster::SelectionOptionAdjuster(SelectionOptionStorage *storage)
-    : _storage(*storage)
+const SelectionOptionStorage &SelectionOptionInfo::option() const
 {
+    return static_cast<const SelectionOptionStorage &>(OptionInfo::option());
 }
 
-void SelectionOptionAdjuster::setValueCount(int count)
+void SelectionOptionInfo::setValueCount(int count)
 {
-    return storage().setAllowedValueCount(count);
+    option().setAllowedValueCount(count);
 }
 
-void SelectionOptionAdjuster::setEvaluateVelocities(bool bEnabled)
+void SelectionOptionInfo::setEvaluateVelocities(bool bEnabled)
 {
-    return storage().setSelectionFlag(efEvaluateVelocities, bEnabled);
+    option().setSelectionFlag(efEvaluateVelocities, bEnabled);
 }
 
-void SelectionOptionAdjuster::setEvaluateForces(bool bEnabled)
+void SelectionOptionInfo::setEvaluateForces(bool bEnabled)
 {
-    return storage().setSelectionFlag(efEvaluateForces, bEnabled);
+    option().setSelectionFlag(efEvaluateForces, bEnabled);
 }
 
-void SelectionOptionAdjuster::setOnlyAtoms(bool bEnabled)
+void SelectionOptionInfo::setOnlyAtoms(bool bEnabled)
 {
-    return storage().setSelectionFlag(efOnlyAtoms, bEnabled);
+    option().setSelectionFlag(efOnlyAtoms, bEnabled);
 }
 
-void SelectionOptionAdjuster::setOnlyStatic(bool bEnabled)
+void SelectionOptionInfo::setOnlyStatic(bool bEnabled)
 {
-    return storage().setSelectionFlag(efOnlyStatic, bEnabled);
+    option().setSelectionFlag(efOnlyStatic, bEnabled);
 }
 
-void SelectionOptionAdjuster::setDynamicMask(bool bEnabled)
+void SelectionOptionInfo::setDynamicMask(bool bEnabled)
 {
-    return storage().setSelectionFlag(efDynamicMask, bEnabled);
+    option().setSelectionFlag(efDynamicMask, bEnabled);
 }
 
-void SelectionOptionAdjuster::setDynamicOnlyWhole(bool bEnabled)
+void SelectionOptionInfo::setDynamicOnlyWhole(bool bEnabled)
 {
-    return storage().setSelectionFlag(efDynamicOnlyWhole, bEnabled);
+    option().setSelectionFlag(efDynamicOnlyWhole, bEnabled);
 }
 
 
index eff1457cd961217847f0032b956cd74fc898289d..b19a2abda385b3cc8e73261ca6980456b4f2c441 100644 (file)
@@ -46,7 +46,7 @@ namespace gmx
 {
 
 class Selection;
-class SelectionOptionAdjuster;
+class SelectionOptionInfo;
 class SelectionOptionStorage;
 
 /*! \brief
@@ -60,7 +60,7 @@ class SelectionOption : public OptionTemplate<Selection *, SelectionOption>
     public:
         //! Initializes an option with the given name.
         explicit SelectionOption(const char *name)
-            : MyBase(name), _adjuster(NULL)
+            : MyBase(name), _infoPtr(NULL)
         { }
 
         /*! \brief
@@ -101,12 +101,13 @@ class SelectionOption : public OptionTemplate<Selection *, SelectionOption>
         { _selectionFlags.set(efDynamicOnlyWhole); return me(); }
 
         /*! \brief
-         * Get an adjuster that can be used to alter the option after creation.
+         * Get an info object that can be used to alter the option after
+         * creation.
          *
-         * \see SelectionOptionAdjuster
+         * \see SelectionOptionInfo
          */
-        MyClass &getAdjuster(SelectionOptionAdjuster **adjusterp)
-        { _adjuster = adjusterp; return me(); }
+        MyClass &getAdjuster(SelectionOptionInfo **infoPtr)
+        { _infoPtr = infoPtr; return me(); }
 
     private:
         // Disable default value because it is impossible to provide a
@@ -117,7 +118,7 @@ class SelectionOption : public OptionTemplate<Selection *, SelectionOption>
         virtual AbstractOptionStorage *createDefaultStorage(Options *options) const;
 
         SelectionFlags          _selectionFlags;
-        SelectionOptionAdjuster **_adjuster;
+        SelectionOptionInfo   **_infoPtr;
 
         /*! \brief
          * Needed to initialize SelectionOptionStorage from this class without
@@ -126,95 +127,6 @@ class SelectionOption : public OptionTemplate<Selection *, SelectionOption>
         friend class SelectionOptionStorage;
 };
 
-
-/*! \brief
- * Allows changes to a selection option after creation.
- *
- * This class provides the necessary interface for changing, e.g., the number
- * of allowed selections for a selection option after the option has been
- * created with Options::addOption().  This is needed if the number or other
- * flags are only known after other options have been parsed.  The main
- * advantage of this class over custom checks is that if used before
- * interactive selection prompt, the interactive prompt is updated accordingly.
- *
- * When using this class, the option should be initially created with the most
- * permissive flags, and this class should be used to place restrictions where
- * appropriate.  Otherwise, values that are provided before adjustments will
- * need to follow the more strict checks.  In most cases in trajectory analysis
- * (which is the main use case for selection options), the adjustments should
- * be done in TrajectoryAnalysisModule::initOptionsDone() for them to take
- * place before interactive selection prompts.
- *
- * An instance of this class for a selection option can be obtained with
- * SelectionOption::getAdjuster() when the option is created.
- *
- * Example use:
- * \code
-std::vector<Selection *> sel;
-Options options("example", "Example options");
-SelectionOptionAdjuster *adjuster;
-options.addOption(SelectionOption("sel").storeVector(&sel)
-                      .multiValue().getAdjuster(&adjuster));
-// < ... assign values to options ...>
-if ( condition )
-{
-    OptionAdjusterErrorContext context(adjuster, errors);
-    // Put limitations on the selections based on the condition,
-    // which can depend on other option values.
-    int rc = adjuster->setValueCount(2);
-    if (rc == 0)
-    {
-        rc = adjuster->setOnlyStatic(true);
-    }
-    if (rc != 0)
-    {
-        return rc;
-    }
-}
- * \endcode
- *
- * \inpublicapi
- * \ingroup module_selection
- */
-class SelectionOptionAdjuster
-{
-    public:
-        /*! \brief
-         * Creates a new adjuster.
-         *
-         * Should only be called internally by the options module.
-         */
-        SelectionOptionAdjuster(SelectionOptionStorage *storage);
-
-        /*! \brief
-         * Sets the number of selections allowed for the option.
-         *
-         * \param[in] count  Number of allowed selections.
-         */
-        void setValueCount(int count);
-
-        //! \copydoc SelectionOption::evaluateVelocities()
-        void setEvaluateVelocities(bool bEnabled);
-        //! \copydoc SelectionOption::evaluateForces()
-        void setEvaluateForces(bool bEnabled);
-        //! \copydoc SelectionOption::onlyAtoms()
-        void setOnlyAtoms(bool bEnabled);
-        //! \copydoc SelectionOption::onlyStatic()
-        void setOnlyStatic(bool bEnabled);
-        //! \copydoc SelectionOption::dynamicMask()
-        void setDynamicMask(bool bEnabled);
-        //! \copydoc SelectionOption::dynamicOnlyWhole()
-        void setDynamicOnlyWhole(bool bEnabled);
-
-    private:
-        //! Returns the storage object associated with this adjuster.
-        SelectionOptionStorage &storage() { return _storage; }
-        //! Returns the storage object associated with this adjuster.
-        const SelectionOptionStorage &storage() const { return _storage; }
-
-        SelectionOptionStorage &_storage;
-};
-
 } // namespace gmx
 
 #endif
index f940668a0caae1f36d7ea161021538abcef8820c..a95bad0457e9cdf2c4b5d4633841f53e490c222d 100644 (file)
@@ -47,7 +47,45 @@ namespace gmx
 class SelectionOptionStorage;
 
 /*! \brief
- * Wrapper class for accessing selection option information.
+ * Wrapper class for accessing and modifying selection option information.
+ *
+ * Allows changes to a selection option after creation.
+ *
+ * This class provides the necessary interface for changing, e.g., the number
+ * of allowed selections for a selection option after the option has been
+ * created with Options::addOption().  This is needed if the number or other
+ * flags are only known after other options have been parsed.  The main
+ * advantage of this class over custom checks is that if used before
+ * interactive selection prompt, the interactive prompt is updated accordingly.
+ *
+ * When using this class, the option should be initially created with the most
+ * permissive flags, and this class should be used to place restrictions where
+ * appropriate.  Otherwise, values that are provided before adjustments will
+ * need to follow the more strict checks.  In most cases in trajectory analysis
+ * (which is the main use case for selection options), the adjustments should
+ * be done in TrajectoryAnalysisModule::initOptionsDone() for them to take
+ * place before interactive selection prompts.
+ *
+ * An instance of this class for a selection option can be obtained with
+ * SelectionOption::getAdjuster() when the option is created.
+ *
+ * Example use:
+ * \code
+std::vector<Selection *> sel;
+Options options("example", "Example options");
+SelectionOptionInfo *info;
+options.addOption(SelectionOption("sel").storeVector(&sel)
+                      .multiValue().getAdjuster(&info));
+// < ... assign values to options ...>
+if ( condition )
+{
+    // Put limitations on the selections based on the condition,
+    // which can depend on other option values.
+    // Throws if input given so far violates the limitations.
+    info->setValueCount(2);
+    info->setOnlyStatic(true);
+}
+ * \endcode
  *
  * \inpublicapi
  * \ingroup module_selection
@@ -55,7 +93,32 @@ class SelectionOptionStorage;
 class SelectionOptionInfo : public OptionInfo
 {
     public:
+        //! Creates option info object for given storage object.
         explicit SelectionOptionInfo(SelectionOptionStorage *option);
+
+        /*! \brief
+         * Sets the number of selections allowed for the option.
+         *
+         * \param[in] count  Number of allowed selections.
+         */
+        void setValueCount(int count);
+
+        //! \copydoc SelectionOption::evaluateVelocities()
+        void setEvaluateVelocities(bool bEnabled);
+        //! \copydoc SelectionOption::evaluateForces()
+        void setEvaluateForces(bool bEnabled);
+        //! \copydoc SelectionOption::onlyAtoms()
+        void setOnlyAtoms(bool bEnabled);
+        //! \copydoc SelectionOption::onlyStatic()
+        void setOnlyStatic(bool bEnabled);
+        //! \copydoc SelectionOption::dynamicMask()
+        void setDynamicMask(bool bEnabled);
+        //! \copydoc SelectionOption::dynamicOnlyWhole()
+        void setDynamicOnlyWhole(bool bEnabled);
+
+    private:
+        SelectionOptionStorage &option();
+        const SelectionOptionStorage &option() const;
 };
 
 } // namespace gmx
index c993521dee913036511db137e1d63fbd77557b90..c5de5cfa3e6a33bcde3b0e3927b0289703de104b 100644 (file)
@@ -47,7 +47,6 @@ namespace gmx
 
 class Selection;
 class SelectionOption;
-class SelectionOptionAdjuster;
 
 /*! \internal \brief
  * Converts, validates, and stores selection values.
@@ -64,7 +63,6 @@ class SelectionOptionStorage : public OptionStorageTemplate<Selection *>
          * \param[in] options    Options object.
          */
         SelectionOptionStorage(const SelectionOption &settings, Options *options);
-        virtual ~SelectionOptionStorage();
 
         virtual OptionInfo &optionInfo() { return _info; }
         virtual const char *typeString() const { return "sel"; }
@@ -118,8 +116,6 @@ class SelectionOptionStorage : public OptionStorageTemplate<Selection *>
 
         SelectionOptionInfo     _info;
         SelectionFlags          _selectionFlags;
-        //! Pointer to the adjuster (there can be only one, can be NULL).
-        SelectionOptionAdjuster *_adjuster;
 };
 
 } // namespace gmx
index d531ccf017fcb40e7d59bc70d0b0f73684ba924e..426ad0635df42daaeb5ead6ea8ae50d9af4db085 100644 (file)
@@ -46,6 +46,7 @@
 #include "gromacs/selection/selection.h"
 #include "gromacs/selection/selectioncollection.h"
 #include "gromacs/selection/selectionoption.h"
+#include "gromacs/selection/selectionoptioninfo.h"
 
 namespace
 {
@@ -142,11 +143,11 @@ TEST_F(SelectionOptionTest, HandlesTooFewSelections)
 TEST_F(SelectionOptionTest, HandlesAdjuster)
 {
     std::vector<gmx::Selection *> sel;
-    gmx::SelectionOptionAdjuster *adjuster;
+    gmx::SelectionOptionInfo *info;
     using gmx::SelectionOption;
     ASSERT_NO_THROW(_options.addOption(
                         SelectionOption("sel").storeVector(&sel).multiValue()
-                            .getAdjuster(&adjuster)));
+                            .getAdjuster(&info)));
 
     gmx::OptionsAssigner assigner(&_options);
     EXPECT_NO_THROW(assigner.start());
@@ -156,18 +157,18 @@ TEST_F(SelectionOptionTest, HandlesAdjuster)
     EXPECT_NO_THROW(assigner.finishOption());
     EXPECT_NO_THROW(assigner.finish());
     EXPECT_NO_THROW(_options.finish());
-    EXPECT_NO_THROW(adjuster->setValueCount(2));
+    EXPECT_NO_THROW(info->setValueCount(2));
 }
 
 
 TEST_F(SelectionOptionTest, HandlesDynamicWhenStaticRequiredWithAdjuster)
 {
     gmx::Selection *sel;
-    gmx::SelectionOptionAdjuster *adjuster;
+    gmx::SelectionOptionInfo *info;
     using gmx::SelectionOption;
     ASSERT_NO_THROW(_options.addOption(
                         SelectionOption("sel").store(&sel)
-                            .getAdjuster(&adjuster)));
+                            .getAdjuster(&info)));
 
     gmx::OptionsAssigner assigner(&_options);
     EXPECT_NO_THROW(assigner.start());
@@ -176,18 +177,18 @@ TEST_F(SelectionOptionTest, HandlesDynamicWhenStaticRequiredWithAdjuster)
     EXPECT_NO_THROW(assigner.finishOption());
     EXPECT_NO_THROW(assigner.finish());
     EXPECT_NO_THROW(_options.finish());
-    EXPECT_THROW(adjuster->setOnlyStatic(true), gmx::InvalidInputError);
+    EXPECT_THROW(info->setOnlyStatic(true), gmx::InvalidInputError);
 }
 
 
 TEST_F(SelectionOptionTest, HandlesTooManySelectionsWithAdjuster)
 {
     std::vector<gmx::Selection *> sel;
-    gmx::SelectionOptionAdjuster *adjuster;
+    gmx::SelectionOptionInfo *info;
     using gmx::SelectionOption;
     ASSERT_NO_THROW(_options.addOption(
                         SelectionOption("sel").storeVector(&sel).multiValue()
-                            .getAdjuster(&adjuster)));
+                            .getAdjuster(&info)));
 
     gmx::OptionsAssigner assigner(&_options);
     EXPECT_NO_THROW(assigner.start());
@@ -197,18 +198,18 @@ TEST_F(SelectionOptionTest, HandlesTooManySelectionsWithAdjuster)
     EXPECT_NO_THROW(assigner.finishOption());
     EXPECT_NO_THROW(assigner.finish());
     EXPECT_NO_THROW(_options.finish());
-    EXPECT_THROW(adjuster->setValueCount(1), gmx::InvalidInputError);
+    EXPECT_THROW(info->setValueCount(1), gmx::InvalidInputError);
 }
 
 
 TEST_F(SelectionOptionTest, HandlesTooFewSelectionsWithAdjuster)
 {
     std::vector<gmx::Selection *> sel;
-    gmx::SelectionOptionAdjuster *adjuster;
+    gmx::SelectionOptionInfo *info;
     using gmx::SelectionOption;
     ASSERT_NO_THROW(_options.addOption(
                         SelectionOption("sel").storeVector(&sel).multiValue()
-                            .getAdjuster(&adjuster)));
+                            .getAdjuster(&info)));
 
     gmx::OptionsAssigner assigner(&_options);
     EXPECT_NO_THROW(assigner.start());
@@ -217,7 +218,7 @@ TEST_F(SelectionOptionTest, HandlesTooFewSelectionsWithAdjuster)
     EXPECT_NO_THROW(assigner.finishOption());
     EXPECT_NO_THROW(assigner.finish());
     EXPECT_NO_THROW(_options.finish());
-    EXPECT_THROW(adjuster->setValueCount(2), gmx::InvalidInputError);
+    EXPECT_THROW(info->setValueCount(2), gmx::InvalidInputError);
 }
 
 
@@ -273,11 +274,11 @@ TEST_F(SelectionOptionTest, HandlesDelayedOptionalSelection)
 TEST_F(SelectionOptionTest, HandlesDelayedSelectionWithAdjuster)
 {
     std::vector<gmx::Selection *> sel;
-    gmx::SelectionOptionAdjuster *adjuster;
+    gmx::SelectionOptionInfo *info;
     using gmx::SelectionOption;
     ASSERT_NO_THROW(_options.addOption(
                         SelectionOption("sel").storeVector(&sel).valueCount(3)
-                            .getAdjuster(&adjuster)));
+                            .getAdjuster(&info)));
 
     gmx::OptionsAssigner assigner(&_options);
     EXPECT_NO_THROW(assigner.start());
@@ -285,7 +286,7 @@ TEST_F(SelectionOptionTest, HandlesDelayedSelectionWithAdjuster)
     EXPECT_NO_THROW(assigner.finishOption());
     EXPECT_NO_THROW(assigner.finish());
     EXPECT_NO_THROW(_options.finish());
-    EXPECT_NO_THROW(adjuster->setValueCount(2));
+    EXPECT_NO_THROW(info->setValueCount(2));
     EXPECT_NO_THROW(_sc.parseRequestedFromString("resname RA RB; resname RB RC"));
 }
 
index 4348090ad75f0bb718eb99e7ad1a3019f8525f6d..f57f341ebfb39eb567cf1a3b31e8d6a76906c65f 100644 (file)
@@ -51,6 +51,7 @@
 #include "gromacs/options/options.h"
 #include "gromacs/selection/selection.h"
 #include "gromacs/selection/selectionoption.h"
+#include "gromacs/selection/selectionoptioninfo.h"
 #include "gromacs/utility/format.h"
 
 namespace gmx
@@ -61,7 +62,7 @@ namespace analysismodules
 
 Angle::Angle()
     : _options("angle", "Angle calculation"),
-      _sel1Adj(NULL), _sel2Adj(NULL),
+      _sel1info(NULL), _sel2info(NULL),
       _bSplit1(false), _bSplit2(false), _bMulti(false), _bAll(false),
       _bDumpDist(false), _vt0(NULL)
 {
@@ -146,10 +147,10 @@ Angle::initOptions(TrajectoryAnalysisSettings *settings)
         .description("Write also distances with -od"));
 
     _options.addOption(SelectionOption("group1").multiValue().required()
-        .dynamicOnlyWhole().storeVector(&_sel1).getAdjuster(&_sel1Adj)
+        .dynamicOnlyWhole().storeVector(&_sel1).getAdjuster(&_sel1info)
         .description("First analysis/vector selection"));
     _options.addOption(SelectionOption("group2").multiValue()
-        .dynamicOnlyWhole().storeVector(&_sel2).getAdjuster(&_sel2Adj)
+        .dynamicOnlyWhole().storeVector(&_sel2).getAdjuster(&_sel2info)
         .description("Second analysis/vector selection"));
 
     return &_options;
@@ -197,7 +198,7 @@ Angle::initOptionsDone(TrajectoryAnalysisSettings *settings)
 
     if (_bAll)
     {
-        _sel1Adj->setOnlyStatic(true);
+        _sel1info->setOnlyStatic(true);
     }
 
     // Set up the number of positions per angle.
@@ -228,11 +229,11 @@ Angle::initOptionsDone(TrajectoryAnalysisSettings *settings)
 
     if (!_bMulti)
     {
-        _sel1Adj->setValueCount(_bSplit1 ? _natoms1 : 1);
+        _sel1info->setValueCount(_bSplit1 ? _natoms1 : 1);
     }
     if (_natoms2 > 0)
     {
-        _sel2Adj->setValueCount(_bSplit2 ? _natoms2 : 1);
+        _sel2info->setValueCount(_bSplit2 ? _natoms2 : 1);
     }
 }
 
index 6aadf4d960b202fe99091b432fc764eab93ef02b..6d2fcf8380dc125a7ac7cba72342f92d9c76c45d 100644 (file)
@@ -49,7 +49,7 @@ namespace gmx
 {
 
 class Selection;
-class SelectionOptionAdjuster;
+class SelectionOptionInfo;
 
 namespace analysismodules
 {
@@ -81,8 +81,8 @@ class Angle : public TrajectoryAnalysisModule
 
         std::vector<Selection *> _sel1;
         std::vector<Selection *> _sel2;
-        SelectionOptionAdjuster *_sel1Adj;
-        SelectionOptionAdjuster *_sel2Adj;
+        SelectionOptionInfo    *_sel1info;
+        SelectionOptionInfo    *_sel2info;
         std::string             _fnAngle;
         std::string             _fnDump;