Reduced use of raw pointers.
authorTeemu Murtola <teemu.murtola@gmail.com>
Sat, 25 Feb 2012 05:40:38 +0000 (07:40 +0200)
committerTeemu Murtola <teemu.murtola@gmail.com>
Sat, 17 Mar 2012 14:31:39 +0000 (16:31 +0200)
- Replaced pointers with references in some member variables and return
  values where the value was never NULL.
- Refactored AnalysisDataHandle to be able to pass them around by value,
  all pointer management happens internally.
- Also improved TrajectoryAnalysisModuleData::dataHandle() to not rely
  on string values (error-prone) when searching for registered data
  objects.

Conflicts:

src/gromacs/analysisdata/analysisdata-impl.h
src/gromacs/analysisdata/analysisdata.cpp

Change-Id: I43caade060ad2d4cd991a6bb5541462d9ac96760

26 files changed:
share/template/template.cpp
src/gromacs/analysisdata/analysisdata-impl.h
src/gromacs/analysisdata/analysisdata.cpp
src/gromacs/analysisdata/analysisdata.h
src/gromacs/analysisdata/datastorage.cpp
src/gromacs/analysisdata/datastorage.h
src/gromacs/analysisdata/tests/analysisdata.cpp
src/gromacs/analysisdata/tests/datatest.cpp
src/gromacs/analysisdata/tests/datatest.h
src/gromacs/options/abstractoption.cpp
src/gromacs/options/abstractoptionstorage.h
src/gromacs/selection/selectioncollection.cpp
src/gromacs/selection/selectioncollection.h
src/gromacs/trajectoryanalysis/analysismodule-impl.h
src/gromacs/trajectoryanalysis/analysismodule.cpp
src/gromacs/trajectoryanalysis/analysismodule.h
src/gromacs/trajectoryanalysis/cmdlinerunner.cpp
src/gromacs/trajectoryanalysis/modules/angle.cpp
src/gromacs/trajectoryanalysis/modules/angle.h
src/gromacs/trajectoryanalysis/modules/distance.cpp
src/gromacs/trajectoryanalysis/modules/distance.h
src/gromacs/trajectoryanalysis/modules/select.cpp
src/gromacs/trajectoryanalysis/modules/select.h
src/gromacs/trajectoryanalysis/runnercommon.cpp
src/gromacs/trajectoryanalysis/runnercommon.h
src/gromacs/trajectoryanalysis/tests/test_selection.cpp

index 1daa79c44c785939065569301b8f8d33cb78e970..0efd239cb2ea51b584da1970244677476b2afd3f 100644 (file)
@@ -43,7 +43,7 @@ class AnalysisTemplate : public TrajectoryAnalysisModule
     public:
         AnalysisTemplate();
 
-        virtual Options *initOptions(TrajectoryAnalysisSettings *settings);
+        virtual Options &initOptions(TrajectoryAnalysisSettings *settings);
         virtual void initAnalysis(const TrajectoryAnalysisSettings &settings,
                                   const TopologyInformation &top);
 
@@ -110,7 +110,7 @@ AnalysisTemplate::AnalysisTemplate()
 }
 
 
-Options *
+Options &
 AnalysisTemplate::initOptions(TrajectoryAnalysisSettings *settings)
 {
     static const char *const desc[] = {
@@ -149,7 +149,7 @@ AnalysisTemplate::initOptions(TrajectoryAnalysisSettings *settings)
 
     settings->setFlag(TrajectoryAnalysisSettings::efRequireTop);
 
-    return &_options;
+    return _options;
 }
 
 
@@ -188,11 +188,11 @@ void
 AnalysisTemplate::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
                                TrajectoryAnalysisModuleData *pdata)
 {
-    AnalysisDataHandle *dh = pdata->dataHandle("avedist");
+    AnalysisDataHandle  dh = pdata->dataHandle(_data);
     NeighborhoodSearch &nb = static_cast<ModuleData *>(pdata)->_nb;
 
     nb.init(pbc, _refsel->positions());
-    dh->startFrame(frnr, fr.time);
+    dh.startFrame(frnr, fr.time);
     for (size_t g = 0; g < _sel.size(); ++g)
     {
         Selection *sel = pdata->parallelSelection(_sel[g]);
@@ -204,9 +204,9 @@ AnalysisTemplate::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
             frave += nb.minimumDistance(p.x());
         }
         frave /= nr;
-        dh->setPoint(g, frave);
+        dh.setPoint(g, frave);
     }
-    dh->finishFrame();
+    dh.finishFrame();
 }
 
 
