Re #6198. Just return the maps by value.

In C++11, map has a move constructor so returning by value isn't expensive. Actually, even on the non-C++11 Mac tests show that the RVO saves us. Now to fix all the client code...
mantidproject · Sep 18, 2013 · 1f525ef · 1f525ef
1 parent 3f28a3b
commit 1f525ef
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 20 deletions.
diff --git a/Code/Mantid/Framework/API/inc/MantidAPI/MatrixWorkspace.h b/Code/Mantid/Framework/API/inc/MantidAPI/MatrixWorkspace.h
@@ -109,16 +109,16 @@ namespace Mantid
       void rebuildSpectraMapping(const bool includeMonitors = true);
 
       // More mapping
-      spec2index_map * getSpectrumToWorkspaceIndexMap() const;
-      detid2index_map * getDetectorIDToWorkspaceIndexMap( bool throwIfMultipleDets=false ) const;
+      spec2index_map getSpectrumToWorkspaceIndexMap() const;
+      detid2index_map getDetectorIDToWorkspaceIndexMap( bool throwIfMultipleDets=false ) const;
       void getDetectorIDToWorkspaceIndexVector( std::vector<size_t> & out, detid_t & offset, bool throwIfMultipleDets=false) const;
       void getSpectrumToWorkspaceIndexVector(std::vector<size_t> & out, specid_t & offset) const;
       void getIndicesFromSpectra(const std::vector<specid_t>& spectraList, std::vector<size_t>& indexList) const;
       size_t getIndexFromSpectrumNumber(const specid_t specNo) const;
       void getIndicesFromDetectorIDs(const std::vector<detid_t>& detIdList, std::vector<size_t>& indexList) const;
       void getSpectraFromDetectorIDs(const std::vector<detid_t>& detIdList, std::vector<specid_t>& spectraList) const;
 
-	  bool hasGroupedDetectors() const;
+      bool hasGroupedDetectors() const;
 
       /// Get the footprint in memory in bytes.
       virtual size_t getMemorySize() const;

diff --git a/Code/Mantid/Framework/API/src/MatrixWorkspace.cpp b/Code/Mantid/Framework/API/src/MatrixWorkspace.cpp
@@ -315,19 +315,18 @@ namespace Mantid
     *    KEY is the Spectrum #
     *    VALUE is the Workspace Index
     */
-    spec2index_map * MatrixWorkspace::getSpectrumToWorkspaceIndexMap() const
+    spec2index_map MatrixWorkspace::getSpectrumToWorkspaceIndexMap() const
     {
       SpectraAxis * ax = dynamic_cast<SpectraAxis * >( this->m_axes[1] );
       if (!ax)
         throw std::runtime_error("MatrixWorkspace::getSpectrumToWorkspaceIndexMap: axis[1] is not a SpectraAxis, so I cannot generate a map.");
-      spec2index_map * map = new spec2index_map();
+      spec2index_map map;
       try
       {
-        ax->getSpectraIndexMap(*map);
+        ax->getSpectraIndexMap(map);
       }
       catch (std::runtime_error &)
       {
-        delete map;
         throw std::runtime_error("MatrixWorkspace::getSpectrumToWorkspaceIndexMap: no elements!");
       }
       return map;
@@ -405,36 +404,36 @@ namespace Mantid
     *  @throw runtime_error if there is more than one detector per spectrum (if throwIfMultipleDets is true)
     *  @return Index to Index Map object. THE CALLER TAKES OWNERSHIP OF THE MAP AND IS RESPONSIBLE FOR ITS DELETION.
     */
-    detid2index_map * MatrixWorkspace::getDetectorIDToWorkspaceIndexMap( bool throwIfMultipleDets ) const
+    detid2index_map MatrixWorkspace::getDetectorIDToWorkspaceIndexMap( bool throwIfMultipleDets ) const
     {
-      detid2index_map * map = new detid2index_map();
+      detid2index_map map;
 
       //Loop through the workspace index
-      for (size_t workspaceIndex=0; workspaceIndex < this->getNumberHistograms(); workspaceIndex++)
+      for (size_t workspaceIndex=0; workspaceIndex < this->getNumberHistograms(); ++workspaceIndex)
       {
         auto detList = getSpectrum(workspaceIndex)->getDetectorIDs();
 
         if (throwIfMultipleDets)
         {
           if (detList.size() > 1)
           {
-            delete map;
             throw std::runtime_error("MatrixWorkspace::getDetectorIDToWorkspaceIndexMap(): more than 1 detector for one histogram! I cannot generate a map of detector ID to workspace index.");
           }
 
           //Set the KEY to the detector ID and the VALUE to the workspace index.
           if (detList.size() == 1)
-            (*map)[ *detList.begin() ] = workspaceIndex;
+            map[ *detList.begin() ] = workspaceIndex;
         }
         else
         {
           //Allow multiple detectors per workspace index
           for (auto it = detList.begin(); it != detList.end(); ++it)
-            (*map)[ *it ] = workspaceIndex;
+            map[ *it ] = workspaceIndex;
         }
 
         //Ignore if the detector list is empty.
       }
+
       return map;
     }
 

diff --git a/Code/Mantid/Framework/API/test/MatrixWorkspaceTest.h b/Code/Mantid/Framework/API/test/MatrixWorkspaceTest.h
@@ -589,22 +589,39 @@ class MatrixWorkspaceTest : public CxxTest::TestSuite
 
   }
 
