[RF] Replace more createIterator occurences by range-based loops

This is part of the RooFit code modernization. After this PR, we will
probably only need one more to replace the remaining occurences of
`createIterator`.

Author: guitargeek

roofit/histfactory/src/RooBarlowBeestonLL.cxx

@@ -56,8 +56,6 @@ ClassImp(RooStats::HistFactory::RooBarlowBeestonLL);
 {
   // Default constructor
   // Should only be used by proof.
-  //  _piter = _par.createIterator() ;
-  //  _oiter = _obs.createIterator() ;
 }
 
 
@@ -86,9 +84,6 @@ RooStats::HistFactory::RooBarlowBeestonLL::RooBarlowBeestonLL(const char *name,
 
   delete actualObs ;
   delete actualPars ;
-
-  _piter = _par.createIterator() ;
-  _oiter = _obs.createIterator() ;
   */
 }
 
@@ -106,9 +101,6 @@ RooStats::HistFactory::RooBarlowBeestonLL::RooBarlowBeestonLL(const RooBarlowBee
 {
   // Copy constructor
 
-  //  _piter = _par.createIterator() ;
-  //  _oiter = _obs.createIterator() ;
-
   // _paramAbsMin.addClone(other._paramAbsMin) ;
   // _obsAbsMin.addClone(other._obsAbsMin) ;
 

roofit/roofit/src/RooMomentMorphFuncND.cxx

@@ -248,18 +248,6 @@ void RooMomentMorphFuncND::cartesian_product(vector<vector<T>> &out, vector<vect
 //_____________________________________________________________________________
 void RooMomentMorphFuncND::initialize()
 {
-   // TIterator* pdfItr = _referenceGrid._pdfList.createIterator() ;
-   // RooAbsReal* pdf ;
-   // for (int i=0; (pdf = dynamic_cast<RooAbsReal*>(pdfItr->Next())); ++i) {
-   //   if (!pdf) {
-   //     coutE(InputArguments) << "RooMomentMorphFunc::ctor(" << GetName() << ") ERROR: pdf " << pdf->GetName() << " is
-   //     not of type RooAbsReal" << endl ;
-   //     throw string("RooPolyMorh::ctor() ERROR pdf is not of type RooAbsReal") ;
-   //   }
-   //   _pdfList.addClone(*pdf) ;
-   // }
-   // delete pdfItr ;
-
    for (vector<RooAbsBinning *>::iterator itr = _referenceGrid._grid.begin(); itr != _referenceGrid._grid.end();
         ++itr) {
       _referenceGrid._nnuis.push_back((*itr)->numBins() + 1);

roofit/roofit/src/RooMomentMorphND.cxx

@@ -248,18 +248,6 @@ inline void cartesian_product(vector<vector<T>> &out, vector<vector<T>> &in)
 //_____________________________________________________________________________
 void RooMomentMorphND::initialize()
 {
-   // TIterator* pdfItr = _referenceGrid._pdfList.createIterator() ;
-   // RooAbsPdf* pdf ;
-   // for (int i=0; (pdf = dynamic_cast<RooAbsPdf*>(pdfItr->Next())); ++i) {
-   //   if (!pdf) {
-   //     coutE(InputArguments) << "RooMomentMorph::ctor(" << GetName() << ") ERROR: pdf " << pdf->GetName() << " is not
-   //     of type RooAbsPdf" << endl ;
-   //     throw string("RooPolyMorh::ctor() ERROR pdf is not of type RooAbsPdf") ;
-   //   }
-   //   _pdfList.addClone(*pdf) ;
-   // }
-   // delete pdfItr ;
-
    for (vector<RooAbsBinning *>::iterator itr = _referenceGrid._grid.begin(); itr != _referenceGrid._grid.end();
         ++itr) {
       _referenceGrid._nnuis.push_back((*itr)->numBins() + 1);

roofit/roofitcore/inc/RooExpensiveObjectCache.h

@@ -24,8 +24,8 @@
 class RooExpensiveObjectCache : public TObject {
 public:
 
-  RooExpensiveObjectCache() ;
-  RooExpensiveObjectCache(const RooExpensiveObjectCache&) ;
+  RooExpensiveObjectCache() {}
+  RooExpensiveObjectCache(const RooExpensiveObjectCache& other) : TObject(other) {}
   ~RooExpensiveObjectCache() override ;
 
   bool registerObject(const char* ownerName, const char* objectName, TObject& cacheObject, TIterator* paramIter) ;
@@ -75,7 +75,7 @@ class RooExpensiveObjectCache : public TObject {
 
 protected:
 
-  Int_t _nextUID ;
+  Int_t _nextUID = 0;
 
   std::map<TString,ExpensiveObject*> _map ;
 

roofit/roofitcore/src/RooDataSet.cxx

@@ -400,26 +400,18 @@ RooDataSet::RooDataSet(RooStringView name, RooStringView title, const RooArgSet&
 
     // process StoreError requests
     if (errorSet) {
-      RooArgSet* intErrorSet = (RooArgSet*) _vars.selectCommon(*errorSet) ;
+      std::unique_ptr<RooArgSet> intErrorSet{static_cast<RooArgSet*>(_vars.selectCommon(*errorSet))};
       intErrorSet->setAttribAll("StoreError") ;
-      TIterator* iter = intErrorSet->createIterator() ;
-      RooAbsArg* arg ;
-      while((arg=(RooAbsArg*)iter->Next())) {
+      for(RooAbsArg* arg : *intErrorSet) {
         arg->attachToStore(*_dstore) ;
       }
-      delete iter ;
-      delete intErrorSet ;
     }
     if (asymErrorSet) {
-      RooArgSet* intAsymErrorSet = (RooArgSet*) _vars.selectCommon(*asymErrorSet) ;
+      std::unique_ptr<RooArgSet> intAsymErrorSet{static_cast<RooArgSet*>(_vars.selectCommon(*asymErrorSet))};
       intAsymErrorSet->setAttribAll("StoreAsymError") ;
-      TIterator* iter = intAsymErrorSet->createIterator() ;
-      RooAbsArg* arg ;
-      while((arg=(RooAbsArg*)iter->Next())) {
+      for(RooAbsArg* arg : *intAsymErrorSet) {
         arg->attachToStore(*_dstore) ;
       }
-      delete iter ;
-      delete intAsymErrorSet ;
     }
 
     // Lookup name of weight variable if it was specified by object reference
@@ -1797,10 +1789,8 @@ void RooDataSet::printValue(ostream& os) const
 void RooDataSet::printArgs(ostream& os) const
 {
   os << "[" ;
-  TIterator* iter = _varsNoWgt.createIterator() ;
-  RooAbsArg* arg ;
   bool first(true) ;
-  while((arg=(RooAbsArg*)iter->Next())) {
+  for(RooAbsArg* arg : _varsNoWgt) {
     if (first) {
       first=false ;
     } else {
@@ -1812,7 +1802,6 @@ void RooDataSet::printArgs(ostream& os) const
     os << ",weight:" << _wgtVar->GetName() ;
   }
   os << "]" ;
-  delete iter ;
 }
 
 

roofit/roofitcore/src/RooExpensiveObjectCache.cxx

@@ -26,40 +26,19 @@ the object is valid, so that other instances can, at a later moment
 retrieve these precalculated objects.
 **/
 
+#include "RooExpensiveObjectCache.h"
 
 #include "TClass.h"
 #include "RooAbsReal.h"
 #include "RooAbsCategory.h"
 #include "RooArgSet.h"
 #include "RooMsgService.h"
 #include <iostream>
-using namespace std ;
-
-#include "RooExpensiveObjectCache.h"
 
 ClassImp(RooExpensiveObjectCache);
 ClassImp(RooExpensiveObjectCache::ExpensiveObject);
 
 
-////////////////////////////////////////////////////////////////////////////////
-/// Constructor
-
-RooExpensiveObjectCache::RooExpensiveObjectCache() : _nextUID(0)
-{
-}
-
-
-
-////////////////////////////////////////////////////////////////////////////////
-/// Copy constructor
-
-RooExpensiveObjectCache::RooExpensiveObjectCache(const RooExpensiveObjectCache& other) :
-  TObject(other), _nextUID(0)
-{
-}
-
-
-
 ////////////////////////////////////////////////////////////////////////////////
 /// Destructor.
 
@@ -231,7 +210,7 @@ RooExpensiveObjectCache::ExpensiveObject::ExpensiveObject(Int_t uidIn, const cha
       if (cat) {
    _catRefParams[cat->GetName()] = cat->getCurrentIndex() ;
       } else {
-   oocoutW(&inPayload,Caching) << "RooExpensiveObject::registerObject() WARNING: ignoring non-RooAbsReal/non-RooAbsCategory reference parameter " << arg->GetName() << endl ;
+   oocoutW(&inPayload,Caching) << "RooExpensiveObject::registerObject() WARNING: ignoring non-RooAbsReal/non-RooAbsCategory reference parameter " << arg->GetName() << std::endl ;
       }
     }
   }
@@ -305,12 +284,9 @@ bool RooExpensiveObjectCache::ExpensiveObject::matches(TClass* tc, const RooArgS
 
 void RooExpensiveObjectCache::print() const
 {
-  map<TString,ExpensiveObject*>::const_iterator iter = _map.begin() ;
-
-  while(iter!=_map.end()) {
-    cout << "uid = " << iter->second->uid() << " key=" << iter->first << " value=" ;
-    iter->second->print() ;
-    ++iter ;
+  for(auto const& item : _map) {
+    std::cout << "uid = " << item.second->uid() << " key=" << item.first << " value=" ;
+    item.second->print() ;
   }
 }
 
@@ -320,22 +296,22 @@ void RooExpensiveObjectCache::print() const
 
 void RooExpensiveObjectCache::ExpensiveObject::print() const
 {
-  cout << _payload->ClassName() << "::" << _payload->GetName() ;
+  std::cout << _payload->ClassName() << "::" << _payload->GetName() ;
   if (_realRefParams.size()>0 || _catRefParams.size()>0) {
-    cout << " parameters=( " ;
+    std::cout << " parameters=( " ;
     auto iter = _realRefParams.begin() ;
     while(iter!=_realRefParams.end()) {
-      cout << iter->first << "=" << iter->second << " " ;
+      std::cout << iter->first << "=" << iter->second << " " ;
       ++iter ;
     }
     auto iter2 = _catRefParams.begin() ;
     while(iter2!=_catRefParams.end()) {
-      cout << iter2->first << "=" << iter2->second << " " ;
+      std::cout << iter2->first << "=" << iter2->second << " " ;
       ++iter2 ;
     }
-    cout << ")" ;
+    std::cout << ")" ;
   }
-  cout << endl ;
+  std::cout << std::endl ;
 }
 
 
@@ -345,16 +321,14 @@ void RooExpensiveObjectCache::ExpensiveObject::print() const
 
 void RooExpensiveObjectCache::importCacheObjects(RooExpensiveObjectCache& other, const char* ownerName, bool verbose)
 {
-  map<TString,ExpensiveObject*>::const_iterator iter = other._map.begin() ;
-  while(iter!=other._map.end()) {
-    if (string(ownerName)==iter->second->ownerName()) {
-      _map[iter->first.Data()] = new ExpensiveObject(_nextUID++, *iter->second) ;
+  for(auto const& item : other._map) {
+    if (std::string(ownerName)==item.second->ownerName()) {
+      _map[item.first.Data()] = new ExpensiveObject(_nextUID++, *item.second) ;
       if (verbose) {
-   oocoutI(iter->second->payload(),Caching) << "RooExpensiveObjectCache::importCache() importing cache object "
-                   << iter->first << " associated with object " << iter->second->ownerName() << endl ;
+   oocoutI(item.second->payload(),Caching) << "RooExpensiveObjectCache::importCache() importing cache object "
+                   << item.first << " associated with object " << item.second->ownerName() << std::endl ;
       }
     }
-    ++iter ;
   }
 
 }