index 2b694e0118343feba94b04c0f0f0208797793ed1..ec4283a7263c3deb453c0b35154c674e903dd687 100644 (file)
 namespace gmx
 {
 
+namespace internal
+{
+/*! \internal \brief
+ * Private implementation class for AnalysisDataHandle.
+ *
+ * \ingroup module_analysisdata
+ */
+class AnalysisDataHandleImpl
+{
+    public:
+        //! Creates a handle associated with the given data object.
+        explicit AnalysisDataHandleImpl(AnalysisData *data);
+
+        //! The data object that this handle belongs to.
+        AnalysisData             &data_;
+        //! Current storage frame object, or NULL if no current frame.
+        AnalysisDataStorageFrame *currentFrame_;
+};
+} // namespace internal
+
 /*! \internal \brief
  * Private implementation class for AnalysisData.
  *
@@ -57,35 +77,21 @@ class AnalysisData::Impl
 {
     public:
         //! Shorthand for a smart pointer to a data handle.
-        typedef gmx_unique_ptr<AnalysisDataHandle>::type HandlePointer;
+        typedef gmx_unique_ptr<internal::AnalysisDataHandleImpl>::type
+                HandlePointer;
         //! Shorthand for a list of data handles.
         typedef std::vector<HandlePointer> HandleList;
 
         Impl();
         ~Impl();
 
+        //! Storage implementation
         AnalysisDataStorage     storage_;
-        //! List of handles for this data object.
+        //! \brief List of handles for this data object.
+        //! Caution: AnalysisDataHandle stores a bare pointer to a HandlePointer
         HandleList              handles_;
 };
 
-/*! \internal \brief
- * Private implementation class for AnalysisDataHandle.
- *
- * \ingroup module_analysisdata
- */
-class AnalysisDataHandle::Impl
-{
-    public:
-        //! Creates a handle associated with the given data object.
-        explicit Impl(AnalysisData *data);
-
-        //! The data object that this handle belongs to.
-        AnalysisData             &data_;
-        //! Current storage frame object, or NULL if no current frame.
-        AnalysisDataStorageFrame *currentFrame_;
-};
-
 } // namespace gmx
 
 #endif
index d1de779ab56451a2da625ade416aabc0a826681d..defec2f5aa55edd6a94a2c68a4f21e32f29ead58 100644 (file)
@@ -89,7 +89,7 @@ AnalysisData::setColumns(int ncol, bool multipoint)
 }
 
 
-AnalysisDataHandle *
+AnalysisDataHandle
 AnalysisData::startData(const AnalysisDataParallelOptions &opt)
 {
     GMX_RELEASE_ASSERT(impl_->handles_.size() < static_cast<unsigned>(opt.parallelizationFactor()),
@@ -105,20 +105,20 @@ AnalysisData::startData(const AnalysisDataParallelOptions &opt)
         GMX_THROW(NotImplementedError("Parallelism not supported for multipoint data"));
     }
 
-    Impl::HandlePointer handle(new AnalysisDataHandle(this));
+    Impl::HandlePointer handle(new internal::AnalysisDataHandleImpl(this));
     impl_->handles_.push_back(move(handle));
-    return impl_->handles_.back().get();
+    return AnalysisDataHandle(impl_->handles_.back().get());
 }
 
 
 void
