From 5455d84221b6c5fddc2cc5850b2137a7ed204b3f Mon Sep 17 00:00:00 2001
From: Stephen Nicholas Swatman <stephen.nicholas.swatman@cern.ch>
Date: Tue, 1 Sep 2020 14:50:38 +0200
Subject: [PATCH] Deduplicate calorimeter eloss state updater

Previously, this bit of code was duplicated three times across the
code, which is a code smell. @cbittric suggested this change in a code
review for !35949. This commit deduplicates the code, converting all
three instances to a call to a single new method.
---
 .../TrkGlobalChi2Fitter/GXFTrajectory.h       |  2 +
 .../TrkGlobalChi2Fitter/src/GXFTrajectory.cxx | 47 +++++++++----------
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/Tracking/TrkFitter/TrkGlobalChi2Fitter/TrkGlobalChi2Fitter/GXFTrajectory.h b/Tracking/TrkFitter/TrkGlobalChi2Fitter/TrkGlobalChi2Fitter/GXFTrajectory.h
index a6980d10e746..2983f85ff095 100644
--- a/Tracking/TrkFitter/TrkGlobalChi2Fitter/TrkGlobalChi2Fitter/GXFTrajectory.h
+++ b/Tracking/TrkFitter/TrkGlobalChi2Fitter/TrkGlobalChi2Fitter/GXFTrajectory.h
@@ -51,6 +51,8 @@ namespace Trk {
     void setChi2(double);
     void setMass(double);
 
+    void conditionalSetCalorimeterEnergyLossState(GXFTrackState *);
+
     std::pair<GXFTrackState *, GXFTrackState *> findFirstLastMeasurement(void);
     bool hasKink(void);
 
diff --git a/Tracking/TrkFitter/TrkGlobalChi2Fitter/src/GXFTrajectory.cxx b/Tracking/TrkFitter/TrkGlobalChi2Fitter/src/GXFTrajectory.cxx
index bfe08707667a..4621243d6ba6 100644
--- a/Tracking/TrkFitter/TrkGlobalChi2Fitter/src/GXFTrajectory.cxx
+++ b/Tracking/TrkFitter/TrkGlobalChi2Fitter/src/GXFTrajectory.cxx
@@ -82,16 +82,7 @@ namespace Trk {
     
     if (rhs.m_caloelossstate != nullptr) {
       for (auto & state : m_states) {
-        GXFMaterialEffects *meff = state->materialEffects();
-        const TrackParameters *par = state->trackParameters();
-        
-        if (
-          (meff != nullptr) && (par != nullptr) && meff->sigmaDeltaE() > 0 && 
-          meff->sigmaDeltaPhi() == 0 && 
-          (par->position().perp() > 1400 || std::abs(par->position().z()) > 3700)
-        ) {
-          m_caloelossstate = state.get();
-        }
+        conditionalSetCalorimeterEnergyLossState(state.get());
       }
     }
     
@@ -141,14 +132,7 @@ namespace Trk {
       
       if (rhs.m_caloelossstate != nullptr) {
         for (auto & state : m_states) {
-          GXFMaterialEffects *meff = state->materialEffects();
-          const TrackParameters *par = state->trackParameters();
-          if (
-            (meff != nullptr) && (par != nullptr) && meff->sigmaDeltaE() > 0 && meff->sigmaDeltaPhi() == 0 && 
-            (par->position().perp() > 1400 || std::abs(par->position().z()) > 3700)
-          ) {
-            m_caloelossstate = state.get();
-          }
+          conditionalSetCalorimeterEnergyLossState(state.get());
         }
       }
       m_upstreammat = rhs.m_upstreammat;
@@ -156,6 +140,24 @@ namespace Trk {
     return *this;
   }
 
+  void GXFTrajectory::conditionalSetCalorimeterEnergyLossState(GXFTrackState * state) {
+    constexpr double perpThreshold = 1400;
+    constexpr double zThreshold = 3700;
+
+    const GXFMaterialEffects *meff = state->materialEffects();
+    const TrackParameters *par = state->trackParameters();
+
+    if (
+      meff != nullptr &&
+      par != nullptr &&
+      meff->sigmaDeltaE() > 0 &&
+      meff->sigmaDeltaPhi() == 0 &&
+      (par->position().perp() > perpThreshold || std::abs(par->position().z()) > zThreshold)
+    ) {
+      m_caloelossstate = state;
+    }
+  }
+
   bool GXFTrajectory::addMeasurementState(std::unique_ptr<GXFTrackState> state, int index) {
     if (!m_states.empty() && (m_states.back()->measurement() != nullptr)) {
       const MeasurementBase *meas = m_states.back()->measurement();
@@ -213,7 +215,6 @@ namespace Trk {
   }
 
   void GXFTrajectory::addMaterialState(std::unique_ptr<GXFTrackState> state, int index) {
-    const TrackParameters *par = state->trackParameters();
     GXFMaterialEffects *meff = state->materialEffects();
     
     if (state->trackStateType() == TrackState::Scatterer) {
@@ -226,13 +227,7 @@ namespace Trk {
     
     if (meff->sigmaDeltaE() > 0) {
       m_nbrems++;
-      
-      if (
-        (par != nullptr) && meff->sigmaDeltaPhi() == 0 && 
-        (par->position().perp() > 1400 || std::abs(par->position().z()) > 3700)
-      ) {
-        m_caloelossstate = state.get();
-      }
+      conditionalSetCalorimeterEnergyLossState(state.get());
     }
     
     m_toteloss += std::abs(meff->deltaE());
-- 
GitLab