Improve (Selection|FileName)OptionManager handling
authorTeemu Murtola <teemu.murtola@gmail.com>
Fri, 9 May 2014 04:40:21 +0000 (07:40 +0300)
committerDavid van der Spoel <davidvanderspoel@gmail.com>
Sat, 7 Jun 2014 13:33:00 +0000 (15:33 +0200)
The way the managers get passed to the option storage classes is now
reversed: the manager is passed already to the constructor.  This has
several benefits:
 - Already the constructor can use the manager if necessary.
 - It is possible to easily assert on all incorrect usage scenarios.
 - There is much less boilerplate code associated with propagating the
   manager into the storage classes.  There is also less code in
   general: even with an addition of a new file and its copyright
   header, as well as more extensive documentation, there are still
   fewer total lines with the new approach.

The minor disadvantage is that also option types that do not use a
manager get the container passed as an argument to createStorage(), but
they can just ignore the parameter, so the above benefits should clearly
outweigh this.

There was an outdated comment in options.h about the usage of
addSubSection(), which is fixed here.  It is now necessary to add
subsections before any options are added to them, but this is likely a
reasonable constraint for some other future stuff as well.

Change-Id: I4a5e69d3fd596dbe265613f81399219f8b8b19c8

26 files changed:
src/gromacs/commandline/pargs.cpp
src/gromacs/commandline/tests/cmdlinehelpwriter.cpp
src/gromacs/options/abstractoption.h
src/gromacs/options/basicoptions.cpp
src/gromacs/options/basicoptions.h
src/gromacs/options/filenameoption.cpp
src/gromacs/options/filenameoption.h
src/gromacs/options/filenameoptionmanager.cpp
src/gromacs/options/filenameoptionmanager.h
src/gromacs/options/filenameoptionstorage.h
src/gromacs/options/optionmanagercontainer.h [new file with mode: 0644]
src/gromacs/options/options-impl.h
src/gromacs/options/options.cpp
src/gromacs/options/options.h
src/gromacs/options/tests/abstractoptionstorage.cpp
src/gromacs/options/tests/filenameoption.cpp
src/gromacs/options/tests/optionsassigner.cpp
src/gromacs/selection/selectionfileoption.h
src/gromacs/selection/selectionfileoptionstorage.h
src/gromacs/selection/selectionoption.cpp
src/gromacs/selection/selectionoption.h
src/gromacs/selection/selectionoptionmanager.cpp
src/gromacs/selection/selectionoptionmanager.h
src/gromacs/selection/selectionoptionstorage.h
src/gromacs/selection/tests/selectionoption.cpp
src/gromacs/trajectoryanalysis/cmdlinerunner.cpp

index 151c3b9e504e6b8f5d3a818753844eb32ba76e1a..c5e2cd36647aeb9d9fe14a62946b4bc8ec50fbc8 100644 (file)
@@ -502,6 +502,7 @@ gmx_bool parse_common_args(int *argc, char *argv[], unsigned long Flags,
         gmx::Options               options(NULL, NULL);
         gmx::FileNameOptionManager fileOptManager;
 
+        options.addManager(&fileOptManager);
         options.setDescription(gmx::ConstArrayRef<const char *>(desc, ndesc));
         options.addOption(
                 gmx::IntegerOption("debug").store(&debug_level).hidden()
@@ -572,8 +573,6 @@ gmx_bool parse_common_args(int *argc, char *argv[], unsigned long Flags,
             adapter.pargsToOptions(&options, &pa[i]);
         }
 
-        setManagerForFileNameOptions(&options, &fileOptManager);
-
         const gmx::CommandLineHelpContext *context =
             gmx::GlobalCommandLineHelpContext::get();
         if (context != NULL)
index b8e5b3adc989eeea19b753e1b7419689b3c88e1e..5d0e7debab2c8740ef32bc20972c48be757dd71e 100644 (file)
@@ -223,15 +223,15 @@ TEST_F(CommandLineHelpWriterTest, HandlesSelectionOptions)
     using gmx::SelectionFileOption;
     using gmx::SelectionOption;
 
-    gmx::Options options(NULL, NULL);
+    gmx::Options                options(NULL, NULL);
+    gmx::SelectionCollection    selections;
+    gmx::SelectionOptionManager manager(&selections);
+    options.addManager(&manager);
     options.addOption(SelectionFileOption("sf"));
     options.addOption(SelectionOption("refsel").required()
                           .description("Reference selection option"));
     options.addOption(SelectionOption("sel").required().valueCount(2)
                           .description("Selection option"));
-    gmx::SelectionCollection    selections;
-    gmx::SelectionOptionManager manager(&selections);
-    setManagerForSelectionOptions(&options, &manager);
     options.finish();
     manager.parseRequestedFromString(
             "resname SOL;"
index ea365d2773b8f5fbf372e54537cdf1ef0789fcdc..4c0153b834b65a72d891c1db069715bc44883ba1 100644 (file)
@@ -67,6 +67,7 @@ namespace gmx
 
 class AbstractOptionStorage;
 template <typename T> class OptionStorageTemplate;
+class OptionManagerContainer;
 class Options;
 
 //! Smart pointer for managing an AbstractOptionStorage object.
@@ -108,8 +109,10 @@ class AbstractOption
         /*! \brief
          * Creates a default storage object for the option.
          *
-         * \returns The created storage object.
-         * \throws  APIError if invalid option settings have been provided.
+         * \param[in] managers  Manager container (unused if the option does
+         *     not use a manager).
+         * \returns   The created storage object.
+         * \throws    APIError if invalid option settings have been provided.
          *
          * This method is called by Options::addOption() when initializing an
          * option from the settings.
@@ -120,7 +123,8 @@ class AbstractOption
          *
          * Should only be called by Options::addOption().
          */
-        virtual AbstractOptionStoragePointer createStorage() const = 0;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const = 0;
 
         //! Sets the description for the option.
         void setDescription(const char *descr) { descr_ = descr; }
index f1af57aa32bed7ea3c075503fbfb457b610b6b42..205a3c102334b72dd5ca6e5dc83fce4a69df175b 100644 (file)
@@ -138,7 +138,8 @@ bool BooleanOptionInfo::defaultValue() const
  * BooleanOption
  */
 
-AbstractOptionStoragePointer BooleanOption::createStorage() const
+AbstractOptionStoragePointer
+BooleanOption::createStorage(const OptionManagerContainer & /*managers*/) const
 {
     return AbstractOptionStoragePointer(new BooleanOptionStorage(*this));
 }
@@ -195,7 +196,8 @@ IntegerOptionInfo::IntegerOptionInfo(IntegerOptionStorage *option)
  * IntegerOption
  */
 
-AbstractOptionStoragePointer IntegerOption::createStorage() const
+AbstractOptionStoragePointer
+IntegerOption::createStorage(const OptionManagerContainer & /*managers*/) const
 {
     return AbstractOptionStoragePointer(new IntegerOptionStorage(*this));
 }
@@ -242,7 +244,8 @@ Int64OptionInfo::Int64OptionInfo(Int64OptionStorage *option)
  * Int64Option
  */
 
-AbstractOptionStoragePointer Int64Option::createStorage() const
+AbstractOptionStoragePointer
+Int64Option::createStorage(const OptionManagerContainer & /*managers*/) const
 {
     return AbstractOptionStoragePointer(new Int64OptionStorage(*this));
 }
@@ -343,7 +346,8 @@ void DoubleOptionInfo::setScaleFactor(double factor)
  * DoubleOption
  */
 
-AbstractOptionStoragePointer DoubleOption::createStorage() const
+AbstractOptionStoragePointer
+DoubleOption::createStorage(const OptionManagerContainer & /*managers*/) const
 {
     return AbstractOptionStoragePointer(new DoubleOptionStorage(*this));
 }
@@ -446,7 +450,8 @@ void FloatOptionInfo::setScaleFactor(double factor)
  * FloatOption
  */
 
-AbstractOptionStoragePointer FloatOption::createStorage() const
+AbstractOptionStoragePointer
+FloatOption::createStorage(const OptionManagerContainer & /*managers*/) const
 {
     return AbstractOptionStoragePointer(new FloatOptionStorage(*this));
 }
@@ -631,7 +636,8 @@ const std::vector<std::string> &StringOptionInfo::allowedValues() const
  * StringOption
  */
 
-AbstractOptionStoragePointer StringOption::createStorage() const
+AbstractOptionStoragePointer
+StringOption::createStorage(const OptionManagerContainer & /*managers*/) const
 {
     return AbstractOptionStoragePointer(new StringOptionStorage(*this));
 }
index 35e91cc817ddb6145a87399a97baf1ea5cf5a578..992d19ed009db906e9fb5f092302b4a3cd4b655e 100644 (file)
@@ -98,7 +98,8 @@ class BooleanOption : public OptionTemplate<bool, BooleanOption>
 
     private:
         //! Creates a BooleanOptionStorage object.
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 };
 
 /*! \brief
@@ -140,7 +141,8 @@ class IntegerOption : public OptionTemplate<int, IntegerOption>
 
     private:
         //! Creates an IntegerOptionStorage object.
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 
         /*! \brief
          * Needed to initialize IntegerOptionStorage from this class without
@@ -169,7 +171,8 @@ class Int64Option : public OptionTemplate<gmx_int64_t, Int64Option>
 
     private:
         //! Creates an Int64OptionStorage object.
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 
         /*! \brief
          * Needed to initialize Int64OptionStorage from this class without
@@ -214,7 +217,8 @@ class DoubleOption : public OptionTemplate<double, DoubleOption>
 
     private:
         //! Creates a DoubleOptionStorage object.
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 
         bool bTime_;
 
@@ -252,7 +256,8 @@ class FloatOption : public OptionTemplate<float, FloatOption>
 
     private:
         //! Creates a FloatOptionStorage object.
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 
         bool bTime_;
 
@@ -372,7 +377,8 @@ class StringOption : public OptionTemplate<std::string, StringOption>
 
     private:
         //! Creates a StringOptionStorage object.
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 
         const char *const      *enumValues_;
         int                     enumValuesCount_;
index 9922e16fa977b6a2b3fc27bb7961745ac3324ee9..b56ba9e012d5a289da862563c102e53d71659d2f 100644 (file)
@@ -49,6 +49,7 @@
 
 #include "gromacs/fileio/filenm.h"
 #include "gromacs/options/filenameoptionmanager.h"
+#include "gromacs/options/optionmanagercontainer.h"
 #include "gromacs/utility/arrayref.h"
 #include "gromacs/utility/file.h"
 #include "gromacs/utility/gmxassert.h"
@@ -294,8 +295,9 @@ std::string completeFileName(const std::string &value, OptionFileType filetype,
  * FileNameOptionStorage
  */
 
-FileNameOptionStorage::FileNameOptionStorage(const FileNameOption &settings)
-    : MyBase(settings), info_(this), manager_(NULL),
+FileNameOptionStorage::FileNameOptionStorage(const FileNameOption  &settings,
+                                             FileNameOptionManager *manager)
+    : MyBase(settings), info_(this), manager_(manager),
       filetype_(settings.filetype_), legacyType_(settings.legacyType_),
       bRead_(settings.bRead_), bWrite_(settings.bWrite_),
       bLibrary_(settings.bLibrary_)
@@ -315,16 +317,6 @@ FileNameOptionStorage::FileNameOptionStorage(const FileNameOption &settings)
     }
 }
 