-AnalysisData::finishData(AnalysisDataHandle *handle)
+AnalysisData::finishData(AnalysisDataHandle handle)
 {
     Impl::HandleList::iterator i;
 
     for (i = impl_->handles_.begin(); i != impl_->handles_.end(); ++i)
     {
-        if (i->get() == handle)
+        if (i->get() == handle.impl_)
         {
             break;
         }
@@ -150,10 +150,10 @@ AnalysisData::requestStorageInternal(int nframes)
 
 
 /********************************************************************
- * AnalysisDataHandle::Impl
+ * AnalysisDataHandleImpl
  */
 
-AnalysisDataHandle::Impl::Impl(AnalysisData *data)
+internal::AnalysisDataHandleImpl::AnalysisDataHandleImpl(AnalysisData *data)
     : data_(*data), currentFrame_(NULL)
 {
 }
@@ -163,13 +163,14 @@ AnalysisDataHandle::Impl::Impl(AnalysisData *data)
  * AnalysisDataHandle
  */
 
-AnalysisDataHandle::AnalysisDataHandle(AnalysisData *data)
-    : impl_(new Impl(data))
+AnalysisDataHandle::AnalysisDataHandle()
+    : impl_(NULL)
 {
 }
 
 
-AnalysisDataHandle::~AnalysisDataHandle()
+AnalysisDataHandle::AnalysisDataHandle(internal::AnalysisDataHandleImpl *impl)
+    : impl_(impl)
 {
 }
 
@@ -177,6 +178,7 @@ AnalysisDataHandle::~AnalysisDataHandle()
 void
 AnalysisDataHandle::startFrame(int index, real x, real dx)
 {
+    GMX_RELEASE_ASSERT(impl_ != NULL, "Invalid data handle used");
     GMX_RELEASE_ASSERT(impl_->currentFrame_ == NULL,
                        "startFrame() called twice without calling finishFrame()");
     impl_->currentFrame_ =
@@ -187,6 +189,7 @@ AnalysisDataHandle::startFrame(int index, real x, real dx)
 void
 AnalysisDataHandle::setPoint(int col, real y, real dy, bool present)
 {
+    GMX_RELEASE_ASSERT(impl_ != NULL, "Invalid data handle used");
     GMX_RELEASE_ASSERT(impl_->currentFrame_ != NULL,
                        "setPoint() called without calling startFrame()");
     impl_->currentFrame_->setValue(col, y, dy, present);
@@ -196,6 +199,7 @@ AnalysisDataHandle::setPoint(int col, real y, real dy, bool present)
 void
 AnalysisDataHandle::setPoints(int firstcol, int n, const real *y)
 {
+    GMX_RELEASE_ASSERT(impl_ != NULL, "Invalid data handle used");
     GMX_RELEASE_ASSERT(impl_->currentFrame_ != NULL,
                        "setPoints() called without calling startFrame()");
     for (int i = 0; i < n; ++i)
@@ -208,6 +212,7 @@ AnalysisDataHandle::setPoints(int firstcol, int n, const real *y)
 void
 AnalysisDataHandle::finishPointSet()
 {
+    GMX_RELEASE_ASSERT(impl_ != NULL, "Invalid data handle used");
     GMX_RELEASE_ASSERT(impl_->data_.isMultipoint(),
                        "finishPointSet() called for non-multipoint data");
     GMX_RELEASE_ASSERT(impl_->currentFrame_ != NULL,
@@ -219,6 +224,7 @@ AnalysisDataHandle::finishPointSet()
 void
 AnalysisDataHandle::finishFrame()
 {
+    GMX_RELEASE_ASSERT(impl_ != NULL, "Invalid data handle used");
     GMX_RELEASE_ASSERT(impl_->currentFrame_ != NULL,
                        "finishFrame() called without calling startFrame()");
     int index = impl_->currentFrame_->frameIndex();
@@ -230,8 +236,10 @@ AnalysisDataHandle::finishFrame()
 void
 AnalysisDataHandle::finishData()
 {
-    // Calls delete this
-    impl_->data_.finishData(this);
+    GMX_RELEASE_ASSERT(impl_ != NULL, "Invalid data handle used");
+    // Deletes the implementation pointer.
+    impl_->data_.finishData(*this);
+    impl_ = NULL;
 }
 
 } // namespace gmx
index 2dc2a9f0b93cf0b4d3c45fe0babf035c64cc3770..1acd88bdd3375dc972c964930d0bae17e2c58d68 100644 (file)
@@ -83,16 +83,22 @@ class AnalysisData : public AbstractAnalysisData
          * \param[in]  opt     Options for setting how this handle will be
          *     used.
          * \returns The created handle.
+         *
+         * The caller should retain the returned handle (or a copy of it), and
+         * pass it to finishData() after successfully adding all data.
+         * The caller should discard the returned handle if an error occurs;
+         * memory allocated for the handle will be freed when the AnalysisData
+         * object is destroyed.
          */
-        AnalysisDataHandle *startData(const AnalysisDataParallelOptions &opt);
+        AnalysisDataHandle startData(const AnalysisDataParallelOptions &opt);
         /*! \brief
          * Destroy a handle after all data has been added.
          *
          * \param[in]  handle  Handle to destroy.
          *
-         * The pointer \p handle is invalid after the call.
+         * The \p handle (and any copies) are invalid after the call.
          */
-        void finishData(AnalysisDataHandle *handle);
+        void finishData(AnalysisDataHandle handle);
 
     private:
         virtual AnalysisDataFrameRef tryGetDataFrameInternal(int index) const;
@@ -105,10 +111,17 @@ class AnalysisData : public AbstractAnalysisData
         friend class AnalysisDataHandle;
 };
 
+namespace internal
+{
+class AnalysisDataHandleImpl;
+} // namespace internal
 
 /*! \brief
  * Handle for inserting data into AnalysisData.
  *
+ * This class works like a pointer type: copying and assignment is lightweight,
+ * and all copies work interchangeably, accessing the same internal handle.
+ *
  * Several handles can exist concurrently.
  *
  * \inpublicapi
@@ -118,12 +131,15 @@ class AnalysisDataHandle
 {
     public:
         /*! \brief
-         * Frees memory allocated for the internal implementation.
+         * Constructs an invalid data handle.
          *
-         * Should not be called directly, but through finishData() or
-         * AnalysisData::finishData().
+         * This constructor is provided for convenience in cases where it is
+         * easiest to declare an AnalysisDataHandle without immediately
+         * assigning a value to it.  Any attempt to call methods without first
+         * assigning a value from AnalysisData::startData() to the handle
+         * causes an assert.
          */
-        ~AnalysisDataHandle();
+        AnalysisDataHandle();
 
         //! Start data for a new frame.
         void startFrame(int index, real x, real dx = 0.0);
@@ -147,11 +163,16 @@ class AnalysisDataHandle
          * The constructor is private because data handles should only be
          * constructed through AnalysisData::startData().
          */
-        explicit AnalysisDataHandle(AnalysisData *data);
-
-        class Impl;
+        explicit AnalysisDataHandle(internal::AnalysisDataHandleImpl *impl);
 
-        PrivateImplPointer<Impl> impl_;
+        /*! \brief
+         * Pointer to the internal implementation class.
+         *
+         * The memory for this object is managed by the AnalysisData object,
+         * and AnalysisDataHandle simply provides a public interface for
+         * accessing the implementation.
+         */
+        internal::AnalysisDataHandleImpl *impl_;
 
         friend class AnalysisData;
 };
index f5f37b26fb0fd57c4366e871cd04328291ea9f69..8d175bfec2c95f17fa4879b86bce7d5191ab8c78 100644 (file)
@@ -74,7 +74,7 @@ AnalysisDataParallelOptions::AnalysisDataParallelOptions(int parallelizationFact
 
 AnalysisDataStorageFrame::AnalysisDataStorageFrame(AnalysisDataStorage *storage,
                                                    int columnCount, int index)
-    : storage_(storage), header_(index, 0.0, 0.0), values_(columnCount)
+    : storage_(*storage), header_(index, 0.0, 0.0), values_(columnCount)
 {
 }
 
@@ -117,7 +117,7 @@ AnalysisDataStorageFrame::clearValues()
 void
 AnalysisDataStorageFrame::finishPointSet()
 {
-    storage_->impl_->notifyPointSet(currentPoints());
+    storage_.impl_->notifyPointSet(currentPoints());
     clearValues();
 }
 
index 48445f359fd09bfc64f4653f001dfafae966dfad..6893e4c6d619917f6e51a84fb3b0c69a712ff224 100644 (file)
@@ -195,7 +195,7 @@ class AnalysisDataStorageFrame
         void clearValues();
 
         //! Storage object that contains this frame.
-        AnalysisDataStorage    *storage_;
+        AnalysisDataStorage    &storage_;
         //! Header for the frame.
         AnalysisDataFrameHeader header_;
         //! Values for the frame.
index 0505fb6899135cc8a6a77b3c23e2da65fceb26b6..b4b1b4e339a6f31f2093f5a147622542691d50b8 100644 (file)
@@ -174,16 +174,16 @@ TEST_F(AnalysisDataTest, CallsModuleCorrectlyWithOutOfOrderFrames)
 
     ASSERT_NO_THROW(addStaticCheckerModule(input, &data));
     ASSERT_NO_THROW(addStaticColumnCheckerModule(input, 1, 2, &data));
-    gmx::AnalysisDataHandle *handle1 = NULL;
-    gmx::AnalysisDataHandle *handle2 = NULL;
+    gmx::AnalysisDataHandle handle1;
+    gmx::AnalysisDataHandle handle2;
     gmx::AnalysisDataParallelOptions options(2);
     ASSERT_NO_THROW(handle1 = data.startData(options));
     ASSERT_NO_THROW(handle2 = data.startData(options));
     ASSERT_NO_THROW(presentDataFrame(input, 1, handle1));
     ASSERT_NO_THROW(presentDataFrame(input, 0, handle2));
     ASSERT_NO_THROW(presentDataFrame(input, 2, handle1));
-    ASSERT_NO_THROW(handle1->finishData());
-    ASSERT_NO_THROW(handle2->finishData());
+    ASSERT_NO_THROW(handle1.finishData());
+    ASSERT_NO_THROW(handle2.finishData());
 }
 
 /*
index 3c5072db4bd9b68c4ab5c7ead3d77bfe3a7c3aaa..02b454db59ed9af50a2de765f3143ec8e7608d80 100644 (file)
@@ -151,34 +151,34 @@ void AnalysisDataTestFixture::presentAllData(const AnalysisDataTestInput &input,
                                              AnalysisData *data)
 {
     gmx::AnalysisDataParallelOptions options;
-    gmx::AnalysisDataHandle *handle = data->startData(options);
+    gmx::AnalysisDataHandle handle = data->startData(options);
     for (int row = 0; row < input.frameCount(); ++row)
     {
         presentDataFrame(input, row, handle);
         EXPECT_EQ(row + 1, data->frameCount());
     }
-    handle->finishData();
+    handle.finishData();
 }
 
 
 void AnalysisDataTestFixture::presentDataFrame(const AnalysisDataTestInput &input,
-                                               int row, AnalysisDataHandle *handle)
+                                               int row, AnalysisDataHandle handle)
 {
     const AnalysisDataTestInputFrame &frame = input.frame(row);
-    handle->startFrame(row, frame.x(), frame.dx());
+    handle.startFrame(row, frame.x(), frame.dx());
     for (int i = 0; i < frame.pointSetCount(); ++i)
     {
         const AnalysisDataTestInputPointSet &points = frame.points(i);
         for (int j = 0; j < points.size(); ++j)
         {
-            handle->setPoint(j, points.y(j), points.dy(j), points.present(j));
+            handle.setPoint(j, points.y(j), points.dy(j), points.present(j));
         }
         if (input.isMultipoint())
         {
-            handle->finishPointSet();
+            handle.finishPointSet();
         }
     }
-    handle->finishFrame();
+    handle.finishFrame();
 }
 
 
index a76c1fb6f36e0dcde0729852a69d4d997f58ef0c..47b7f6f67a94bc25c17eff6ec40e6f2cc80ed2a0 100644 (file)
@@ -222,7 +222,7 @@ class AnalysisDataTestFixture : public ::testing::Test
          * Adds a single frame from AnalysisDataTestInput into an AnalysisData.
          */
         static void presentDataFrame(const AnalysisDataTestInput &input, int row,
-                                     AnalysisDataHandle *handle);
+                                     AnalysisDataHandle handle);
         /*! \brief
          * Initializes an array data object from AnalysisDataTestInput.
          *
index 2c30e741cb3aadb654923881a6ff5ca30b1ac7c5..af18299d55f801fc9940d6c12e1529f315ba7c24 100644 (file)
@@ -58,7 +58,7 @@ AbstractOptionStorage::AbstractOptionStorage(const AbstractOption &settings,
       _minValueCount(settings._minValueCount),
       _maxValueCount(settings._maxValueCount),
       _inSet(false),
-      _options(options)
+      _options(*options)
 {
     // If the maximum number of values is not known, storage to
     // caller-allocated memory is unsafe.
index 482c2c23f7fdb952b57da42849dd30b1e6db9736..118aa11b31467073aaa589eeb9be1bcacaf8124d 100644 (file)
@@ -232,9 +232,9 @@ class AbstractOptionStorage
         void setMaxValueCount(int count);
 
         //! Returns the Options object that houses the option.
-        Options &hostOptions() { return *_options; }
+        Options &hostOptions() { return _options; }
         //! \copydoc hostOptions()
-        const Options &hostOptions() const { return *_options; }
+        const Options &hostOptions() const { return _options; }
 
         /*! \brief
          * Removes all values from temporary storage for a set.
@@ -298,7 +298,7 @@ class AbstractOptionStorage
         //! Whether we are currently assigning values to a set.
         bool                    _inSet;
         //! Parent Options object.
-        Options                *_options;
+        Options                &_options;
 
         GMX_DISALLOW_COPY_AND_ASSIGN(AbstractOptionStorage);
 };
index fe4268112a27af246f47cbb370ecc05abe632e89..a73750ff76c37a782625258096c1b5a96319607f 100644 (file)
@@ -261,7 +261,7 @@ SelectionCollection::~SelectionCollection()
 }
 
 
-Options *
+Options &
 SelectionCollection::initOptions()
 {
     static const char * const debug_levels[]
@@ -298,7 +298,7 @@ SelectionCollection::initOptions()
                           .description("Print out selection trees for debugging"));
     sfree(postypes);
 
-    return &_impl->_options;
+    return _impl->_options;
 }
 
 
index d1f0d352e73b2c7692f95629977a2900de81fa6c..4ff241499ca9fff03304ecb91bfe020e8ded2914 100644 (file)
@@ -102,10 +102,8 @@ class SelectionCollection
 
         /*! \brief
          * Initializes options for setting global properties on the collection.
-         *
-         * The return value should not be deleted by the caller.
          */
-        Options *initOptions();
+        Options &initOptions();
 
         /*! \brief
          * Sets the default reference position handling for a selection
index 3d5280868cb894f7b0d9a886538895b35c0f921b..00857d0440ccd0cb54216b5c567fa5447b463b80 100644 (file)
@@ -55,7 +55,8 @@ class AnalysisDataHandle;
 class TrajectoryAnalysisModuleData::Impl
 {
     public:
-        typedef std::map<std::string, AnalysisDataHandle *> HandleContainer;
+        typedef std::map<const AnalysisData *, AnalysisDataHandle>
+                HandleContainer;
 
         Impl(TrajectoryAnalysisModule *module,
              const AnalysisDataParallelOptions &opt,
index a21aa6fd65ffe185969ea171bbd1091a7f512587..3c2f3532ba11201c1b1f3a51a444423f4b0a5e5a 100644 (file)
  */
 #include "gromacs/trajectoryanalysis/analysismodule.h"
 
+#include <utility>
+
 #include "gromacs/analysisdata/analysisdata.h"
+#include "gromacs/fatalerror/exceptions.h"
 #include "gromacs/fatalerror/gmxassert.h"
 
 #include "analysismodule-impl.h"
@@ -59,7 +62,7 @@ TrajectoryAnalysisModuleData::Impl::Impl(
     for (i = module->_impl->_analysisDatasets.begin();
          i != module->_impl->_analysisDatasets.end(); ++i)
     {
-        _handles[i->first] = i->second->startData(opt);
+        _handles.insert(std::make_pair(i->second, i->second->startData(opt)));
     }
 }
 
@@ -71,10 +74,10 @@ TrajectoryAnalysisModuleData::Impl::~Impl()
 void TrajectoryAnalysisModuleData::Impl::finishHandles()
 {
     // FIXME: Call finishData() for all handles even if one throws
-    HandleContainer::const_iterator i;
+    HandleContainer::iterator i;
     for (i = _handles.begin(); i != _handles.end(); ++i)
     {
-        i->second->finishData();
+        i->second.finishData();
     }
     _handles.clear();
 }
@@ -104,12 +107,13 @@ void TrajectoryAnalysisModuleData::finishDataHandles()
 }
 
 
-AnalysisDataHandle *TrajectoryAnalysisModuleData::dataHandle(const char *name)
+AnalysisDataHandle
+TrajectoryAnalysisModuleData::dataHandle(const AnalysisData &data)
 {
-    Impl::HandleContainer::const_iterator i = _impl->_handles.find(name);
+    Impl::HandleContainer::const_iterator i = _impl->_handles.find(&data);
     GMX_RELEASE_ASSERT(i != _impl->_handles.end(),
                        "Data handle requested on unknown dataset");
-    return (i != _impl->_handles.end()) ? (*i).second : NULL;
+    return i->second;
 }
 
 
@@ -203,35 +207,35 @@ const std::vector<std::string> &TrajectoryAnalysisModule::datasetNames() const
 }
 
 
-AbstractAnalysisData *TrajectoryAnalysisModule::datasetFromIndex(int index) const
+AbstractAnalysisData &TrajectoryAnalysisModule::datasetFromIndex(int index) const
 {
     if (index < 0 || index >= datasetCount())
     {
-        return NULL;
+        GMX_THROW(APIError("Out of range data set index"));
     }
     Impl::DatasetContainer::const_iterator item
         = _impl->_datasets.find(_impl->_datasetNames[index]);
     GMX_RELEASE_ASSERT(item != _impl->_datasets.end(),
                        "Inconsistent data set names");
-    return item->second;
+    return *item->second;
 }
 
 
-AbstractAnalysisData *TrajectoryAnalysisModule::datasetFromName(const char *name) const
+AbstractAnalysisData &TrajectoryAnalysisModule::datasetFromName(const char *name) const
 {
     Impl::DatasetContainer::const_iterator item = _impl->_datasets.find(name);
     if (item == _impl->_datasets.end())
     {
-        return NULL;
+        GMX_THROW(APIError("Unknown data set name"));
     }
-    return item->second;
+    return *item->second;
 }
 
 
 void TrajectoryAnalysisModule::registerBasicDataset(AbstractAnalysisData *data,
                                                     const char *name)
 {
-    GMX_RELEASE_ASSERT(datasetFromName(name) == NULL,
+    GMX_RELEASE_ASSERT(_impl->_datasets.find(name) == _impl->_datasets.end(),
                        "Duplicate data set name registered");
     _impl->_datasets[name] = data;
     _impl->_datasetNames.push_back(name);
index 73382d35a52c5a8643ca38ab5b8d73ce8139305d..c34bff24893496efffb92a74334de3907b340aef 100644 (file)
@@ -89,12 +89,12 @@ class TrajectoryAnalysisModuleData
         virtual void finish() = 0;
 
         /*! \brief
-         * Returns a data handle for a dataset with a given name.
+         * Returns a data handle for a given dataset.
          *
-         * Allowed names are those that have been registered with
+         * Allowed data sets are those that have been registered with
          * TrajectoryAnalysisModule::registerAnalysisDataset().
          */
-        AnalysisDataHandle *dataHandle(const char *name);
+        AnalysisDataHandle dataHandle(const AnalysisData &data);
         /*! \brief
          * Returns a selection that corresponds to the given selection.
          */
@@ -169,7 +169,7 @@ class TrajectoryAnalysisModule
          * If settings depend on the option values provided by the user, see
          * initOptionsDone().
          */
-        virtual Options *initOptions(TrajectoryAnalysisSettings *settings) = 0;
+        virtual Options &initOptions(TrajectoryAnalysisSettings *settings) = 0;
         /*! \brief
          * Called after all option values have been set.
          *
@@ -311,7 +311,7 @@ class TrajectoryAnalysisModule
          * provide any means to alter the data, so the module does not need to
          * care about external modifications.
          */
-        AbstractAnalysisData *datasetFromIndex(int index) const;
+        AbstractAnalysisData &datasetFromIndex(int index) const;
         /*! \brief
          * Returns a pointer to the data set with name \p name
          *
@@ -324,7 +324,7 @@ class TrajectoryAnalysisModule
          * provide any means to alter the data, so the module does not need to
          * care about external modifications.
          */
-        AbstractAnalysisData *datasetFromName(const char *name) const;
+        AbstractAnalysisData &datasetFromName(const char *name) const;
 
     protected:
         //! Initializes the dataset registration mechanism.
index e11d2d1b16f4727e99be7bf6ec8a01c48ad48af8..2b86bcba068128778b09c1114ea8aa711bc5e507 100644 (file)
@@ -121,14 +121,13 @@ TrajectoryAnalysisCommandLineRunner::Impl::parseOptions(
         Options *options,
         int *argc, char *argv[])
 {
-    Options *moduleOptions = _module->initOptions(settings);
-    GMX_RELEASE_ASSERT(moduleOptions != NULL, "Module returned NULL options");
-    Options *commonOptions = common->initOptions();
-    Options *selectionOptions = selections->initOptions();
-
-    options->addSubSection(commonOptions);
-    options->addSubSection(selectionOptions);
-    options->addSubSection(moduleOptions);
+    Options &moduleOptions = _module->initOptions(settings);
+    Options &commonOptions = common->initOptions();
+    Options &selectionOptions = selections->initOptions();
+
+    options->addSubSection(&commonOptions);
+    options->addSubSection(&selectionOptions);
+    options->addSubSection(&moduleOptions);
 
     setSelectionCollectionForOptions(options, selections);
 
index 98e480789782b1ef734b30521ed3d5be87b31420..09c8c26268f67449451680bca00f5fe95ce16c70 100644 (file)
@@ -76,7 +76,7 @@ Angle::~Angle()
 }
 
 
-Options *
+Options &
 Angle::initOptions(TrajectoryAnalysisSettings *settings)
 {
     static const char *const desc[] = {
@@ -154,7 +154,7 @@ Angle::initOptions(TrajectoryAnalysisSettings *settings)
         .dynamicOnlyWhole().storeVector(&_sel2).getAdjuster(&_sel2info)
         .description("Second analysis/vector selection"));
 
-    return &_options;
+    return _options;
 }
 
 
@@ -431,7 +431,7 @@ void
 Angle::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
                     TrajectoryAnalysisModuleData *pdata)
 {
-    AnalysisDataHandle *dh = pdata->dataHandle("angle");
+    AnalysisDataHandle       dh = pdata->dataHandle(_data);
     std::vector<Selection *> sel1 = pdata->parallelSelections(_sel1);
     std::vector<Selection *> sel2 = pdata->parallelSelections(_sel2);
 
@@ -451,7 +451,7 @@ Angle::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
             break;
     }
 
-    dh->startFrame(frnr, fr.time);
+    dh.startFrame(frnr, fr.time);
 
     int incr1 = _bSplit1 ? 1 : _natoms1;
     int incr2 = _bSplit2 ? 1 : _natoms2;
@@ -563,7 +563,7 @@ Angle::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
             }
             if (_bAll)
             {
-                dh->setPoint(n + 1, angle);
+                dh.setPoint(n + 1, angle);
             }
             ave += angle;
             ++n;
@@ -572,9 +572,9 @@ Angle::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
         {
             ave /= n;
         }
-        dh->setPoint(g, ave);
+        dh.setPoint(g, ave);
     }
-    dh->finishFrame();
+    dh.finishFrame();
 }
 
 
