Clean up RooAddition, RooConstraintSum of member iterators.

Small cleanups and comments in RooSpan, RooAbsTestStatistic, RooAbsReal.

roofit/roofitcore/inc/RooAddition.h

@@ -60,7 +60,6 @@ class RooAddition : public RooAbsReal {
 
   RooArgList   _ownedList ;      // List of owned components
   RooListProxy _set ;            // set of terms to be summed
-  mutable TIterator* _setIter ;  //! Iterator over set
 
   class CacheElem : public RooAbsCacheElement {
   public:

roofit/roofitcore/inc/RooConstraintSum.h

@@ -40,7 +40,6 @@ class RooConstraintSum : public RooAbsReal {
 
   RooListProxy _set1 ;    // Set of constraint terms
   RooSetProxy _paramSet ; // Set of parameters to which constraints apply
-  TIterator* _setIter1 ;  //! do not persist
 
   Double_t evaluate() const;
 

roofit/roofitcore/inc/RooSpan.h

@@ -114,7 +114,7 @@ class RooSpan {
 
 private:
 
-  /// If a class has does not own a contiguous block of memory, which
+  /// If a class does not own a contiguous block of memory, which
   /// could be used to create a span, the memory has to be kept alive
   /// until all referring spans are destroyed.
   std::shared_ptr<std::vector<value_type>> _auxStorage;

roofit/roofitcore/src/RooAbsReal.cxx

@@ -4789,7 +4789,7 @@ void RooAbsReal::evaluateBatch(RooSpan<double> output,
   std::vector<std::pair<std::size_t, RooAbsRealLValue*>> serverVars;
   //First find out which values from the inputVars we need to set,
   //because we depend on them.
-  for (int i = 0; i < inputVars.size(); ++i) {
+  for (unsigned int i = 0; i < inputVars.size(); ++i) {
     const RooAbsArg* var = inputVars[i];
 //    std::cout << "\n\nVar #" << i << " coming in:\n";
 //    var->Print("v");

roofit/roofitcore/src/RooAbsTestStatistic.cxx

@@ -234,11 +234,11 @@ RooAbsTestStatistic::~RooAbsTestStatistic()
 
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Calculates and return value of test statistic. If the test statistic
-/// is calculated from on a RooSimultaneous, the test statistic calculation
+/// Calculate and return value of test statistic. If the test statistic
+/// is calculated from a RooSimultaneous, the test statistic calculation
 /// is performed separately on each simultaneous p.d.f component and associated
-/// data and then combined. If the test statistic calculation is parallelized
-/// partitions are calculated in nCPU processes and a posteriori combined.
+/// data, and then combined. If the test statistic calculation is parallelized,
+/// partitions are calculated in nCPU processes and combined a posteriori.
 
 Double_t RooAbsTestStatistic::evaluate() const
 {

roofit/roofitcore/src/RooAddition.cxx

@@ -22,7 +22,7 @@
 RooAddition calculates the sum of a set of RooAbsReal terms, or
 when constructed with two sets, it sums the product of the terms
 in the two sets. This class does not (yet) do any smart handling of integrals, 
-i.e. all integrals of the product are handled numerically
+i.e. all integrals of the product are handled numerically.
 **/
 
 
@@ -51,7 +51,6 @@ ClassImp(RooAddition);
 ////////////////////////////////////////////////////////////////////////////////
 /// Empty constructor
 RooAddition::RooAddition()
-  : _setIter( _set.createIterator() )
 {
 }
 
@@ -67,12 +66,9 @@ RooAddition::RooAddition()
 RooAddition::RooAddition(const char* name, const char* title, const RooArgList& sumSet, Bool_t takeOwnership) 
   : RooAbsReal(name, title)
   , _set("!set","set of components",this)
-  , _setIter( _set.createIterator() ) // yes, _setIter is defined _after_ _set ;-)
   , _cacheMgr(this,10)
 {
-  std::unique_ptr<TIterator> inputIter( sumSet.createIterator() );
-  RooAbsArg* comp ;
-  while((comp = (RooAbsArg*)inputIter->Next())) {
+  for (const auto comp : sumSet) {
     if (!dynamic_cast<RooAbsReal*>(comp)) {
       coutE(InputArguments) << "RooAddition::ctor(" << GetName() << ") ERROR: component " << comp->GetName() 
 			    << " is not of type RooAbsReal" << endl ;
@@ -105,7 +101,6 @@ RooAddition::RooAddition(const char* name, const char* title, const RooArgList&
 RooAddition::RooAddition(const char* name, const char* title, const RooArgList& sumSet1, const RooArgList& sumSet2, Bool_t takeOwnership) 
     : RooAbsReal(name, title)
     , _set("!set","set of components",this)
-    , _setIter( _set.createIterator() ) // yes, _setIter is defined _after_ _set ;-)
     , _cacheMgr(this,10)
 {
   if (sumSet1.getSize() != sumSet2.getSize()) {
@@ -153,7 +148,6 @@ RooAddition::RooAddition(const char* name, const char* title, const RooArgList&
 RooAddition::RooAddition(const RooAddition& other, const char* name) 
     : RooAbsReal(other, name)
     , _set("!set",this,other._set)
-    , _setIter( _set.createIterator() ) // yes, _setIter is defined _after_ _set ;-)
     , _cacheMgr(other._cacheMgr,this)
 {
   // Member _ownedList is intentionally not copy-constructed -- ownership is not transferred
@@ -164,7 +158,7 @@ RooAddition::RooAddition(const RooAddition& other, const char* name)
 
 RooAddition::~RooAddition() 
 { // Destructor
-  delete _setIter ;
+
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -241,11 +235,8 @@ Double_t RooAddition::defaultErrorLevel() const
 
 void RooAddition::enableOffsetting(Bool_t flag) 
 {
-  _setIter->Reset() ;
-
-  RooAbsReal* arg;
-  while((arg=(RooAbsReal*)_setIter->Next())) {
-    arg->enableOffsetting(flag) ;
+  for (auto arg : _set) {
+    static_cast<RooAbsReal*>(arg)->enableOffsetting(flag) ;
   }  
 }
 
@@ -255,11 +246,8 @@ void RooAddition::enableOffsetting(Bool_t flag)
 
 Bool_t RooAddition::setData(RooAbsData& data, Bool_t cloneData) 
 {
-  _setIter->Reset() ;
-
-  RooAbsReal* arg;
-  while((arg=(RooAbsReal*)_setIter->Next())) {
-    arg->setData(data,cloneData) ;
+  for (const auto arg : _set) {
+    static_cast<RooAbsReal*>(arg)->setData(data,cloneData) ;
   }  
   return kTRUE ;
 }
@@ -270,13 +258,12 @@ Bool_t RooAddition::setData(RooAbsData& data, Bool_t cloneData)
 
 void RooAddition::printMetaArgs(ostream& os) const 
 {
-  _setIter->Reset() ;
-
   Bool_t first(kTRUE) ;
-  RooAbsArg* arg;
-  while((arg=(RooAbsArg*)_setIter->Next())) {
-    if (!first) { os << " + " ;
-    } else { first = kFALSE ; 
+  for (const auto arg : _set) {
+    if (!first) {
+      os << " + " ;
+    } else {
+      first = kFALSE ;
     }
     os << arg->GetName() ; 
   }  
@@ -300,10 +287,8 @@ Int_t RooAddition::getAnalyticalIntegral(RooArgSet& allVars, RooArgSet& analVars
 
   // we don't, so we make it right here....
   cache = new CacheElem;
-  _setIter->Reset();
-  RooAbsReal *arg(0);
-  while( (arg=(RooAbsReal*)_setIter->Next())!=0 ) {  // checked in c'tor that this will work...
-      RooAbsReal *I = arg->createIntegral(analVars,rangeName);
+  for (const auto arg : _set) {// checked in c'tor that this will work...
+      RooAbsReal *I = static_cast<const RooAbsReal*>(arg)->createIntegral(analVars,rangeName);
       cache->_I.addOwned(*I);
   }
 
@@ -330,10 +315,10 @@ Double_t RooAddition::analyticalIntegral(Int_t code, const char* rangeName) cons
   assert(cache!=0);
 
   // loop over cache, and sum...
-  std::unique_ptr<TIterator> iter( cache->_I.createIterator() );
-  RooAbsReal *I;
   double result(0);
-  while ( ( I=(RooAbsReal*)iter->Next() ) != 0 ) result += I->getVal();
+  for (auto I : cache->_I) {
+    result += static_cast<const RooAbsReal*>(I)->getVal();
+  }
   return result;
 
 }

roofit/roofitcore/src/RooConstraintSum.cxx

@@ -22,8 +22,8 @@
 RooConstraintSum calculates the sum of the -(log) likelihoods of
 a set of RooAbsPfs that represent constraint functions. This class
 is used to calculate the composite -log(L) of constraints to be
-added the regular -log(L) in RooAbsPdf::fitTo() with Constrain(..)
-arguments
+added to the regular -log(L) in RooAbsPdf::fitTo() with Constrain(..)
+arguments.
 **/
 
 
@@ -53,7 +53,7 @@ ClassImp(RooConstraintSum);
 
 RooConstraintSum::RooConstraintSum()
 {
-  _setIter1 = _set1.createIterator() ;
+
 }
 
 
@@ -67,12 +67,7 @@ RooConstraintSum::RooConstraintSum(const char* name, const char* title, const Ro
   _set1("set1","First set of components",this),
   _paramSet("paramSet","Set of parameters",this)
 {
-
-  _setIter1 = _set1.createIterator() ;
-
-  TIterator* inputIter = constraintSet.createIterator() ;
-  RooAbsArg* comp ;
-  while((comp = (RooAbsArg*)inputIter->Next())) {
+  for (const auto comp : constraintSet) {
     if (!dynamic_cast<RooAbsPdf*>(comp)) {
       coutE(InputArguments) << "RooConstraintSum::ctor(" << GetName() << ") ERROR: component " << comp->GetName() 
 			    << " is not of type RooAbsPdf" << endl ;
@@ -82,8 +77,6 @@ RooConstraintSum::RooConstraintSum(const char* name, const char* title, const Ro
   }
 
   _paramSet.add(normSet) ;
-
-  delete inputIter ;
 }
 
 
@@ -98,7 +91,7 @@ RooConstraintSum::RooConstraintSum(const RooConstraintSum& other, const char* na
   _set1("set1",this,other._set1),
   _paramSet("paramSet",this,other._paramSet)
 {
-  _setIter1 = _set1.createIterator() ;  
+
 }
 
 
@@ -108,7 +101,7 @@ RooConstraintSum::RooConstraintSum(const RooConstraintSum& other, const char* na
 
 RooConstraintSum::~RooConstraintSum() 
 {
-  if (_setIter1) delete _setIter1 ;
+
 }
 
 
@@ -119,13 +112,11 @@ RooConstraintSum::~RooConstraintSum()
 Double_t RooConstraintSum::evaluate() const 
 {
   Double_t sum(0);
-  RooAbsReal* comp ;
-  RooFIter setIter1 = _set1.fwdIterator() ;
 
-  while((comp=(RooAbsReal*)setIter1.next())) {
-    sum -= ((RooAbsPdf*)comp)->getLogVal(&_paramSet) ;
+  for (const auto comp : _set1) {
+    sum -= static_cast<RooAbsPdf*>(comp)->getLogVal(&_paramSet);
   }
 
-  return sum ;
+  return sum;
 }