fix virtual specifiers, remove redundant methods, unclutter (key4hep#228

)
jmcarcell · Sep 2, 2024 · ddd3ff3 · ddd3ff3
1 parent edb8535
commit ddd3ff3
Show file tree

Hide file tree

Showing 40 changed files with 47 additions and 186 deletions.
diff --git a/k4FWCore/components/CollectionMerger.cpp b/k4FWCore/components/CollectionMerger.cpp
@@ -121,7 +121,7 @@ struct CollectionMerger final : k4FWCore::Transformer<std::shared_ptr<podio::Col
   void mergeCollections(const std::shared_ptr<podio::CollectionBase>& source,
                         std::shared_ptr<podio::CollectionBase>&       ret) const {
     if (!ret) {
-      ret.reset(new T());
+      ret = std::make_shared<T>();
       if (!m_copy) {
         ret->setSubsetCollection();
       }

diff --git a/k4FWCore/components/EventHeaderCreator.cpp b/k4FWCore/components/EventHeaderCreator.cpp
@@ -26,12 +26,6 @@ EventHeaderCreator::EventHeaderCreator(const std::string& name, ISvcLocator* svc
                   "Name of the EventHeaderCollection that will be stored in the output root file.");
 }
 
-StatusCode EventHeaderCreator::initialize() {
-  if (Gaudi::Algorithm::initialize().isFailure())
-    return StatusCode::FAILURE;
-  return StatusCode::SUCCESS;
-}
-
 StatusCode EventHeaderCreator::execute(const EventContext&) const {
   static int eventNumber = 0;
   debug() << "Filling EventHeader with runNumber " << int(m_runNumber) << " and eventNumber "
@@ -42,9 +36,3 @@ StatusCode EventHeaderCreator::execute(const EventContext&) const {
   header.setEventNumber(eventNumber++ + m_eventNumberOffset);
   return StatusCode::SUCCESS;
 }
-
-StatusCode EventHeaderCreator::finalize() {
-  if (Gaudi::Algorithm::finalize().isFailure())
-    return StatusCode::FAILURE;
-  return StatusCode::SUCCESS;
-}
diff --git a/k4FWCore/components/EventHeaderCreator.h b/k4FWCore/components/EventHeaderCreator.h
@@ -34,9 +34,7 @@ class EventHeaderCreator : public Gaudi::Algorithm {
 public:
   EventHeaderCreator(const std::string& name, ISvcLocator* svcLoc);
 
-  StatusCode initialize();
-  StatusCode execute(const EventContext&) const;
-  StatusCode finalize();
+  StatusCode execute(const EventContext&) const override;
 
 private:
   // Run number value (fixed for the entire job, to be set by the job submitter)

diff --git a/k4FWCore/components/FCCDataSvc.cpp b/k4FWCore/components/FCCDataSvc.cpp
@@ -28,6 +28,3 @@ FCCDataSvc::FCCDataSvc(const std::string& name, ISvcLocator* svc) : PodioDataSvc
   declareProperty("inputs", m_filenames = {}, "Names of the files to read");
   declareProperty("input", m_filename = "", "Name of the file to read");
 }
-
-/// Standard Destructor
-FCCDataSvc::~FCCDataSvc() {}
diff --git a/k4FWCore/components/FCCDataSvc.h b/k4FWCore/components/FCCDataSvc.h
@@ -27,8 +27,5 @@ class FCCDataSvc : public PodioDataSvc {
 public:
   /// Standard Constructor
   FCCDataSvc(const std::string& name, ISvcLocator* svc);
-
-  /// Standard Destructor
-  virtual ~FCCDataSvc();
 };
 #endif
diff --git a/k4FWCore/components/IOSvc.h b/k4FWCore/components/IOSvc.h
@@ -40,8 +40,6 @@ class IOSvc : public extends<Service, IIOSvc, IIncidentListener> {
   using extends::extends;
 
 public:
-  ~IOSvc() override = default;
-
   StatusCode initialize() override;
   StatusCode finalize() override;
 

diff --git a/k4FWCore/components/MetadataSvc.cpp b/k4FWCore/components/MetadataSvc.cpp
@@ -25,6 +25,8 @@
 #include <GaudiKernel/IDataProviderSvc.h>
 #include <GaudiKernel/Service.h>
 
+#include <memory>
+
 StatusCode MetadataSvc::initialize() {
   StatusCode sc = Service::initialize();
   if (sc.isFailure()) {
@@ -37,13 +39,13 @@ StatusCode MetadataSvc::initialize() {
     return StatusCode::FAILURE;
   }
 
-  m_frame.reset(new podio::Frame());
+  m_frame = std::make_unique<podio::Frame>();
 
   return StatusCode::SUCCESS;
 }
 
 StatusCode MetadataSvc::finalize() { return Service::finalize(); }
 
-void MetadataSvc::setFrame(podio::Frame&& fr) { m_frame.reset(new podio::Frame(std::move(fr))); }
+void MetadataSvc::setFrame(podio::Frame&& fr) { m_frame = std::make_unique<podio::Frame>(std::move(fr)); }
 
 DECLARE_COMPONENT(MetadataSvc)
diff --git a/k4FWCore/components/MetadataSvc.h b/k4FWCore/components/MetadataSvc.h
@@ -30,8 +30,6 @@ class MetadataSvc : public extends<Service, IMetadataSvc> {
   using extends::extends;
 
 public:
-  ~MetadataSvc() override = default;
-
   StatusCode initialize() override;
   StatusCode finalize() override;
 

diff --git a/k4FWCore/components/PodioOutput.h b/k4FWCore/components/PodioOutput.h
@@ -34,12 +34,12 @@ class PodioOutput : public Gaudi::Algorithm {
   PodioOutput(const std::string& name, ISvcLocator* svcLoc);
 
   /// Initialization of PodioOutput. Acquires the data service, creates trees and root file.
-  StatusCode initialize();
+  StatusCode initialize() override;
   /// Execute. For the first event creates branches for all collections known to PodioDataSvc and prepares them for
   /// writing. For the following events it reconnects the branches with collections and prepares them for write.
-  StatusCode execute(const EventContext&) const;
+  StatusCode execute(const EventContext&) const override;
   /// Finalize. Writes the meta data tree; writes file and cleans up all ROOT-pointers.
-  StatusCode finalize();
+  StatusCode finalize() override;
 
 private:
   /// First event or not

diff --git a/k4FWCore/components/Reader.cpp b/k4FWCore/components/Reader.cpp
@@ -69,7 +69,7 @@ class CollectionPusher : public Gaudi::Functional::details::BaseClass_t<Gaudi::F
                            Gaudi::Details::Property::ImmediatelyInvokeHandler{true}} {}
 
   // derived classes can NOT implement execute
-  StatusCode execute(const EventContext&) const override final {
+  StatusCode execute(const EventContext&) const final {
     try {
       auto out = (*this)();
 

diff --git a/k4FWCore/components/k4DataSvc.cpp b/k4FWCore/components/k4DataSvc.cpp
@@ -29,6 +29,3 @@ k4DataSvc::k4DataSvc(const std::string& name, ISvcLocator* svc) : PodioDataSvc(n
   declareProperty("input", m_filename = "", "Name of the file to read");
   declareProperty("FirstEventEntry", m_1stEvtEntry = 0, "First event to read");
 }
-
-/// Standard Destructor
-k4DataSvc::~k4DataSvc() {}
diff --git a/k4FWCore/components/k4DataSvc.h b/k4FWCore/components/k4DataSvc.h
@@ -26,8 +26,5 @@ class k4DataSvc : public PodioDataSvc {
 public:
   /// Standard Constructor
   k4DataSvc(const std::string& name, ISvcLocator* svc);
-
-  /// Standard Destructor
-  virtual ~k4DataSvc();
 };
 #endif
diff --git a/k4FWCore/include/k4FWCore/Consumer.h b/k4FWCore/include/k4FWCore/Consumer.h
@@ -80,7 +80,7 @@ namespace k4FWCore {
           : Consumer(std::move(name), locator, inputs, std::index_sequence_for<In...>{}) {}
 
       // derived classes are NOT allowed to implement execute ...
-      StatusCode execute(const EventContext& ctx) const override final {
+      StatusCode execute(const EventContext& ctx) const final {
         try {
           filter_evtcontext_tt<In...>::apply(*this, ctx, m_inputs);
           return Gaudi::Functional::FilterDecision::PASSED;

diff --git a/k4FWCore/include/k4FWCore/DataHandle.h b/k4FWCore/include/k4FWCore/DataHandle.h
@@ -118,7 +118,7 @@ template <typename T> const T* DataHandle<T>::get() {
       // only do this once (if both are false after this, we throw exception)
       m_isGoodType = nullptr != dynamic_cast<DataWrapper<T>*>(dataObjectp);
       if (!m_isGoodType) {
-        auto tmp = dynamic_cast<DataWrapper<podio::CollectionBase>*>(dataObjectp);
+        auto* tmp = dynamic_cast<DataWrapper<podio::CollectionBase>*>(dataObjectp);
         if (tmp != nullptr) {
           m_isCollection = nullptr != dynamic_cast<T*>(tmp->collectionBase());
         }
@@ -129,7 +129,7 @@ template <typename T> const T* DataHandle<T>::get() {
     } else if (m_isCollection) {
       // The reader does not know the specific type of the collection. So we need a reinterpret_cast if the handle was
       // created by the reader.
-      DataWrapper<podio::CollectionBase>* tmp = static_cast<DataWrapper<podio::CollectionBase>*>(dataObjectp);
+      auto* tmp = static_cast<DataWrapper<podio::CollectionBase>*>(dataObjectp);
       return reinterpret_cast<const T*>(tmp->collectionBase());
     } else {
       // When a functional has pushed a std::shared_ptr<podio::CollectionBase> into the store
@@ -183,9 +183,6 @@ namespace Gaudi {
   template <class T> class Property<::DataHandle<T>&> : public ::DataHandleProperty {
   public:
     Property(const std::string& name, ::DataHandle<T>& value) : ::DataHandleProperty(name, value) {}
-
-    /// virtual Destructor
-    virtual ~Property() {}
   };
 }  // namespace Gaudi
 

diff --git a/k4FWCore/include/k4FWCore/DataWrapper.h b/k4FWCore/include/k4FWCore/DataWrapper.h
@@ -33,8 +33,7 @@ class GAUDI_API DataWrapperBase : public DataObject {
   // ugly hack to circumvent the usage of boost::any yet
   // DataSvc would need a templated register method
   virtual podio::CollectionBase* collectionBase() = 0;
-  virtual ~DataWrapperBase(){};
-  virtual void resetData() = 0;
+  virtual void                   resetData()      = 0;
 };
 
 template <class T> class GAUDI_API DataWrapper : public DataWrapperBase {
@@ -53,15 +52,15 @@ template <class T> class GAUDI_API DataWrapper : public DataWrapperBase {
   };
   virtual ~DataWrapper();
 
-  const T*     getData() const { return m_data; }
-  void         setData(const T* data) { m_data = data; }
-  virtual void resetData() { m_data = nullptr; }
+  const T* getData() const { return m_data; }
+  void     setData(const T* data) { m_data = data; }
+  void     resetData() override { m_data = nullptr; }
 
   operator const T&() const& { return *m_data; }
 
 private:
   /// try to cast to collectionBase; may return nullptr;
-  virtual podio::CollectionBase* collectionBase();
+  podio::CollectionBase* collectionBase() override;
 
 private:
   const T* m_data;

diff --git a/k4FWCore/include/k4FWCore/IMetadataSvc.h b/k4FWCore/include/k4FWCore/IMetadataSvc.h
@@ -19,6 +19,8 @@
 #ifndef FWCORE_IMETADATASERVICE_H
 #define FWCORE_IMETADATASERVICE_H
 
+#include <memory>
+
 #include "GaudiKernel/IInterface.h"
 
 #include "podio/Frame.h"
@@ -32,7 +34,7 @@ class IMetadataSvc : virtual public IInterface {
   virtual void               setFrame(podio::Frame&& fr) = 0;
   template <typename T> void put(const std::string& name, const T& obj) {
     if (!m_frame) {
-      m_frame.reset(new podio::Frame());
+      m_frame = std::make_unique<podio::Frame>();
     }
     m_frame->putParameter(name, obj);
   }

diff --git a/k4FWCore/include/k4FWCore/KeepDropSwitch.h b/k4FWCore/include/k4FWCore/KeepDropSwitch.h
@@ -31,13 +31,13 @@ class KeepDropSwitch {
 public:
   enum Cmd { KEEP, DROP, UNKNOWN };
   typedef std::vector<std::string> CommandLines;
-  KeepDropSwitch() {}
+  KeepDropSwitch() = default;
   explicit KeepDropSwitch(const CommandLines& cmds) { m_commandlines = cmds; }
   bool isOn(const std::string& astring) const;
 
 private:
   bool                                getFlag(const std::string& astring) const;
-  Cmd                                 extractCommand(const std::string cmdLine) const;
+  Cmd                                 extractCommand(const std::string& cmdLine) const;
   CommandLines                        m_commandlines;
   mutable std::map<std::string, bool> m_cache;
 };

diff --git a/k4FWCore/include/k4FWCore/MetaDataHandle.h b/k4FWCore/include/k4FWCore/MetaDataHandle.h
@@ -29,7 +29,6 @@ template <typename T> class MetaDataHandle {
   MetaDataHandle();
   MetaDataHandle(const std::string& descriptor, Gaudi::DataHandle::Mode a);
   MetaDataHandle(const Gaudi::DataHandle& handle, const std::string& descriptor, Gaudi::DataHandle::Mode a);
-  ~MetaDataHandle();
 
   /// Get the value that is stored in this MetaDataHandle
   ///
@@ -69,8 +68,6 @@ template <typename T> class MetaDataHandle {
   Gaudi::DataHandle::Mode         m_mode;
 };
 
-template <typename T> MetaDataHandle<T>::~MetaDataHandle() {}
-
 //---------------------------------------------------------------------------
 template <typename T>
 MetaDataHandle<T>::MetaDataHandle(const std::string& descriptor, Gaudi::DataHandle::Mode a)

diff --git a/k4FWCore/include/k4FWCore/PodioDataSvc.h b/k4FWCore/include/k4FWCore/PodioDataSvc.h
@@ -57,15 +57,11 @@ class PodioDataSvc : public DataSvc {
   /// Standard Constructor
   PodioDataSvc(const std::string& name, ISvcLocator* svc);
 
-  /// Standard Destructor
-  virtual ~PodioDataSvc();
-
   // Use DataSvc functionality except where we override
   using DataSvc::registerObject;
   /// Overriding standard behaviour of evt service
   /// Register object with the data store.
-  virtual StatusCode registerObject(std::string_view parentPath, std::string_view fullPath,
-                                    DataObject* pObject) override final;
+  StatusCode registerObject(std::string_view parentPath, std::string_view fullPath, DataObject* pObject) final;
 
   const std::string_view getCollectionType(const std::string& collName);
 

diff --git a/k4FWCore/include/k4FWCore/Transformer.h b/k4FWCore/include/k4FWCore/Transformer.h
@@ -109,7 +109,7 @@ namespace k4FWCore {
                         std::index_sequence_for<Out>{}) {}
 
       // derived classes are NOT allowed to implement execute ...
-      StatusCode execute(const EventContext& ctx) const override final {
+      StatusCode execute(const EventContext& ctx) const final {
         try {
           if constexpr (isVectorLike<Out>::value) {
             std::tuple<Out> tmp = filter_evtcontext_tt<In...>::apply(*this, ctx, this->m_inputs);
@@ -245,7 +245,7 @@ namespace k4FWCore {
                              std::index_sequence_for<Out...>{}) {}
 
       // derived classes are NOT allowed to implement execute ...
-      StatusCode execute(const EventContext& ctx) const override final {
+      StatusCode execute(const EventContext& ctx) const final {
         try {
           auto tmp = filter_evtcontext_tt<In...>::apply(*this, ctx, this->m_inputs);
           putVectorOutputs<0, Out...>(std::move(tmp), m_outputs, this);

diff --git a/k4FWCore/src/KeepDropSwitch.cpp b/k4FWCore/src/KeepDropSwitch.cpp
@@ -58,15 +58,14 @@ std::vector<std::string> split(const std::string& s, char delim) {
   std::stringstream        ss(s);
   std::string              item;
   while (std::getline(ss, item, delim)) {
-    if (item != "")
+    if (!item.empty())
       elems.push_back(item);
   }
   return elems;
 }
 
 bool KeepDropSwitch::isOn(const std::string& astring) const {
-  typedef std::map<std::string, bool>::const_iterator MIter;
-  MIter                                               im = m_cache.find(astring);
+  auto im = m_cache.find(astring);
   if (im != m_cache.end())
     return im->second;
   else {
@@ -110,8 +109,8 @@ bool KeepDropSwitch::getFlag(const std::string& astring) const {
   return flag;
 }
 
-KeepDropSwitch::Cmd KeepDropSwitch::extractCommand(const std::string cmdline) const {
-  std::vector<std::string> words = split(cmdline, ' ');
+KeepDropSwitch::Cmd KeepDropSwitch::extractCommand(const std::string& cmdline) const {
+  auto words = split(cmdline, ' ');
   for (auto& word : words)
     std::cout << "'" << word << "' ";
   std::cout << std::endl;

diff --git a/k4FWCore/src/PodioDataSvc.cpp b/k4FWCore/src/PodioDataSvc.cpp
@@ -35,12 +35,12 @@ StatusCode PodioDataSvc::initialize() {
   m_cnvSvc = svc_loc->service("EventPersistencySvc");
   status   = setDataLoader(m_cnvSvc);
 
-  if (m_filename != "") {
+  if (!m_filename.empty()) {
     m_filenames.push_back(m_filename);
   }
 
-  if (m_filenames.size() > 0) {
-    if (m_filenames[0] != "") {
+  if (!m_filenames.empty()) {
+    if (!m_filenames[0].empty()) {
       m_reading_from_file = true;
       m_reader.openFiles(m_filenames);
       m_numAvailableEvents = m_reader.getEntries("events");
@@ -84,7 +84,7 @@ StatusCode PodioDataSvc::reinitialize() {
 }
 /// Service finalization
 StatusCode PodioDataSvc::finalize() {
-  m_cnvSvc = 0;  // release
+  m_cnvSvc = nullptr;  // release
   DataSvc::finalize().ignore();
   return StatusCode::SUCCESS;
 }
@@ -143,9 +143,6 @@ void PodioDataSvc::endOfRead() {
 /// Standard Constructor
 PodioDataSvc::PodioDataSvc(const std::string& name, ISvcLocator* svc) : DataSvc(name, svc) {}
 
-/// Standard Destructor
-PodioDataSvc::~PodioDataSvc() {}
-
 const std::string_view PodioDataSvc::getCollectionType(const std::string& collName) {
   const auto coll = m_eventframe.get(collName);
   if (coll == nullptr) {
@@ -156,7 +153,7 @@ const std::string_view PodioDataSvc::getCollectionType(const std::string& collNa
 }
 
 StatusCode PodioDataSvc::registerObject(std::string_view parentPath, std::string_view fullPath, DataObject* pObject) {
-  DataWrapperBase* wrapper = dynamic_cast<DataWrapperBase*>(pObject);
+  auto* wrapper = dynamic_cast<DataWrapperBase*>(pObject);
   if (wrapper != nullptr) {
     podio::CollectionBase* coll = wrapper->collectionBase();
     if (coll != nullptr) {

diff --git a/k4Interface/include/k4Interface/IGeoSvc.h b/k4Interface/include/k4Interface/IGeoSvc.h
@@ -36,7 +36,6 @@ class GAUDI_API IGeoSvc : virtual public IService {
   virtual dd4hep::Detector*            getDetector()                             = 0;
   virtual G4VUserDetectorConstruction* getGeant4Geo()                            = 0;
   virtual std::string                  constantAsString(std::string const& name) = 0;
-  virtual ~IGeoSvc() {}
 };
 
 #endif  // IGEOSVC_H
diff --git a/k4Interface/include/k4Interface/ITowerTool.h b/k4Interface/include/k4Interface/ITowerTool.h
@@ -85,8 +85,6 @@ class ITowerTool : virtual public IAlgTool {
   virtual void attachCells(float aEta, float aPhi, uint aHalfEtaFinal, uint aHalfPhiFinal,
                            edm4hep::MutableCluster& aEdmCluster, edm4hep::CalorimeterHitCollection* aEdmClusterCells,
                            bool aEllipse) = 0;
-
-  virtual ~ITowerTool() {}
 };
 
 #endif /* RECINTERFACE_ITOWERTOOL_H */
diff --git a/k4Interface/include/k4Interface/ITowerToolThetaModule.h b/k4Interface/include/k4Interface/ITowerToolThetaModule.h
@@ -85,8 +85,6 @@ class ITowerToolThetaModule : virtual public IAlgTool {
   virtual void attachCells(float aTheta, float aPhi, uint aHalfThetaFinal, uint aHalfPhiFinal,
                            edm4hep::MutableCluster& aEdmCluster, edm4hep::CalorimeterHitCollection* aEdmClusterCells,
                            bool aEllipse) = 0;
-
-  virtual ~ITowerToolThetaModule() {}
 };
 
 #endif /* RECINTERFACE_ITOWERTOOLTHETAMODULE_H */