From 2b27273463dd88f82f29c529e6335ab18876e2d6 Mon Sep 17 00:00:00 2001
From: "E. G. Patrick Bos" <egpbos@gmail.com>
Date: Thu, 15 Jul 2021 15:30:06 +0200
Subject: [PATCH] apply more review comments

From comments by @guitargeek and @hageboeck on PR #8567 (https://github.com/root-project/root/pull/8567):

- Simplify copy constructor.
- Inline simple getter/setters.
- Replace fabs with std::abs.
- Fix warning due to changed member ordering.
---
 math/minuit2/inc/Minuit2/NumericalDerivator.h | 15 +++----
 math/minuit2/src/NumericalDerivator.cxx       | 44 ++-----------------
 2 files changed, 11 insertions(+), 48 deletions(-)

diff --git a/math/minuit2/inc/Minuit2/NumericalDerivator.h b/math/minuit2/inc/Minuit2/NumericalDerivator.h
index 889ad9279e25f..ee6203274f066 100644
--- a/math/minuit2/inc/Minuit2/NumericalDerivator.h
+++ b/math/minuit2/inc/Minuit2/NumericalDerivator.h
@@ -61,10 +61,10 @@ class NumericalDerivator {
                                const DerivatorElement &previous);
 
    double GetValue() const { return fVal; }
-   void SetStepTolerance(double value);
-   void SetGradTolerance(double value);
-   void SetNCycles(unsigned int value);
-   void SetErrorLevel(double value);
+   inline void SetStepTolerance(double value) { fStepTolerance = value; }
+   inline void SetGradTolerance(double value) { fGradTolerance = value; }
+   inline void SetNCycles(unsigned int value) { fNCycles = value; }
+   inline void SetErrorLevel(double value) { fUp = value; }
 
    double Int2ext(const ROOT::Fit::ParameterSettings &parameter, double val) const;
    double Ext2int(const ROOT::Fit::ParameterSettings &parameter, double val) const;
@@ -74,9 +74,9 @@ class NumericalDerivator {
                            const std::vector<ROOT::Fit::ParameterSettings> &parameters,
                            std::vector<DerivatorElement> &gradient);
 
-   bool AlwaysExactlyMimicMinuit2() const;
-   void SetAlwaysExactlyMimicMinuit2(bool flag);
-   
+   inline bool AlwaysExactlyMimicMinuit2() const { return fAlwaysExactlyMimicMinuit2; }
+   inline void SetAlwaysExactlyMimicMinuit2(bool flag) { fAlwaysExactlyMimicMinuit2 = flag; }
+
 private:
    double fStepTolerance = 0.5;
    double fGradTolerance = 0.1;
@@ -100,7 +100,6 @@ class NumericalDerivator {
    unsigned int fNCycles = 2;
    bool fAlwaysExactlyMimicMinuit2;
 
-
 };
 
 std::ostream &operator<<(std::ostream &out, const DerivatorElement &value);
diff --git a/math/minuit2/src/NumericalDerivator.cxx b/math/minuit2/src/NumericalDerivator.cxx
index 03939723573a8..6c2b3f2ac5ec0 100644
--- a/math/minuit2/src/NumericalDerivator.cxx
+++ b/math/minuit2/src/NumericalDerivator.cxx
@@ -44,39 +44,13 @@ NumericalDerivator::NumericalDerivator(bool always_exactly_mimic_minuit2)
 
 NumericalDerivator::NumericalDerivator(double step_tolerance, double grad_tolerance, unsigned int ncycles,
                                        double error_level, bool always_exactly_mimic_minuit2)
-   : fStepTolerance(step_tolerance), fGradTolerance(grad_tolerance), fNCycles(ncycles), fUp(error_level),
+   : fStepTolerance(step_tolerance), fGradTolerance(grad_tolerance), fUp(error_level), fNCycles(ncycles),
      fAlwaysExactlyMimicMinuit2(always_exactly_mimic_minuit2)
 {
 }
 
-// deep copy constructor
-NumericalDerivator::NumericalDerivator(const NumericalDerivator &other)
-   : fStepTolerance(other.fStepTolerance), fGradTolerance(other.fGradTolerance), fNCycles(other.fNCycles),
-     fUp(other.fUp), fVal(other.fVal), fVx(other.fVx), fVxExternal(other.fVxExternal), fDfmin(other.fDfmin),
-     fVrysml(other.fVrysml), fPrecision(other.fPrecision), fAlwaysExactlyMimicMinuit2(other.fAlwaysExactlyMimicMinuit2),
-     fVxFValCache(other.fVxFValCache)
-{
-}
-
-void NumericalDerivator::SetStepTolerance(double value)
-{
-   fStepTolerance = value;
-}
-
-void NumericalDerivator::SetGradTolerance(double value)
-{
-   fGradTolerance = value;
-}
-
-void NumericalDerivator::SetNCycles(unsigned int value)
-{
-   fNCycles = value;
-}
+NumericalDerivator::NumericalDerivator(const NumericalDerivator &/*other*/) = default;
 
-void NumericalDerivator::SetErrorLevel(double value)
-{
-   fUp = value;
-}
 
 /// This function sets internal state based on input parameters. This state
 /// setup is used in the actual (partial) derivative calculations.
@@ -292,9 +266,9 @@ void NumericalDerivator::SetInitialGradient(const ROOT::Math::IBaseFunctionMulti
 
       var2 = Ext2int(*parameter, sav2);
       double vmin = var2 - var;
-      double gsmin = 8. * eps2 * (fabs(var) + eps2);
+      double gsmin = 8. * eps2 * (std::abs(var) + eps2);
       // protect against very small step sizes which can cause dirin to zero and then nan values in grd
-      double dirin = std::max(0.5 * (fabs(vplu) + fabs(vmin)), gsmin);
+      double dirin = std::max(0.5 * (std::abs(vplu) + std::abs(vmin)), gsmin);
       double g2 = 2.0 * fUp / (dirin * dirin);
       double gstep = std::max(gsmin, 0.1 * dirin);
       double grd = g2 * dirin;
@@ -309,16 +283,6 @@ void NumericalDerivator::SetInitialGradient(const ROOT::Math::IBaseFunctionMulti
    }
 }
 
-bool NumericalDerivator::AlwaysExactlyMimicMinuit2() const
-{
-   return fAlwaysExactlyMimicMinuit2;
-};
-
-void NumericalDerivator::SetAlwaysExactlyMimicMinuit2(bool flag)
-{
-   fAlwaysExactlyMimicMinuit2 = flag;
-}
-
 std::ostream &operator<<(std::ostream &out, const DerivatorElement &value)
 {
    return out << "(derivative: " << value.derivative << ", second_derivative: " << value.second_derivative