+  void test_getSpectrumToWorkspaceIndexMap()
+  {
+    WorkspaceTester ws;
+    ws.initialize(2,1,1);
+    const auto map = ws.getSpectrumToWorkspaceIndexMap();
+    TS_ASSERT_EQUALS( map.size(), 2 );
+    TS_ASSERT_EQUALS( map.begin()->first, 1 );
+    TS_ASSERT_EQUALS( map.begin()->second, 0 );
+    TS_ASSERT_EQUALS( map.rbegin()->first, 2 );
+    TS_ASSERT_EQUALS( map.rbegin()->second, 1 );
+
+    // Check it throws for non-spectra axis
+    ws.replaceAxis(1,new NumericAxis(1));
+    TS_ASSERT_THROWS( ws.getSpectrumToWorkspaceIndexMap(), std::runtime_error);
+  }
+
   void test_getDetectorIDToWorkspaceIndexMap()
   {
     auto ws = makeWorkspaceWithDetectors(5, 1);
-    boost::scoped_ptr<detid2index_map> idmap(ws->getDetectorIDToWorkspaceIndexMap(true));
-    TS_ASSERT_EQUALS( idmap->size(), 5 );
+    detid2index_map idmap = ws->getDetectorIDToWorkspaceIndexMap(true);
+
+    TS_ASSERT_EQUALS( idmap.size(), 5 );
     int i = 0;
-    for ( auto it = idmap->begin(); it != idmap->end(); ++it, ++i )
+    for ( auto it = idmap.begin(); it != idmap.end(); ++it, ++i )
     {
-      TS_ASSERT_EQUALS( idmap->count(i), 1 );
-      TS_ASSERT_EQUALS( (*idmap)[i], i );
+      TS_ASSERT_EQUALS( idmap.count(i), 1 );
+      TS_ASSERT_EQUALS( idmap[i], i );
     }
 
     ws->getSpectrum(2)->addDetectorID(99); // Set a second ID on one spectrum
     TS_ASSERT_THROWS( ws->getDetectorIDToWorkspaceIndexMap(true), std::runtime_error );
-    boost::scoped_ptr<detid2index_map> idmap2(ws->getDetectorIDToWorkspaceIndexMap());
-    TS_ASSERT_EQUALS( idmap2->size(), 6 );
+    detid2index_map idmap2 = ws->getDetectorIDToWorkspaceIndexMap();
+    TS_ASSERT_EQUALS( idmap2.size(), 6 );
   }
 
   void test_getDetectorIDToWorkspaceIndexVector()