index 6d2fcf8380dc125a7ac7cba72342f92d9c76c45d..30835c0ca2559e9d300a10952d2549209cadfe28 100644 (file)
@@ -62,7 +62,7 @@ class Angle : public TrajectoryAnalysisModule
 
         static TrajectoryAnalysisModule *create();
 
-        virtual Options *initOptions(TrajectoryAnalysisSettings *settings);
+        virtual Options &initOptions(TrajectoryAnalysisSettings *settings);
         virtual void initOptionsDone(TrajectoryAnalysisSettings *settings);
         virtual void initAnalysis(const TrajectoryAnalysisSettings &settings,
                                   const TopologyInformation &top);
index 7995fccf4fe0916816b35941a2a5d18a16e3dc9f..7c1cc94ca0a4796ae0d4fe89fb2989df833e38dc 100644 (file)
@@ -72,7 +72,7 @@ Distance::~Distance()
 }
 
 
-Options *
+Options &
 Distance::initOptions(TrajectoryAnalysisSettings *settings)
 {
     static const char *const desc[] = {
@@ -88,7 +88,7 @@ Distance::initOptions(TrajectoryAnalysisSettings *settings)
                            .store(&_fnDist).defaultValue("dist"));
     _options.addOption(SelectionOption("select").required().valueCount(2)
                            .store(_sel));
-    return &_options;
+    return _options;
 }
 
 