-void FileNameOptionStorage::setManager(FileNameOptionManager *manager)
-{
-    GMX_RELEASE_ASSERT(manager_ == NULL || manager_ == manager,
-                       "Manager cannot be changed once set");
-    if (manager_ == NULL)
-    {
-        manager_ = manager;
-    }
-}
-
 std::string FileNameOptionStorage::typeString() const
 {
     const FileTypeRegistry       &registry    = FileTypeRegistry::instance();
@@ -442,21 +434,11 @@ FileNameOptionInfo::FileNameOptionInfo(FileNameOptionStorage *option)
 {
 }
 
-FileNameOptionStorage &FileNameOptionInfo::option()
-{
-    return static_cast<FileNameOptionStorage &>(OptionInfo::option());
-}
-
 const FileNameOptionStorage &FileNameOptionInfo::option() const
 {
     return static_cast<const FileNameOptionStorage &>(OptionInfo::option());
 }
 
-void FileNameOptionInfo::setManager(FileNameOptionManager *manager)
-{
-    option().setManager(manager);
-}
-
 bool FileNameOptionInfo::isInputFile() const
 {
     return option().isInputFile();
@@ -491,9 +473,11 @@ FileNameOptionInfo::ExtensionList FileNameOptionInfo::extensions() const
  * FileNameOption
  */
 
-AbstractOptionStoragePointer FileNameOption::createStorage() const
+AbstractOptionStoragePointer
+FileNameOption::createStorage(const OptionManagerContainer &managers) const
 {
-    return AbstractOptionStoragePointer(new FileNameOptionStorage(*this));
+    return AbstractOptionStoragePointer(
+            new FileNameOptionStorage(*this, managers.get<FileNameOptionManager>()));
 }
 
 } // namespace gmx
index 45fd6fa1a9abab5fbf6b22d0686f11f41a16c0be..7b5abb146da9efdad654ca2e81dd5f3d2c505ab3 100644 (file)
@@ -165,7 +165,8 @@ class FileNameOption : public OptionTemplate<std::string, FileNameOption>
         using MyBase::defaultValueIfSet;
 
         //! Creates a FileNameOptionStorage object.
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 
         OptionFileType          filetype_;
         int                     legacyType_;
@@ -197,21 +198,6 @@ class FileNameOptionInfo : public OptionInfo
         //! Creates an option info object for the given option.
         explicit FileNameOptionInfo(FileNameOptionStorage *option);
 
-        /*! \brief
-         * Set manager for handling interaction with other options.
-         *
-         * \param   manager  File name option manager to set.
-         *
-         * This must be called before the values are added.
-         *
-         * Typically it is called through setManagerForFileNameOptions(),
-         * which recursively sets the manager for all file name options in
-         * an Options object.
-         *
-         * Does not throw.
-         */
-        void setManager(FileNameOptionManager *manager);
-
         //! Whether the option specifies an input file.
         bool isInputFile() const;
         //! Whether the option specifies an output file.
@@ -231,7 +217,6 @@ class FileNameOptionInfo : public OptionInfo
         ExtensionList extensions() const;
 
     private:
-        FileNameOptionStorage &option();
         const FileNameOptionStorage &option() const;
 };
 
index db7b2fde10e24ee25ebf66d929735df64426e965..3ba0aeac410719eb5bbb21c17828c22d72608d35 100644 (file)
@@ -46,7 +46,6 @@
 #include "gromacs/options/basicoptions.h"
 #include "gromacs/options/filenameoption.h"
 #include "gromacs/options/options.h"
