Modernize code related to Surfaces and the SurfaceManager (#1498)

Co-authored-by: Juan Miguel Carceller <[email protected]>
Co-authored-by: MarkusFrankATcernch <[email protected]>

Author: jmcarcell

DDRec/include/DDRec/DetectorSurfaces.h

@@ -25,23 +25,19 @@ namespace dd4hep {
      * @date Apr, 10 2014
      * @version $Id$
      */
-    class DetectorSurfaces: virtual public DetElement {
+    class DetectorSurfaces {
 
     public:
-      typedef DetElement DetElement;
-
       DetectorSurfaces(const DetElement& e);
 
-      virtual ~DetectorSurfaces();
-
       /// get the list of surfaces added to this DetElement
       const SurfaceList& surfaceList() { return *_sL ; }
 
     protected :
-      SurfaceList* _sL ;
+      SurfaceList* _sL { nullptr };
 
       /// initializes surfaces from VolSurfaces assigned to this DetElement in detector construction
-      void initialize() ;
+      void initialize(const DetElement& det) ;
 
     };
 

DDRec/include/DDRec/Surface.h

@@ -32,7 +32,11 @@ namespace dd4hep {
 
       SurfaceHelper(const DetElement& e);
 
-      ~SurfaceHelper();
+      ~SurfaceHelper() = default;
+      SurfaceHelper(const SurfaceHelper&) = delete;
+      SurfaceHelper& operator=(const SurfaceHelper&) = delete;
+      SurfaceHelper(SurfaceHelper&&) = default;
+      SurfaceHelper& operator=(SurfaceHelper&&) = delete;
 
       /** Get the list of all surfaces added to this DetElement and all its daughters -
        *  instantiate SurfaceHelper with description.world() to get all surfaces.

DDRec/include/DDRec/SurfaceHelper.h

@@ -46,26 +46,25 @@ namespace dd4hep {
 #else
       SurfaceManager() = delete ;
 #endif
-      /// No copy constructor
-      SurfaceManager(const SurfaceManager& copy) = delete;
 
-      /// Default destructor
-      ~SurfaceManager();
+      ~SurfaceManager() = default;
+      SurfaceManager(const SurfaceManager&) = delete;
+      SurfaceManager& operator=(const SurfaceManager&) = delete;
+      SurfaceManager(SurfaceManager&&) = default;
+      SurfaceManager& operator=(SurfaceManager&&) = default;
+
 
-      /// No assignment operator
-      SurfaceManager& operator=(const SurfaceManager& copy) = delete;
-      
       /** Get the maps of all surfaces associated to the given detector or
        *  type of detectors, e.g. map("tracker") returns a map with all surfaces
        *  assigned to tracking detectors. Returns 0 if no map exists.
        */
-      const SurfaceMap* map( const std::string name ) const ;
+      const SurfaceMap* map( const std::string& name ) const ;
 
 
       ///create a string with all available maps and their size (number of surfaces)
       std::string toString() const ;
 
-    protected :
+    private :
 
 
       /// initialize all known surface maps

DDRec/include/DDRec/SurfaceManager.h

@@ -16,69 +16,52 @@ namespace dd4hep {
   namespace rec {
 
 
+    DetectorSurfaces::DetectorSurfaces(dd4hep::DetElement const& e) {
 
-    DetectorSurfaces::DetectorSurfaces(dd4hep::DetElement const& e) : DetElement(e) , _sL( 0 ) {
-
-      initialize() ;
-    }
-    
-    DetectorSurfaces::~DetectorSurfaces(){
-      // nothing to do: SurfaceList is added as extension
-      // and is deleted automatically
+      initialize(e) ;
     }
 
-    
-    void DetectorSurfaces::initialize() {
-      
-      DetElement det = *this ;
+    void DetectorSurfaces::initialize(dd4hep::DetElement const& det) {
 
       const VolSurfaceList* vsL = volSurfaceList(det) ;
 
-      try {
-        _sL = det.extension< SurfaceList >(false) ;
-        if (not _sL) {
-          _sL = det.addExtension<SurfaceList >(  new SurfaceList( true )  ) ;
-        }
-      } catch(const std::exception& e) { 
-        _sL = det.addExtension<SurfaceList >(  new SurfaceList( true )  ) ; 
+      _sL = det.extension< SurfaceList >(false) ;
+      if (! _sL) {
+        _sL = det.addExtension<SurfaceList >(  new SurfaceList( true )  ) ;
       }
 
-      if( ! vsL->empty() && _sL->empty() ) {  // only fill surfaces for this DetElement once
-	
-	// std::cout <<  "     detector  " << det.name() << " id: " << det.id() << " has " <<  vsL->size() << " surfaces "  << std::endl ;
-	
-	// std::cout << " ------------------------- " 
-	// 	  << " 	DetectorSurfaces::initialize()  adding surfaces : " 
-	// 	  << std::endl ;
-	
-	for( VolSurfaceList::const_iterator it = vsL->begin() ; it != vsL->end() ; ++it ){
-	  
-	  VolSurface volSurf =  *it ;
-	  
-	  Surface* surf = 0 ;
-	  
-	  if( volSurf.type().isCylinder() )
-	    surf = new CylinderSurface(  det,  volSurf ) ;
-	  
-	  else if( volSurf.type().isCone() ) 
-	    surf = new ConeSurface( det, volSurf ) ;
-	  
-	  else
-	    surf = new Surface(  det,  volSurf ) ;
+      if( ! _sL->empty() ) {  // only fill surfaces for this DetElement once
+        return ;
+      }
 
-	  // std::cout << " ------------------------- " 
-	  //   	    << " surface: "   << *surf        << std::endl
-	  //   	    << " ------------------------- "  << std::endl ;
-	  
-	  _sL->push_back( surf ) ;
-	  
-	}
+      // std::cout <<  "     detector  " << det.name() << " id: " << det.id() << " has " <<  vsL->size() << " surfaces "  << std::endl ;
 
+      // std::cout << " ------------------------- "
+      // 	  << " 	DetectorSurfaces::initialize()  adding surfaces : "
+      // 	  << std::endl ;
+
+      for( const auto& volSurf : *vsL ) {
+
+        Surface* surf = nullptr ;
+
+        if( volSurf.type().isCylinder() )
+          surf = new CylinderSurface(  det,  volSurf ) ;
+
+        else if( volSurf.type().isCone() )
+          surf = new ConeSurface( det, volSurf ) ;
+
+        else
+          surf = new Surface(  det,  volSurf ) ;
+
+        // std::cout << " ------------------------- "
+        //   	    << " surface: "   << *surf        << std::endl
+        //   	    << " ------------------------- "  << std::endl ;
+
+        _sL->push_back( surf ) ;
 
       }
 
     }
-    
 
 
   } // namespace

DDRec/src/DetectorSurfaces.cpp

@@ -18,7 +18,6 @@
 
 #include <cmath>
 #include <memory>
-#include <exception>
 
 #include "TGeoMatrix.h"
 #include "TGeoShape.h"
@@ -70,8 +69,8 @@ namespace dd4hep {
       return g ;
     }
 
-    const IMaterial&  VolSurfaceBase::innerMaterial() const{  return  _innerMat ;  }
-    const IMaterial&  VolSurfaceBase::outerMaterial() const { return  _outerMat  ; }
+    const IMaterial&  VolSurfaceBase::innerMaterial() const { return  _innerMat ; }
+    const IMaterial&  VolSurfaceBase::outerMaterial() const { return  _outerMat ; }
     double VolSurfaceBase::innerThickness() const { return _th_i ; }
     double VolSurfaceBase::outerThickness() const { return _th_o ; }
 
@@ -507,15 +506,6 @@ namespace dd4hep {
 
     //================================================================================================================
 
-
-    VolSurfaceList::~VolSurfaceList(){
-      // // delete all surfaces attached to this volume
-      // for( VolSurfaceList::iterator i=begin(), n=end() ; i !=n ; ++i ) {
-      //   i->clear() ;
-      // }
-    }
-    //=======================================================
-
     SurfaceList::~SurfaceList(){
       if( _isOwner ) {
         // delete all surfaces attached to this volume
@@ -525,7 +515,7 @@ namespace dd4hep {
 
     //================================================================================================================
 
-    VolSurfaceList* volSurfaceList( DetElement& det ) {
+    VolSurfaceList* volSurfaceList( const DetElement& det ) {
       VolSurfaceList* list = det.extension< VolSurfaceList >(false);
       if ( !list )  {
         list = det.addExtension<VolSurfaceList >(new VolSurfaceList);
@@ -627,7 +617,7 @@ namespace dd4hep {
 
       const IMaterial& mat = _volSurf.innerMaterial() ;
 
-      if( ! ( mat.Z() > 0 ) ) {
+      if( mat.Z() <= 0 ) {
 
         MaterialManager matMgr( _det.placement().volume() )  ;
 
@@ -646,7 +636,7 @@ namespace dd4hep {
 
       const IMaterial& mat = _volSurf.outerMaterial() ;
 
-      if( ! ( mat.Z() > 0 ) ) {
+      if( mat.Z() <= 0 ) {
 
         MaterialManager matMgr( _det.placement().volume() ) ;
 
@@ -750,7 +740,7 @@ namespace dd4hep {
 
       //---- if the volSurface is not in the DetElement's volume, we need to mutliply the path to the volume to the
       // DetElements world transform
-      for( std::list<PlacedVolume>::iterator it = ++( pVList.begin() ) , n = pVList.end() ; it != n ; ++it ){
+      for( auto it = std::next(pVList.begin()) ; it != pVList.end() ; ++it ) {
 
       	PlacedVolume pvol = *it ;
       	TGeoMatrix* m = pvol->GetMatrix();

DDRec/src/Surface.cpp

@@ -29,11 +29,6 @@ namespace dd4hep {
       initialize() ;
     }
 
-    SurfaceHelper::~SurfaceHelper(){
-      // nothing to do
-    }
-    
-    
     void SurfaceHelper::initialize() {
 
       // have to populate the volume manager once in order to have 
@@ -50,11 +45,9 @@ namespace dd4hep {
 
       while( ! daugs.empty() ) {
 
-        for( std::list< DetElement >::iterator li=daugs.begin() ; li != daugs.end() ; ++li ){
-          DetElement dau = *li ;
-          DetElement::Children chMap = dau.children() ;
-          for ( DetElement::Children::const_iterator it=chMap.begin() ; it != chMap.end() ; ++it ){
-            DetElement de = (*it).second ;
+        for( const auto& dag : daugs ){
+          const DetElement::Children& chMap = dag.children() ;
+          for ( const auto& [_, de] : chMap ){
             gdaugs.push_back( de ) ;
           }  
         }
@@ -65,12 +58,8 @@ namespace dd4hep {
 
       //      std::cout << " **** SurfaceHelper::initialize() : # DetElements found  " << dets.size() << std::endl ;
 
-      for( std::list< DetElement >::iterator li=dets.begin() ; li != dets.end() ; ++li ) {
+      for( const auto& det : dets) {
 
-        DetElement det = (*li) ;
-	
-
-
         // create surfaces
         DetectorSurfaces ds( det ) ;
 

DDRec/src/SurfaceHelper.cpp

@@ -19,11 +19,7 @@
 #include <sstream>
 
 namespace dd4hep {
-  
-  using namespace detail ;
-
   namespace rec {
-    
 
     SurfaceManager::SurfaceManager(const Detector& theDetector){
 
@@ -35,12 +31,8 @@ namespace dd4hep {
       initialize(theDetector) ;
     }
 
-    SurfaceManager::~SurfaceManager(){
-      // nothing to do
-    }
-    
 
-    const SurfaceMap* SurfaceManager::map( const std::string name ) const {
+    const SurfaceMap* SurfaceManager::map( const std::string& name ) const {
 
       SurfaceMapsMap::const_iterator it = _map.find( name ) ;
 
@@ -49,36 +41,32 @@ namespace dd4hep {
         return & it->second ;
       }
 
-      return 0 ;
+      return nullptr ;
     }
 
     void SurfaceManager::initialize(const Detector& description) {
 
-      const std::vector<std::string>& types = description.detectorTypes() ;
-
-      for(unsigned i=0,N=types.size();i<N;++i){
+      for(const auto& type : description.detectorTypes()) {
 
-        const std::vector<DetElement>& dets = description.detectors( types[i] ) ;  
+        const std::vector<DetElement>& dets = description.detectors( type ) ;
 
-        for(unsigned j=0,M=dets.size();j<M;++j){
+        for(const auto& det : dets) {
 
-          std::string name = dets[j].name() ;
+          const std::string& name = det.name() ;
 
-          SurfaceHelper surfH( dets[j] ) ;
+          SurfaceHelper surfH( det ) ;
 
           const SurfaceList& detSL = surfH.surfaceList() ;
 
           // add an empty map for this detector in case there are no surfaces attached 
           _map.emplace(name , SurfaceMap());
 
-          for( SurfaceList::const_iterator it = detSL.begin() ; it != detSL.end() ; ++it ){
-            ISurface* surf =  *it ;
-	    
+          for( auto* surf : detSL ) {
             // enter surface into map for this detector
             _map[ name ].emplace(surf->id(), surf );
 
             // enter surface into map for detector type
-            _map[ types[i] ].emplace(surf->id(), surf );
+            _map[ type ].emplace(surf->id(), surf );
 
             // enter surface into world map 
             _map[ "world" ].emplace(surf->id(), surf );