@@ -123,7 +123,7 @@ void
 Distance::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
                        TrajectoryAnalysisModuleData *pdata)
 {
-    AnalysisDataHandle *dh = pdata->dataHandle("distance");
+    AnalysisDataHandle  dh = pdata->dataHandle(_data);
     Selection          *sel1 = pdata->parallelSelection(_sel[0]);
     Selection          *sel2 = pdata->parallelSelection(_sel[1]);
     rvec                dx;
@@ -140,10 +140,10 @@ Distance::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
         rvec_sub(p1.x(), p2.x(), dx);
     }
     r = norm(dx);
-    dh->startFrame(frnr, fr.time);
-    dh->setPoint(0, r);
-    dh->setPoints(1, 3, dx);
-    dh->finishFrame();
+    dh.startFrame(frnr, fr.time);
+    dh.setPoint(0, r);
+    dh.setPoints(1, 3, dx);
+    dh.finishFrame();
 }
 
 
index f038d659379f99c273728ea3f2bb67146ef16cc1..35be8ed6cf7b3ecfcc34bddcf179754b358c27ad 100644 (file)
@@ -63,7 +63,7 @@ class Distance : public TrajectoryAnalysisModule
 
         static TrajectoryAnalysisModule *create();
 
-        virtual Options *initOptions(TrajectoryAnalysisSettings *settings);
+        virtual Options &initOptions(TrajectoryAnalysisSettings *settings);
         virtual void initAnalysis(const TrajectoryAnalysisSettings &settings,
                                   const TopologyInformation &top);
 