-#include "gromacs/options/optionsvisitor.h"
 
 namespace gmx
 {
@@ -93,54 +92,4 @@ const std::string &FileNameOptionManager::defaultFileName() const
     return impl_->defaultFileName_;
 }
 
-/********************************************************************
- * Global functions
- */
-
-namespace
-{
-
-/*! \internal \brief
- * Visitor that sets the manager for each file name option.
- *
- * \ingroup module_options
- */
-class FileNameOptionManagerSetter : public OptionsModifyingVisitor
-{
-    public:
-        //! Construct a visitor that sets given manager.
-        explicit FileNameOptionManagerSetter(FileNameOptionManager *manager)
-            : manager_(manager)
-        {
-        }
-
-        void visitSubSection(Options *section)
-        {
-            OptionsModifyingIterator iterator(section);
-            iterator.acceptSubSections(this);
-            iterator.acceptOptions(this);
-        }
-
-        void visitOption(OptionInfo *option)
-        {
-            FileNameOptionInfo *fileOption
-                = option->toType<FileNameOptionInfo>();
-            if (fileOption != NULL)
-            {
-                fileOption->setManager(manager_);
-            }
-        }
-
-    private:
-        FileNameOptionManager *manager_;
-};
-
-}   // namespace
-
-void setManagerForFileNameOptions(Options               *options,
-                                  FileNameOptionManager *manager)
-{
-    FileNameOptionManagerSetter(manager).visitSubSection(options);
-}
-
 } // namespace gmx
index 24853ce7b75861bbbe1290591495abc3af1d5b00..caa96443639bc98acbf1409d98491a05ebe0722c 100644 (file)
@@ -45,6 +45,8 @@
 
 #include <string>
 
+#include "options.h"
+
 #include "../utility/common.h"
 
 namespace gmx
@@ -64,20 +66,20 @@ class Options;
  * most of the non-trivial file name completion logic in this class, so that
  * the customizations would be centralized here.
  *
- * Creating a FileNameOptionManager for an Options object is optional, even if
+ * Adding a FileNameOptionManager for an Options object is optional, even if
  * the Options contains FileNameOption options.  Features from the manager are
  * not available if the manager is not created, but otherwise the options work.
  *
- * \see setManagerForFileNameOptions()
+ * \see Options::addManager()
  *
  * \inpublicapi
  * \ingroup module_selection
  */
-class FileNameOptionManager
+class FileNameOptionManager : public OptionManagerInterface
 {
     public:
         FileNameOptionManager();
-        ~FileNameOptionManager();
+        virtual ~FileNameOptionManager();
 
         /*! \brief
          * Adds an option for setting the default global file name.
@@ -102,20 +104,6 @@ class FileNameOptionManager
         PrivateImplPointer<Impl> impl_;
 };
 
-/*! \brief
- * Set manager for all file name options.
- *
- * Recursively sets the manager to \p manager for all file name options in
- * \p options.
- * Must be called before value assignment starts for \p options.
- *
- * Does not throw.
- *
- * \inpublicapi
- */
-void setManagerForFileNameOptions(Options               *options,
-                                  FileNameOptionManager *manager);
-
 } // namespace gmx
 
 #endif
index e90470775879e512628012111518ae924e9fead8..0586bbae887444bfb48ff61c9b2e84afadae89bc 100644 (file)
@@ -60,17 +60,20 @@ class FileNameOptionManager;
 class FileNameOptionStorage : public OptionStorageTemplate<std::string>
 {
     public:
-        //! \copydoc StringOptionStorage::StringOptionStorage()
-        explicit FileNameOptionStorage(const FileNameOption &settings);
+        /*! \brief
+         * Initializes the storage from option settings.
+         *
+         * \param[in] settings   Storage settings.
+         * \param     manager    Manager for this object (can be NULL).
+         */
+        FileNameOptionStorage(const FileNameOption  &settings,
+                              FileNameOptionManager *manager);
 
         virtual OptionInfo &optionInfo() { return info_; }
         virtual std::string typeString() const;
         virtual std::string formatExtraDescription() const;
         virtual std::string formatSingleValue(const std::string &value) const;
 
-        //! \copydoc FileNameOptionInfo::setManager()
-        void setManager(FileNameOptionManager *manager);
-
         //! \copydoc FileNameOptionInfo::isInputFile()
         bool isInputFile() const { return bRead_ && !bWrite_; }
         //! \copydoc FileNameOptionInfo::isOutputFile()
diff --git a/src/gromacs/options/optionmanagercontainer.h b/src/gromacs/options/optionmanagercontainer.h
new file mode 100644 (file)
index 0000000..b282cd6
--- /dev/null
@@ -0,0 +1,124 @@
+/*
+ * This file is part of the GROMACS molecular simulation package.
+ *
+ * Copyright (c) 2014, 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.
+ *
+ * GROMACS is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1
+ * of the License, or (at your option) any later version.
+ *
+ * GROMACS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with GROMACS; if not, see
+ * http://www.gnu.org/licenses, or write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+ *
+ * If you want to redistribute modifications to GROMACS, please
+ * consider that scientific software is very special. Version
+ * control is crucial - bugs must be traceable. We will be happy to
+ * consider code for inclusion in the official distribution, but
+ * derived work must not be called official GROMACS. Details are found
+ * in the README & COPYING files - if they are missing, get the
+ * official version at http://www.gromacs.org.
+ *
+ * To help us fund GROMACS development, we humbly ask that you cite
+ * the research papers on the package. Check out http://www.gromacs.org.
+ */
+/*! \libinternal \file
+ * \brief
+ * Declares gmx::OptionManagerContainer.
+ *
+ * \author Teemu Murtola <teemu.murtola@gmail.com>
+ * \inlibraryapi
+ * \ingroup module_options
+ */
+#ifndef GMX_OPTIONS_OPTIONMANAGERCONTAINER_H
+#define GMX_OPTIONS_OPTIONMANAGERCONTAINER_H
+
+#include <vector>
+
+#include "gromacs/utility/common.h"
+#include "gromacs/utility/gmxassert.h"
+
+namespace gmx
+{
+
+class OptionManagerInterface;
+
+/*! \libinternal
+ * \brief
+ * Container to keep managers added with Options::addManager() and pass them
+ * to options.
+ *
+ * Consistency of the managers (e.g., that there is at most one manager of a
+ * certain type) is only checked when the managers are accessed.
+ *
+ * \inlibraryapi
+ * \ingroup module_options
+ */
+class OptionManagerContainer
+{
+    public:
+        OptionManagerContainer()
+        {
+        }
+
+        //! Returns `true` if there are no managers.
+        bool empty() const { return list_.empty(); }
+
+        //! Adds a manager to the container.
+        void add(OptionManagerInterface *manager)
+        {
+            list_.push_back(manager);
+        }
+        /*! \brief
+         * Retrieves a manager of a certain type.
+         *
+         * \tparam  ManagerType  Type of manager to retrieve
+         *     (should derive from OptionManagerInterface).
+         * \returns The manager, or `NULL` if there is none.
+         *
+         * This method is used in AbstractOption::createStorage() to retrieve
+         * a manager of a certain type for options that use a manager.
+         *
+         * The return value is `NULL` if there is no manager of the given type.
+         * The caller needs to handle this (either by asserting, or by handling
+         * the manager as optional).
+         */
+        template <class ManagerType>
+        ManagerType *get() const
+        {
+            ManagerType *result = NULL;
+            for (ListType::const_iterator i = list_.begin(); i != list_.end(); ++i)
+            {
+                ManagerType *curr = dynamic_cast<ManagerType *>(*i);
+                if (curr != NULL)
+                {
+                    GMX_RELEASE_ASSERT(result == NULL,
+                                       "More than one applicable option manager is set");
+                    result = curr;
+                }
+            }
+            return result;
+        }
+
+    private:
+        //! Shorthand for the internal container type.
+        typedef std::vector<OptionManagerInterface *> ListType;
+
+        ListType  list_;
+
+        GMX_DISALLOW_COPY_AND_ASSIGN(OptionManagerContainer);
+};
+
+} // namespace gmx
+
+#endif
index 6b06f1fcd7c4820941f66733dd3be0b91b53843a..c126f07e8579236a660aa2c9ca46a3d6daee7a71 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012,2013, by the GROMACS development team, led by
+ * Copyright (c) 2010,2011,2012,2013,2014, 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.
@@ -46,6 +46,7 @@
 #include <vector>
 
 #include "abstractoption.h"
+#include "optionmanagercontainer.h"
 #include "options.h"
 
 namespace gmx
@@ -107,6 +108,12 @@ class Options::Impl
         std::string             title_;
         //! Full description for the Options object.
         std::string             description_;
+        /*! \brief
+         * Option managers set for this collection.
+         *
+         * This is non-empty only for the top-level Options object.
+         */
+        OptionManagerContainer  managers_;
         /*! \brief
          * List of subsections, in insertion order.
          *
index 8b3c22e2646ec3970f1bff7054c397d1c187d1e9..b64f285268d408a51d31db01e27301eeb59390c6 100644 (file)
 namespace gmx
 {
 
+/********************************************************************
+ * OptionManagerInterface
+ */
+
+OptionManagerInterface::~OptionManagerInterface()
+{
+}
+
 /********************************************************************
  * Options::Impl
  */
@@ -148,8 +156,28 @@ void Options::setDescription(const ConstArrayRef<const char *> &descArray)
     impl_->description_ = concatenateStrings(descArray.data(), descArray.size());
 }
 
