Use smart pointers instead of raw pointers of StructureFunction

Make memory management easier. Thanks to @tackandr for the suggestion.
metno · Mar 14, 2024 · 117809a · 117809a
1 parent 3852034
commit 117809a
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 31 deletions.
diff --git a/include/gridpp.h b/include/gridpp.h
@@ -1890,6 +1890,7 @@ namespace gridpp {
             not_implemented_exception();
     };
 
+    typedef std::shared_ptr<gridpp::StructureFunction> StructureFunctionPtr;
     /** Covariance structure function */
     class StructureFunction {
         public:
@@ -1914,7 +1915,7 @@ namespace gridpp {
               * @returns Distance [m]
             */
             virtual float localization_distance(const Point& p) const;
-            virtual StructureFunction* clone() const = 0;
+            virtual StructureFunctionPtr clone() const = 0;
             static const float default_min_rho;
         protected:
             /** Barnes correlation function
@@ -1940,15 +1941,14 @@ namespace gridpp {
               * @param structure_w: Land/sea structure function
             */
             MultipleStructure(const StructureFunction& structure_h, const StructureFunction& structure_v, const StructureFunction& structure_w);
-            ~MultipleStructure();
             float corr(const Point& p1, const Point& p2) const;
             vec corr(const Point& p1, const std::vector<Point>& p2) const;
-            StructureFunction* clone() const;
+            StructureFunctionPtr clone() const;
             float localization_distance(const Point& p) const;
         private:
-            StructureFunction* m_structure_h;
-            StructureFunction* m_structure_v;
-            StructureFunction* m_structure_w;
+            StructureFunctionPtr m_structure_h;
+            StructureFunctionPtr m_structure_v;
+            StructureFunctionPtr m_structure_w;
     };
     /** Simple structure function based on distance, elevation, and land area fraction */
     class BarnesStructure: public StructureFunction {
@@ -1971,7 +1971,7 @@ namespace gridpp {
             BarnesStructure(Grid grid, vec2 h, vec2 v, vec2 w, float min_rho=StructureFunction::default_min_rho);
             float corr(const Point& p1, const Point& p2) const;
             vec corr(const Point& p1, const std::vector<Point>& p2) const;
-            StructureFunction* clone() const;
+            StructureFunctionPtr clone() const;
             float localization_distance(const Point& p) const;
         private:
             float localization_distance(float h) const;
@@ -1988,7 +1988,7 @@ namespace gridpp {
         public:
             CressmanStructure(float h, float v=0, float w=0);
             float corr(const Point& p1, const Point& p2) const;
-            StructureFunction* clone() const;
+            StructureFunctionPtr clone() const;
         private:
             float mH;
             float mV;
@@ -2002,14 +2002,13 @@ namespace gridpp {
               * @param dist: Force background-to-obs correlation to 0 for points within this distance [m]
             */
             CrossValidation(StructureFunction& structure, float dist);
-            ~CrossValidation();
             float corr(const Point& p1, const Point& p2) const;
             float corr_background(const Point& p1, const Point& p2) const;
             vec corr_background(const Point& p1, const std::vector<Point>& p2) const;
-            StructureFunction* clone() const;
+            StructureFunctionPtr clone() const;
             float localization_distance(const Point& p) const;
         private:
-            StructureFunction* m_structure;
+            StructureFunctionPtr m_structure;
             float m_dist;
     };
 

diff --git a/src/api/structure.cpp b/src/api/structure.cpp
@@ -91,14 +91,8 @@ vec gridpp::MultipleStructure::corr(const Point& p1, const std::vector<Point>& p
     return corr_total;
 
 }
-gridpp::StructureFunction* gridpp::MultipleStructure::clone() const {
-    gridpp::StructureFunction* val = new gridpp::MultipleStructure(*m_structure_h, *m_structure_v, *m_structure_w);
-    return val;
-}
-gridpp::MultipleStructure::~MultipleStructure() {
-    delete m_structure_h;
-    delete m_structure_v;
-    delete m_structure_w;
+gridpp::StructureFunctionPtr gridpp::MultipleStructure::clone() const {
+    return std::make_shared<gridpp::MultipleStructure>(*m_structure_h, *m_structure_v, *m_structure_w);
 }
 
 /** Barnes */
@@ -227,9 +221,8 @@ vec gridpp::BarnesStructure::corr(const Point& p1, const std::vector<Point>& p2)
     }
     return output;
 }
-gridpp::StructureFunction* gridpp::BarnesStructure::clone() const {
-    gridpp::StructureFunction* val = new gridpp::BarnesStructure(m_grid, mH, mV, mW, m_min_rho);
-    return val;
+gridpp::StructureFunctionPtr gridpp::BarnesStructure::clone() const {
+    return std::make_shared<gridpp::BarnesStructure>(m_grid, mH, mV, mW, m_min_rho);
 }
 float gridpp::BarnesStructure::localization_distance(const Point& p) const {
     if(m_is_spatial) {
@@ -268,9 +261,8 @@ float gridpp::CressmanStructure::corr(const Point& p1, const Point& p2) const {
     }
     return rho;
 }
-gridpp::StructureFunction* gridpp::CressmanStructure::clone() const {
-    gridpp::StructureFunction* val = new gridpp::CressmanStructure(mH, mV, mW);
-    return val;
+gridpp::StructureFunctionPtr gridpp::CressmanStructure::clone() const {
+    return std::make_shared<gridpp::CressmanStructure>(mH, mV, mW);
 }
 
 /** CrossValidation */
@@ -303,13 +295,9 @@ vec gridpp::CrossValidation::corr_background(const Point& p1, const std::vector<
     }
     return curr_corr;
 }
-gridpp::StructureFunction* gridpp::CrossValidation::clone() const {
-    gridpp::StructureFunction* val = new gridpp::CrossValidation(*m_structure, m_dist);
-    return val;
+gridpp::StructureFunctionPtr gridpp::CrossValidation::clone() const {
+    return std::make_shared<gridpp::CrossValidation>(*m_structure, m_dist);
 }
 float gridpp::CrossValidation::localization_distance(const Point& p) const {
     return m_structure->localization_distance(p);
 }
-gridpp::CrossValidation::~CrossValidation() {
-    delete m_structure;
-}
diff --git a/swig/gridpp.i b/swig/gridpp.i
@@ -38,6 +38,17 @@
         SWIG_exception(SWIG_RuntimeError, "Unknown exception");
     }
 }
+/* This is needed to make sure that functions returning smart pointers
+ * can be accessed properly in python. E.g. calling .corr on a structure function
+ * smart pointer
+*/
+%include <std_shared_ptr.i>
+%shared_ptr(gridpp::StructureFunction)
+%shared_ptr(gridpp::MultipleStructure)
+%shared_ptr(gridpp::BarnesStructure)
+%shared_ptr(gridpp::CressmanStructure)
+%shared_ptr(gridpp::CrossValidation)
+
 %include "vector.i"
 %apply std::vector<std::vector<float> >& OUTPUT { std::vector<std::vector<float> >& output };
 %apply std::vector<std::vector<int> >& OUTPUT { std::vector<std::vector<int> >& count };