index 687352bc91b864891ecac8220ec6f751ce4747ce..5ba48861abdaa64ad865f16ccea13ee8b5550ff9 100644 (file)
@@ -277,7 +277,7 @@ Select::~Select()
 }
 
 
-Options *
+Options &
 Select::initOptions(TrajectoryAnalysisSettings *settings)
 {
     static const char *const desc[] = {
@@ -354,7 +354,7 @@ Select::initOptions(TrajectoryAnalysisSettings *settings)
         .enumValue(cResNumberEnum).defaultEnumIndex(0)
         .description("Residue number output type"));
 
-    return &_options;
+    return _options;
 }
 
 
@@ -485,68 +485,56 @@ Select::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
                      TrajectoryAnalysisModuleData *pdata)
 {
     ModuleData *d = static_cast<ModuleData *>(pdata);
-    AnalysisDataHandle *sdh = pdata->dataHandle("size");
-    AnalysisDataHandle *cdh = pdata->dataHandle("cfrac");
-    AnalysisDataHandle *idh = pdata->dataHandle("index");
-    AnalysisDataHandle *mdh = pdata->dataHandle("mask");
+    AnalysisDataHandle sdh = pdata->dataHandle(_sdata);
+    AnalysisDataHandle cdh = pdata->dataHandle(_cdata);
+    AnalysisDataHandle idh = pdata->dataHandle(_idata);
+    AnalysisDataHandle mdh = pdata->dataHandle(_mdata);
     std::vector<Selection *> sel(pdata->parallelSelections(_sel));
 
-    if (sdh != NULL)
+    sdh.startFrame(frnr, fr.time);
+    for (size_t g = 0; g < sel.size(); ++g)
     {
-        sdh->startFrame(frnr, fr.time);
-        for (size_t g = 0; g < sel.size(); ++g)
-        {
-            real normfac = _bFracNorm ? 1.0 / sel[g]->coveredFraction() : 1.0;
-            normfac /= _totsize[g];
-            sdh->setPoint(g, sel[g]->posCount() * normfac);
-        }
-        sdh->finishFrame();
+        real normfac = _bFracNorm ? 1.0 / sel[g]->coveredFraction() : 1.0;
+        normfac /= _totsize[g];
+        sdh.setPoint(g, sel[g]->posCount() * normfac);
     }
+    sdh.finishFrame();
 
-    if (cdh != NULL)
+    cdh.startFrame(frnr, fr.time);
+    for (size_t g = 0; g < sel.size(); ++g)
     {
-        cdh->startFrame(frnr, fr.time);
-        for (size_t g = 0; g < sel.size(); ++g)
-        {
-            cdh->setPoint(g, sel[g]->coveredFraction());
-        }
-        cdh->finishFrame();
+        cdh.setPoint(g, sel[g]->coveredFraction());
     }
+    cdh.finishFrame();
 
-    if (idh != NULL)
+    idh.startFrame(frnr, fr.time);
+    for (size_t g = 0; g < sel.size(); ++g)
     {
-        idh->startFrame(frnr, fr.time);
-        for (size_t g = 0; g < sel.size(); ++g)
+        idh.setPoint(0, sel[g]->posCount());
+        idh.finishPointSet();
+        for (int i = 0; i < sel[g]->posCount(); ++i)
         {
-            idh->setPoint(0, sel[g]->posCount());
-            idh->finishPointSet();
-            for (int i = 0; i < sel[g]->posCount(); ++i)
+            SelectionPosition p = sel[g]->position(i);
+            if (sel[g]->type() == INDEX_RES && !_bResInd)
             {
-                SelectionPosition p = sel[g]->position(i);
-                if (sel[g]->type() == INDEX_RES && !_bResInd)
-                {
-                    idh->setPoint(1, _top->atoms.resinfo[p.mappedId()].nr);
-                }
-                else
-                {
-                    idh->setPoint(1, p.mappedId() + 1);
-                }
-                idh->finishPointSet();
+                idh.setPoint(1, _top->atoms.resinfo[p.mappedId()].nr);
             }
+            else
+            {
+                idh.setPoint(1, p.mappedId() + 1);
+            }
+            idh.finishPointSet();
         }
-        idh->finishFrame();
     }
+    idh.finishFrame();
 
-    if (mdh != NULL)
+    gmx_ana_indexmap_update(d->_mmap, sel[0]->indexGroup(), true);
+    mdh.startFrame(frnr, fr.time);
+    for (int b = 0; b < d->_mmap->nr; ++b)
     {
-        gmx_ana_indexmap_update(d->_mmap, sel[0]->indexGroup(), true);
-        mdh->startFrame(frnr, fr.time);
-        for (int b = 0; b < d->_mmap->nr; ++b)
-        {
-            mdh->setPoint(b, d->_mmap->refid[b] == -1 ? 0 : 1);
-        }
-        mdh->finishFrame();
+        mdh.setPoint(b, d->_mmap->refid[b] == -1 ? 0 : 1);
     }
+    mdh.finishFrame();
 }
 
 