+void Options::addManager(OptionManagerInterface *manager)
+{
+    GMX_RELEASE_ASSERT(impl_->parent_ == NULL,
+                       "Can only add a manager in a top-level Options object");
+    // This ensures that all options see the same set of managers.
+    GMX_RELEASE_ASSERT(impl_->options_.empty(),
+                       "Can only add a manager before options");
+    // This check could be relaxed if we instead checked that the subsections
+    // do not have options.
+    GMX_RELEASE_ASSERT(impl_->subSections_.empty(),
+                       "Can only add a manager before subsections");
+    impl_->managers_.add(manager);
+}
+
 void Options::addSubSection(Options *section)
 {
+    // This is required, because managers are used from the root Options
+    // object, so they are only seen after the subsection has been added.
+    GMX_RELEASE_ASSERT(section->impl_->options_.empty(),
+                       "Can only add a subsection before it has any options");
+    GMX_RELEASE_ASSERT(section->impl_->managers_.empty(),
+                       "Can only have managers in a top-level Options object");
     // Make sure that section is not already inserted somewhere.
     GMX_RELEASE_ASSERT(section->impl_->parent_ == NULL,
                        "Cannot add as subsection twice");
@@ -162,7 +190,12 @@ void Options::addSubSection(Options *section)
 
 OptionInfo *Options::addOption(const AbstractOption &settings)
 {
-    AbstractOptionStoragePointer option(settings.createStorage());
+    Options::Impl *root = impl_.get();
+    while (root->parent_ != NULL)
+    {
+        root = root->parent_->impl_.get();
+    }
+    AbstractOptionStoragePointer option(settings.createStorage(root->managers_));
     if (impl_->findOption(option->name().c_str()) != NULL)
     {
         GMX_THROW(APIError("Duplicate option: " + option->name()));
index cd5f1dc7b012c1cc5da47db40b596c08214c0aa2..7623086796c57ac63347d1afcafec2dc2fea5a22 100644 (file)
@@ -62,6 +62,27 @@ class AbstractOption;
 class OptionsAssigner;
 class OptionsIterator;
 
+/*! \brief
+ * Base class for option managers.
+ *
+ * This class is used as a marker for all classes that are used with
+ * Options::addManager().  It doesn't provide any methods, but only supports
+ * transporting these classes through the Options collection into the
+ * individual option implementation classes.
+ *
+ * The virtual destructor is present to make this class polymorphic, such that
+ * `dynamic_cast` can be used when retrieving a manager of a certain type for
+ * the individual options.
+ *
+ * \inlibraryapi
+ * \ingroup module_options
+ */
+class OptionManagerInterface
+{
+    protected:
+        virtual ~OptionManagerInterface();
+};
+
 /*! \brief
  * Collection of options.
  *
@@ -152,6 +173,28 @@ class Options
          */
         void setDescription(const ConstArrayRef<const char *> &descArray);
 
+        /*! \brief
+         * Adds an option manager.
+         *
+         * \param    manager Manager to add.
+         * \throws   std::bad_alloc if out of memory.
+         *
+         * Option managers are used by some types of options that require
+         * interaction between different option instances (e.g., selection
+         * options), or need to support globally set properties (e.g., a global
+         * default file prefix).  Option objects can retrieve the pointer to
+         * their manager when they are created, and the caller can alter the
+         * behavior of the options through the manager.
+         * See the individual managers for details.
+         *
+         * Caller is responsible for memory management of \p manager.
+         * The Options object (and its contained options) only stores a
+         * reference to the object.
+         *
+         * This method cannot be called after adding options or subsections.
+         */
+        void addManager(OptionManagerInterface *manager);
+
         /*! \brief
          * Adds an option collection as a subsection of this collection.
          *
@@ -161,9 +204,8 @@ class Options
          * subsection.  If an attempt is made to add two different subsections
          * with the same name, this function asserts.
          *
-         * For certain functionality to work properly, no options should
-         * be added to the subsection after it has been added to another
-         * collection.
+         * \p section should not have any options added at the point this
+         * method is called.
          *
          * Only a pointer to the provided object is stored.  The caller is
          * responsible that the object exists for the lifetime of the
index 716e0b1f56de278c8b101c4d6015b84ad2a7f437..6df36ee6eb63c1f1654232ca282de8a37bf1ecca 100644 (file)
@@ -151,7 +151,8 @@ class MockOption : public gmx::OptionTemplate<std::string, MockOption>
         }
 
     private:
-        virtual gmx::AbstractOptionStoragePointer createStorage() const
+        virtual gmx::AbstractOptionStoragePointer createStorage(
+            const gmx::OptionManagerContainer & /*managers*/) const
         {
             return gmx::AbstractOptionStoragePointer(new MockOptionStorage(*this));
         }
index 949928c182328cc2569493ef81f54fb5284fa822..be0acd969b8fa9084c440b52be7e5f2a59f24cf4 100644 (file)
@@ -237,12 +237,12 @@ TEST(FileNameOptionTest, AddsMissingExtensionForRequiredFromDefaultNameOptionBas
     gmx::FileNameOptionManager manager;
     gmx::Options               options(NULL, NULL);
     std::string                value;
+    ASSERT_NO_THROW_GMX(options.addManager(&manager));
     ASSERT_NO_THROW_GMX(options.addOption(
                                 FileNameOption("f").store(&value).required()
                                     .filetype(gmx::eftTrajectory).inputFile()
                                     .defaultBasename("foo")));
     ASSERT_NO_THROW_GMX(manager.addDefaultFileNameOption(&options, "deffnm"));
-    ASSERT_NO_THROW_GMX(setManagerForFileNameOptions(&options, &manager));
     EXPECT_EQ("foo.xtc", value);
 
     gmx::OptionsAssigner assigner(&options);
@@ -266,12 +266,12 @@ TEST(FileNameOptionTest, AddsMissingExtensionForOptionalFromDefaultNameOptionBas
     gmx::FileNameOptionManager manager;
     gmx::Options               options(NULL, NULL);
     std::string                value;
+    ASSERT_NO_THROW_GMX(options.addManager(&manager));
     ASSERT_NO_THROW_GMX(options.addOption(
                                 FileNameOption("f").store(&value)
                                     .filetype(gmx::eftTrajectory).inputFile()
                                     .defaultBasename("foo")));
     ASSERT_NO_THROW_GMX(manager.addDefaultFileNameOption(&options, "deffnm"));
-    ASSERT_NO_THROW_GMX(setManagerForFileNameOptions(&options, &manager));
 
     gmx::OptionsAssigner assigner(&options);
     EXPECT_NO_THROW_GMX(assigner.start());
index e12d787ba2c29ff7ce3222a5fe123106c58aa992..8fb44f0bd4b9e3113a4db68fab42c45c5750c3b1 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012,2013, by the GROMACS development team, led by
+ * Copyright (c) 2010,2011,2012,2013,2014, 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.
@@ -183,11 +183,11 @@ TEST(OptionsAssignerTest, HandlesSubSections)
     int          value1 = 1;
     int          value2 = 2;
     using gmx::IntegerOption;
+    ASSERT_NO_THROW(options.addSubSection(&sub1));
+    ASSERT_NO_THROW(options.addSubSection(&sub2));
     ASSERT_NO_THROW(options.addOption(IntegerOption("p").store(&value)));
     ASSERT_NO_THROW(sub1.addOption(IntegerOption("p").store(&value1)));
     ASSERT_NO_THROW(sub2.addOption(IntegerOption("p").store(&value2)));
-    ASSERT_NO_THROW(options.addSubSection(&sub1));
-    ASSERT_NO_THROW(options.addSubSection(&sub2));
 
     gmx::OptionsAssigner assigner(&options);
     EXPECT_NO_THROW(assigner.start());
@@ -223,13 +223,13 @@ TEST(OptionsAssignerTest, HandlesNoStrictSubSections)
     int          pvalue2 = 2;
     int          rvalue  = 5;
     using gmx::IntegerOption;
+    ASSERT_NO_THROW(options.addSubSection(&sub1));
+    ASSERT_NO_THROW(options.addSubSection(&sub2));
     ASSERT_NO_THROW(options.addOption(IntegerOption("p").store(&pvalue)));
     ASSERT_NO_THROW(sub1.addOption(IntegerOption("p").store(&pvalue1)));
     ASSERT_NO_THROW(sub1.addOption(IntegerOption("q").store(&qvalue)));
     ASSERT_NO_THROW(sub2.addOption(IntegerOption("p").store(&pvalue2)));
     ASSERT_NO_THROW(sub2.addOption(IntegerOption("r").store(&rvalue)));
-    ASSERT_NO_THROW(options.addSubSection(&sub1));
-    ASSERT_NO_THROW(options.addSubSection(&sub2));
 
     gmx::OptionsAssigner assigner(&options);
     assigner.setNoStrictSectioning(true);
index c0a70dad005beb6cc5ba8231b76b61d5f05bc1d7..773bff96b3821a77a663aa5946537dc0d888c24f 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2012, by the GROMACS development team, led by
+ * Copyright (c) 2012,2014, 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.
@@ -76,7 +76,8 @@ class SelectionFileOption : public AbstractOption
         explicit SelectionFileOption(const char *name);
 
     private:
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 };
 
 /*! \libinternal \brief
@@ -94,13 +95,6 @@ class SelectionFileOptionInfo : public OptionInfo
          * Does not throw.
          */
         explicit SelectionFileOptionInfo(SelectionFileOptionStorage *option);
-
-        //! \copydoc SelectionOptionInfo::setManager()
-        void setManager(SelectionOptionManager *manager);
-
-    private:
-        SelectionFileOptionStorage &option();
-        const SelectionFileOptionStorage &option() const;
 };
 
 } // namespace gmx
index a19321962fe2d2314b490cec50c456b721da8f16..ac628bf7ba26a7b47ab034a0614b2ed883d8dbb5 100644 (file)
@@ -63,29 +63,25 @@ class SelectionFileOptionStorage : public AbstractOptionStorage
          * Initializes the storage from option settings.
          *
          * \param[in] settings   Storage settings.
+         * \param     manager    Manager for this object.
          */
-        SelectionFileOptionStorage(const SelectionFileOption &settings);
+        SelectionFileOptionStorage(const SelectionFileOption &settings,
+                                   SelectionOptionManager    *manager);
 
         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 ""; }
 