index b1c98a93663149d01d3925d16a16f1d8a223d81e..2a57c32b24641c6ed7a834eee6836e1e83fae970 100644 (file)
@@ -62,7 +62,7 @@ class Select : public TrajectoryAnalysisModule
 
         static TrajectoryAnalysisModule *create();
 
-        virtual Options *initOptions(TrajectoryAnalysisSettings *settings);
+        virtual Options &initOptions(TrajectoryAnalysisSettings *settings);
         virtual void initAnalysis(const TrajectoryAnalysisSettings &settings,
                                   const TopologyInformation &top);
 
index 94f29a77a1b27d3a9d37277447f5eacd218dabd5..642e89258011fb88d53feccd7e5161e651993422 100644 (file)
@@ -161,7 +161,7 @@ TrajectoryAnalysisRunnerCommon::~TrajectoryAnalysisRunnerCommon()
 }
 
 
-Options *
+Options &
 TrajectoryAnalysisRunnerCommon::initOptions()
 {
     TrajectoryAnalysisSettings &settings = _impl->_settings;
@@ -217,7 +217,7 @@ TrajectoryAnalysisRunnerCommon::initOptions()
                               .description("Use periodic boundary conditions for distance calculation"));
     }
 
-    return &_impl->_options;
+    return _impl->_options;
 }
 
 
index d7cdc1abea9a12541247f807631340fa95b606a0..4a1cf53eada5e2b5d06a6b0df8e608211d32f585 100644 (file)
@@ -74,7 +74,7 @@ class TrajectoryAnalysisRunnerCommon
         explicit TrajectoryAnalysisRunnerCommon(TrajectoryAnalysisSettings *settings);
         ~TrajectoryAnalysisRunnerCommon();
 
-        Options *initOptions();
+        Options &initOptions();
         void scaleTimeOptions(Options *options);
         bool initOptionsDone();
         void initIndexGroups(SelectionCollection *selections);
index 3eb4679d3e1e0ce9ac64ab367da25b31e1c21727..018ba2ae7d981f48c2aba94753693c341fbd9ca8 100644 (file)
@@ -55,7 +55,7 @@ class SelectionTester : public TrajectoryAnalysisModule
         SelectionTester();
         virtual ~SelectionTester();
 
-        virtual Options *initOptions(TrajectoryAnalysisSettings *settings);
+        virtual Options &initOptions(TrajectoryAnalysisSettings *settings);
         virtual void initAnalysis(const TrajectoryAnalysisSettings &settings,
                                   const TopologyInformation &top);
 
@@ -94,7 +94,7 @@ SelectionTester::printSelections()
     fprintf(stderr, "\n");
 }
 
-Options *
+Options &
 SelectionTester::initOptions(TrajectoryAnalysisSettings * /*settings*/)
 {
     static const char *const desc[] = {
@@ -110,7 +110,7 @@ SelectionTester::initOptions(TrajectoryAnalysisSettings * /*settings*/)
     _options.addOption(IntegerOption("pmax").store(&_nmaxind)
                            .description("Maximum number of indices to print in lists (-1 = print all)"));
 
-    return &_options;
+    return _options;
 }
 
 void