-        //! \copydoc SelectionFileOptionInfo::setManager()
-        void setManager(SelectionOptionManager *manager)
-        {
-            manager_ = manager;
-        }
-
     private:
         virtual void clearSet();
         virtual void convertValue(const std::string &value);
         virtual void processSet();
         virtual void processAll() {}
 
-        SelectionFileOptionInfo info_;
-        SelectionOptionManager *manager_;
-        bool                    bValueParsed_;
+        SelectionFileOptionInfo  info_;
+        SelectionOptionManager  &manager_;
+        bool                     bValueParsed_;
 };
 
 } // namespace gmx
index 41da1f5de019f73d4364cc9fe94ca04b2ddbc66f..62fbc9cf50d32933cf5ebb61dfb30493f34ba0ba 100644 (file)
@@ -46,6 +46,7 @@
 
 #include <string>
 
+#include "gromacs/options/optionmanagercontainer.h"
 #include "gromacs/selection/selection.h"
 #include "gromacs/selection/selectionoptionmanager.h"
 #include "gromacs/utility/exceptions.h"
@@ -59,26 +60,18 @@ namespace gmx
  * SelectionOptionStorage
  */
 
-SelectionOptionStorage::SelectionOptionStorage(const SelectionOption &settings)
+SelectionOptionStorage::SelectionOptionStorage(const SelectionOption  &settings,
+                                               SelectionOptionManager *manager)
     : MyBase(settings, OptionFlags() | efOption_NoDefaultValue
              | efOption_DontCheckMinimumCount),
-      info_(this), manager_(NULL), defaultText_(settings.defaultText_),
+      info_(this), manager_(*manager), defaultText_(settings.defaultText_),
       selectionFlags_(settings.selectionFlags_)
 {
+    GMX_RELEASE_ASSERT(manager != NULL,
+                       "SelectionOptionManager must be added before SelectionOption");
     GMX_RELEASE_ASSERT(!hasFlag(efOption_MultipleTimes),
                        "allowMultiple() is not supported for selection options");
-}
-
-
-void SelectionOptionStorage::setManager(SelectionOptionManager *manager)
-{
-    GMX_RELEASE_ASSERT(manager_ == NULL || manager_ == manager,
-                       "Manager cannot be changed once set");
-    if (manager_ == NULL)
-    {
-        manager->registerOption(this);
-        manager_ = manager;
-    }
+    manager_.registerOption(this);
 }
 
 
@@ -124,18 +117,14 @@ void SelectionOptionStorage::addSelections(
 
 void SelectionOptionStorage::convertValue(const std::string &value)
 {
-    GMX_RELEASE_ASSERT(manager_ != NULL, "Manager is not set");
-
-    manager_->convertOptionValue(this, value, false);
+    manager_.convertOptionValue(this, value, false);
 }
 
 void SelectionOptionStorage::processSetValues(ValueList *values)
 {
-    GMX_RELEASE_ASSERT(manager_ != NULL, "Manager is not set");
-
     if (values->size() == 0)
     {
-        manager_->requestOptionDelayedParsing(this);
+        manager_.requestOptionDelayedParsing(this);
     }
     else if (values->size() < static_cast<size_t>(minValueCount()))
     {
@@ -145,14 +134,13 @@ void SelectionOptionStorage::processSetValues(ValueList *values)
 
 void SelectionOptionStorage::processAll()
 {
-    GMX_RELEASE_ASSERT(manager_ != NULL, "Manager is not set");
     if (!isSet() && !defaultText_.empty())
     {
-        manager_->convertOptionValue(this, defaultText_, true);
+        manager_.convertOptionValue(this, defaultText_, true);
     }
     if (isRequired() && !isSet())
     {
-        manager_->requestOptionDelayedParsing(this);
+        manager_.requestOptionDelayedParsing(this);
         markAsSet();
     }
 }
@@ -227,11 +215,6 @@ const SelectionOptionStorage &SelectionOptionInfo::option() const
     return static_cast<const SelectionOptionStorage &>(OptionInfo::option());
 }
 
-void SelectionOptionInfo::setManager(SelectionOptionManager *manager)
-{
-    option().setManager(manager);
-}
-
 void SelectionOptionInfo::setValueCount(int count)
 {
     option().setAllowedValueCount(count);
@@ -267,9 +250,12 @@ void SelectionOptionInfo::setDynamicMask(bool bEnabled)
  * SelectionOption
  */
 
-AbstractOptionStoragePointer SelectionOption::createStorage() const
+AbstractOptionStoragePointer
+SelectionOption::createStorage(const OptionManagerContainer &managers) const
 {
-    return AbstractOptionStoragePointer(new SelectionOptionStorage(*this));
+    return AbstractOptionStoragePointer(
+            new SelectionOptionStorage(
+                    *this, managers.get<SelectionOptionManager>()));
 }
 
 
@@ -277,11 +263,14 @@ AbstractOptionStoragePointer SelectionOption::createStorage() const
  * SelectionFileOptionStorage
  */
 
-SelectionFileOptionStorage::SelectionFileOptionStorage(const SelectionFileOption &settings)
+SelectionFileOptionStorage::SelectionFileOptionStorage(
+        const SelectionFileOption &settings, SelectionOptionManager *manager)
     : AbstractOptionStorage(settings, OptionFlags() | efOption_MultipleTimes
                             | efOption_DontCheckMinimumCount),
-      info_(this), manager_(NULL), bValueParsed_(false)
+      info_(this), manager_(*manager), bValueParsed_(false)
 {
+    GMX_RELEASE_ASSERT(manager != NULL,
+                       "SelectionOptionManager must be added before SelectionFileOption");
 }
 
 void SelectionFileOptionStorage::clearSet()
@@ -291,15 +280,13 @@ void SelectionFileOptionStorage::clearSet()
 
 void SelectionFileOptionStorage::convertValue(const std::string &value)
 {
-    GMX_RELEASE_ASSERT(manager_ != NULL, "Manager is not set");
-
     if (bValueParsed_)
     {
         GMX_THROW(InvalidInputError("More than one file name provided"));
     }
     bValueParsed_ = true;
     // TODO: Should we throw an InvalidInputError if the file does not exist?
-    manager_->parseRequestedFromFile(value);
+    manager_.parseRequestedFromFile(value);
 }
 
 void SelectionFileOptionStorage::processSet()
@@ -320,21 +307,6 @@ SelectionFileOptionInfo::SelectionFileOptionInfo(SelectionFileOptionStorage *opt
 {
 }
 
-SelectionFileOptionStorage &SelectionFileOptionInfo::option()
-{
-    return static_cast<SelectionFileOptionStorage &>(OptionInfo::option());
-}
-
-const SelectionFileOptionStorage &SelectionFileOptionInfo::option() const
-{
-    return static_cast<const SelectionFileOptionStorage &>(OptionInfo::option());
-}
-
-void SelectionFileOptionInfo::setManager(SelectionOptionManager *manager)
-{
-    option().setManager(manager);
-}
-
 
 /********************************************************************
  * SelectionFileOption
@@ -346,9 +318,12 @@ SelectionFileOption::SelectionFileOption(const char *name)
     setDescription("Provide selections from files");
 }
 
-AbstractOptionStoragePointer SelectionFileOption::createStorage() const
+AbstractOptionStoragePointer
+SelectionFileOption::createStorage(const OptionManagerContainer &managers) const
 {
-    return AbstractOptionStoragePointer(new SelectionFileOptionStorage(*this));
+    return AbstractOptionStoragePointer(
+            new SelectionFileOptionStorage(
+                    *this, managers.get<SelectionOptionManager>()));
 }
 
 } // namespace gmx
index cba014a055ab048070dfc87dd061f6edcf477e69..a67e4e337ba1d975942d26120b81d32432e91318 100644 (file)
@@ -60,6 +60,10 @@ class SelectionOptionStorage;
  *
  * Public methods in this class do not throw.
  *
+ * To use options of this type, SelectionOptionManager must first be added to
+ * the Options collection.  For trajectory analysis tools, the framework takes
+ * care of this.
+ *
  * \todo
  * Support for specifying that an option accepts, e.g., two to four selections.
  * Currently, only a fixed count or any number of selections is possible.
@@ -145,7 +149,8 @@ class SelectionOption : public OptionTemplate<Selection, SelectionOption>
         using MyBase::defaultValue;
         using MyBase::defaultValueIfSet;
 
-        virtual AbstractOptionStoragePointer createStorage() const;
+        virtual AbstractOptionStoragePointer createStorage(
+            const OptionManagerContainer &managers) const;
 
         const char             *defaultText_;
         SelectionFlags          selectionFlags_;
@@ -211,22 +216,6 @@ class SelectionOptionInfo : public OptionInfo
          */
         explicit SelectionOptionInfo(SelectionOptionStorage *option);
 
-        /*! \brief
-         * Set manager for handling interaction with other options and the
-         * selection collection.
-         *
-         * \param   manager  Selection manager to set.
-         *
-         * This must be called before the values are added.
-         *
-         * Typically it is called through setManagerForSelectionOptions(),
-         * which recursively sets the manager for all selection options in
-         * an Options object.
-         *
-         * Does not throw.
-         */
-        void setManager(SelectionOptionManager *manager);
-
         /*! \brief
          * Sets the number of selections allowed for the option.
          *
index 3600bd29233994a659941d064dcb21cd43be3b4b..effb758ba09914d4bcceb5de6fda2d7ffbda21a5 100644 (file)
@@ -43,7 +43,6 @@
 
 #include <cstdio>
 
-#include "gromacs/options/optionsvisitor.h"
 #include "gromacs/selection/selection.h"
 #include "gromacs/selection/selectioncollection.h"
 #include "gromacs/selection/selectionoption.h"
@@ -331,60 +330,4 @@ SelectionOptionManager::parseRequestedFromString(const std::string &str)
     impl_->placeSelectionsInRequests(selections);
 }
 
-/********************************************************************
- * Global functions
- */
-
-namespace
-{
-
-/*! \internal \brief
- * Visitor that sets the manager for each selection option.
- *
- * \ingroup module_selection
- */
-class SelectionOptionManagerSetter : public OptionsModifyingVisitor
-{
-    public:
-        //! Construct a visitor that sets given manager.
-        explicit SelectionOptionManagerSetter(SelectionOptionManager *manager)
-            : manager_(manager)
-        {
-        }
-
-        void visitSubSection(Options *section)
-        {
-            OptionsModifyingIterator iterator(section);
-            iterator.acceptSubSections(this);
-            iterator.acceptOptions(this);
-        }
-
-        void visitOption(OptionInfo *option)
-        {
-            SelectionOptionInfo *selOption
-                = option->toType<SelectionOptionInfo>();
-            if (selOption != NULL)
-            {
-                selOption->setManager(manager_);
-            }
-            SelectionFileOptionInfo *selFileOption
-                = option->toType<SelectionFileOptionInfo>();
-            if (selFileOption != NULL)
-            {
-                selFileOption->setManager(manager_);
-            }
-        }
-
-    private:
-        SelectionOptionManager *manager_;
-};
-
-}   // namespace
-
-void setManagerForSelectionOptions(Options                *options,
-                                   SelectionOptionManager *manager)
-{
-    SelectionOptionManagerSetter(manager).visitSubSection(options);
-}
-
 } // namespace gmx
index 8545661bbf2d2115cc71ee4aa5617c5327e8cf61..ec26193b58876a7a4e5ffae6a44622fa0a61dfa6 100644 (file)
@@ -45,6 +45,7 @@
 
 #include <string>
 
+#include "../options/options.h"
 #include "../utility/common.h"
 
 namespace gmx
@@ -62,6 +63,8 @@ class SelectionOptionStorage;
  * require actions outside options parsing.
  * It also implements the coupling between SelectionOption and
  * SelectionFileOption.
+ * It needs to be added using Options::addManager() before SelectionOption or
+ * SelectionFileOption options can be added to an Options collection.
  *
  * The main features of this class are:
  *  - convertOptionValue(), which is used to convert string values into
@@ -73,12 +76,10 @@ class SelectionOptionStorage;
  *    parseRequestedFromStdin(), parseRequestedFromFile() or
  *    parseRequstedFromString().
  *
- * \see setManagerForSelectionOptions()
- *
  * \inpublicapi
  * \ingroup module_selection
  */
-class SelectionOptionManager
+class SelectionOptionManager : public OptionManagerInterface
 {
     public:
         /*! \brief
@@ -87,7 +88,7 @@ class SelectionOptionManager
          * \throws  std::bad_alloc if out of memory.
          */
         explicit SelectionOptionManager(SelectionCollection *selections);
-        ~SelectionOptionManager();
+        virtual ~SelectionOptionManager();
 
         /*! \brief
          * Adds a selection option to be managed.
@@ -209,20 +210,6 @@ class SelectionOptionManager
         friend class SelectionOptionStorage;
 };
 
-/*! \brief
- * Set manager for all selection options.
- *
- * Recursively sets the manager to \p manager for all selection options in
- * \p options.
- * Must be called before value assignment starts for \p options.
- *
- * Does not throw.
- *
- * \inpublicapi
- */
-void setManagerForSelectionOptions(Options                *options,
-                                   SelectionOptionManager *manager);
-
 } // namespace gmx
 
 #endif
index 00e3622ba82717d2811090dc789c7e9703308524..fb9300d2b10eda04dba0e7fb1bb3ad5271c44c95 100644 (file)
@@ -69,16 +69,15 @@ class SelectionOptionStorage : public OptionStorageTemplate<Selection>
          * Initializes the storage from option settings.
          *
          * \param[in] settings   Storage settings.
+         * \param     manager    Manager for this object.
          */
-        SelectionOptionStorage(const SelectionOption &settings);
+        SelectionOptionStorage(const SelectionOption  &settings,
+                               SelectionOptionManager *manager);
 
         virtual OptionInfo &optionInfo() { return info_; }
         virtual std::string typeString() const { return "selection"; }
         virtual std::string formatSingleValue(const Selection &value) const;
 
-        //! \copydoc SelectionOptionInfo::setManager()
-        void setManager(SelectionOptionManager *manager);
-
         /*! \brief
          * Adds selections to the storage.
          *
@@ -127,7 +126,7 @@ class SelectionOptionStorage : public OptionStorageTemplate<Selection>
         virtual void processAll();
 
         SelectionOptionInfo     info_;
-        SelectionOptionManager *manager_;
+        SelectionOptionManager &manager_;
         std::string             defaultText_;
         SelectionFlags          selectionFlags_;
 };
index 4c464006905a2313c77200627d1b16c8e18b189c..dc5c3ec79967191432a7f17e0bfdfbe4a91b6c63 100644 (file)
@@ -69,7 +69,6 @@ class SelectionOptionTestBase : public ::testing::Test
     public:
         SelectionOptionTestBase();
 
-        void setManager();
         void loadTopology(const char *filename);
 
         gmx::SelectionCollection    sc_;
@@ -83,15 +82,11 @@ class SelectionOptionTestBase : public ::testing::Test
 SelectionOptionTestBase::SelectionOptionTestBase()
     : manager_(&sc_), options_(NULL, NULL)
 {
+    options_.addManager(&manager_);
     sc_.setReferencePosType("atom");
     sc_.setOutputPosType("atom");
 }
 
-void SelectionOptionTestBase::setManager()
-{
-    setManagerForSelectionOptions(&options_, &manager_);
-}
-
 void SelectionOptionTestBase::loadTopology(const char *filename)
 {
     topManager_.loadTopology(filename);
@@ -112,7 +107,6 @@ TEST_F(SelectionOptionTest, ParsesSimpleSelection)
     gmx::Selection sel;
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(SelectionOption("sel").store(&sel)));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -132,7 +126,6 @@ TEST_F(SelectionOptionTest, HandlesDynamicSelectionWhenStaticRequired)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").store(&sel).onlyStatic()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -150,7 +143,6 @@ TEST_F(SelectionOptionTest, HandlesNonAtomicSelectionWhenAtomsRequired)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").store(&sel).onlyAtoms()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -171,7 +163,6 @@ TEST_F(SelectionOptionTest, ChecksEmptySelections)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").store(&sel)));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -191,7 +182,6 @@ TEST_F(SelectionOptionTest, ChecksEmptyDelayedSelections)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").store(&sel)));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -210,7 +200,6 @@ TEST_F(SelectionOptionTest, HandlesTooManySelections)
     gmx::Selection sel;
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(SelectionOption("sel").store(&sel)));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -229,7 +218,6 @@ TEST_F(SelectionOptionTest, HandlesTooFewSelections)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").store(sel).valueCount(2)));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -247,7 +235,6 @@ TEST_F(SelectionOptionTest, HandlesDefaultSelectionText)
     using gmx::SelectionOption;
     options_.addOption(SelectionOption("sel").store(&sel)
                            .defaultSelectionText("all"));
-    setManager();
 
     EXPECT_NO_THROW_GMX(options_.finish());
 
@@ -266,7 +253,6 @@ TEST_F(SelectionOptionTest, HandlesAdjuster)
     using gmx::SelectionOption;
     gmx::SelectionOptionInfo *info = options_.addOption(
                 SelectionOption("sel").storeVector(&sel).multiValue());
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -286,7 +272,6 @@ TEST_F(SelectionOptionTest, HandlesDynamicWhenStaticRequiredWithAdjuster)
     using gmx::SelectionOption;
     gmx::SelectionOptionInfo *info = options_.addOption(
                 SelectionOption("sel").store(&sel));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -305,7 +290,6 @@ TEST_F(SelectionOptionTest, HandlesTooManySelectionsWithAdjuster)
     using gmx::SelectionOption;
     gmx::SelectionOptionInfo *info = options_.addOption(
                 SelectionOption("sel").storeVector(&sel).multiValue());
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -325,7 +309,6 @@ TEST_F(SelectionOptionTest, HandlesTooFewSelectionsWithAdjuster)
     using gmx::SelectionOption;
     gmx::SelectionOptionInfo *info = options_.addOption(
                 SelectionOption("sel").storeVector(&sel).multiValue());
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -344,7 +327,6 @@ TEST_F(SelectionOptionTest, HandlesDelayedRequiredSelection)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").store(&sel).required()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -362,7 +344,6 @@ TEST_F(SelectionOptionTest, HandlesTooFewDelayedRequiredSelections)
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").store(sel).required()
                                     .valueCount(2)));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -377,7 +358,6 @@ TEST_F(SelectionOptionTest, HandlesDelayedOptionalSelection)
     gmx::Selection sel;
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(SelectionOption("sel").store(&sel)));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -396,7 +376,6 @@ TEST_F(SelectionOptionTest, HandlesDelayedSelectionWithAdjuster)
     using gmx::SelectionOption;
     gmx::SelectionOptionInfo *info = options_.addOption(
                 SelectionOption("sel").storeVector(&sel).valueCount(3));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -435,7 +414,6 @@ TEST_F(SelectionFileOptionTest, HandlesSingleSelectionOptionFromFile)
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("reqsel").storeVector(&reqsel)
                                     .multiValue().required()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -464,7 +442,6 @@ TEST_F(SelectionFileOptionTest, HandlesTwoSeparateSelectionOptions)
                                 SelectionOption("sel1").storeVector(&sel1).multiValue()));
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel2").storeVector(&sel2).multiValue()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     std::string          value(TestFileManager::getInputFilePath("selfile.dat"));
@@ -501,7 +478,6 @@ TEST_F(SelectionFileOptionTest, HandlesTwoSelectionOptionsFromSingleFile)
                                 SelectionOption("sel1").storeVector(&sel1)));
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel2").storeVector(&sel2)));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     std::string          value(TestFileManager::getInputFilePath("selfile.dat"));
@@ -535,7 +511,6 @@ TEST_F(SelectionFileOptionTest, HandlesRequiredOptionFromFile)
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("optsel").storeVector(&optsel)
                                     .multiValue()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -569,7 +544,6 @@ TEST_F(SelectionFileOptionTest, HandlesRequiredOptionFromFileWithOtherOptionSet)
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel2").storeVector(&sel2)
                                     .multiValue().required()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -600,7 +574,6 @@ TEST_F(SelectionFileOptionTest, HandlesTwoRequiredOptionsFromSingleFile)
                                 SelectionOption("sel1").storeVector(&sel1).required()));
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel2").storeVector(&sel2).required()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     std::string          value(TestFileManager::getInputFilePath("selfile.dat"));
@@ -625,7 +598,6 @@ TEST_F(SelectionFileOptionTest, GivesErrorWithNoFile)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").storeVector(&sel).multiValue()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -644,7 +616,6 @@ TEST_F(SelectionFileOptionTest, GivesErrorWithNonExistentFile)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").storeVector(&sel).multiValue()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
@@ -667,7 +638,6 @@ TEST_F(SelectionFileOptionTest, GivesErrorWithMultipleFiles)
     using gmx::SelectionOption;
     ASSERT_NO_THROW_GMX(options_.addOption(
                                 SelectionOption("sel").storeVector(&sel).multiValue()));
-    setManager();
 
     gmx::OptionsAssigner assigner(&options_);
     EXPECT_NO_THROW_GMX(assigner.start());
index b7165f94bf776076cbdf8a6f32d214f61a0a905c..d80bc883aee8221a2d4c0bc9d4d3ea966b95d737 100644 (file)
@@ -108,20 +108,20 @@ TrajectoryAnalysisCommandLineRunner::Impl::parseOptions(
         SelectionCollection *selections,
         int *argc, char *argv[])
 {
-    Options options(NULL, NULL);
-    Options moduleOptions(module_->name(), module_->description());
-    Options commonOptions("common", "Common analysis control");
-    Options selectionOptions("selection", "Common selection control");
-    module_->initOptions(&moduleOptions, settings);
-    common->initOptions(&commonOptions);
-    selections->initOptions(&selectionOptions);
+    SelectionOptionManager seloptManager(selections);
+    Options                options(NULL, NULL);
+    Options                moduleOptions(module_->name(), module_->description());
+    Options                commonOptions("common", "Common analysis control");
+    Options                selectionOptions("selection", "Common selection control");
 
+    options.addManager(&seloptManager);
     options.addSubSection(&commonOptions);
     options.addSubSection(&selectionOptions);
     options.addSubSection(&moduleOptions);
 
-    SelectionOptionManager seloptManager(selections);
-    setManagerForSelectionOptions(&options, &seloptManager);
+    module_->initOptions(&moduleOptions, settings);
+    common->initOptions(&commonOptions);
+    selections->initOptions(&selectionOptions);
 
     {
         CommandLineParser  parser(&options);
@@ -254,21 +254,20 @@ TrajectoryAnalysisCommandLineRunner::writeHelp(const CommandLineHelpContext &con
     TrajectoryAnalysisSettings      settings;
     TrajectoryAnalysisRunnerCommon  common(&settings);
 
-    Options options(NULL, NULL);
-    Options moduleOptions(impl_->module_->name(), impl_->module_->description());
-    Options commonOptions("common", "Common analysis control");
-    Options selectionOptions("selection", "Common selection control");
-
-    impl_->module_->initOptions(&moduleOptions, &settings);
-    common.initOptions(&commonOptions);
-    selections.initOptions(&selectionOptions);
+    SelectionOptionManager          seloptManager(&selections);
+    Options                         options(NULL, NULL);
+    Options                         moduleOptions(impl_->module_->name(), impl_->module_->description());
+    Options                         commonOptions("common", "Common analysis control");
+    Options                         selectionOptions("selection", "Common selection control");
 
+    options.addManager(&seloptManager);
     options.addSubSection(&commonOptions);
     options.addSubSection(&selectionOptions);
     options.addSubSection(&moduleOptions);
 
-    SelectionOptionManager seloptManager(&selections);
-    setManagerForSelectionOptions(&options, &seloptManager);
+    impl_->module_->initOptions(&moduleOptions, &settings);
+    common.initOptions(&commonOptions);
+    selections.initOptions(&selectionOptions);
 
     CommandLineHelpWriter(options)
         .setShowDescriptions(true)