From 6eafacb50757d446e7f3b3ac03a91437271ed137 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Traizet?= <cedric.traizet@c-s.fr>
Date: Fri, 16 Oct 2020 16:48:02 +0200
Subject: [PATCH] PERF: use hidden friend idiom pattern when overloading
 operator== (code review)

---
 .../Metadata/include/otbGeometryMetadata.h    | 22 +++++++++++--
 .../Core/Metadata/include/otbMetaDataKey.h    | 32 ++++++++++++-------
 .../Core/Metadata/src/otbGeometryMetadata.cxx | 18 -----------
 Modules/Core/Metadata/src/otbMetaDataKey.cxx  | 18 -----------
 Modules/IO/TestKernel/src/otbTestHelper.cxx   |  1 -
 5 files changed, 40 insertions(+), 51 deletions(-)

diff --git a/Modules/Core/Metadata/include/otbGeometryMetadata.h b/Modules/Core/Metadata/include/otbGeometryMetadata.h
index c370625026..802ae6c3e2 100644
--- a/Modules/Core/Metadata/include/otbGeometryMetadata.h
+++ b/Modules/Core/Metadata/include/otbGeometryMetadata.h
@@ -161,12 +161,28 @@ struct OTBMetadata_EXPORT RPCParam
     oss << "]";
     return oss.str();
   };
+  
+  // Equality comparison operator (hidden friend idiom)
+  friend bool operator==(const RPCParam & lhs, const RPCParam & rhs)
+  {
+    return lhs.LineOffset == rhs.LineOffset
+        && lhs.SampleOffset == rhs.SampleOffset
+        && lhs.LatOffset == rhs.LatOffset
+        && lhs.LonOffset == rhs.LonOffset
+        && lhs.HeightOffset == rhs.HeightOffset
+        && lhs.LineScale == rhs.LineScale
+        && lhs.SampleScale == rhs.SampleScale
+        && lhs.LatScale == rhs.LatScale
+        && lhs.LonScale == rhs.LonScale
+        && lhs.HeightScale == rhs.HeightScale
+        && std::equal(std::begin(lhs.LineNum), std::end(lhs.LineNum), std::begin(rhs.LineNum))
+        && std::equal(std::begin(lhs.LineDen), std::end(lhs.LineDen), std::begin(rhs.LineDen))
+        && std::equal(std::begin(lhs.SampleNum), std::end(lhs.SampleNum), std::begin(rhs.SampleNum))
+        && std::equal(std::begin(lhs.SampleDen), std::end(lhs.SampleDen), std::begin(rhs.SampleDen));
+  }
 
 };
 
-OTBMetadata_EXPORT bool operator==(const RPCParam & lhs, const RPCParam & rhs);
-
-
 } // end namespace Projection
 
 } // end namespace otb
diff --git a/Modules/Core/Metadata/include/otbMetaDataKey.h b/Modules/Core/Metadata/include/otbMetaDataKey.h
index df3ee36f57..1dfbe1e7cf 100644
--- a/Modules/Core/Metadata/include/otbMetaDataKey.h
+++ b/Modules/Core/Metadata/include/otbMetaDataKey.h
@@ -238,9 +238,14 @@ struct OTBMetadata_EXPORT Time : tm
 
   friend OTBMetadata_EXPORT std::istream& operator>>(std::istream& is, Time& val);
 
+  friend OTBMetadata_EXPORT bool operator==(const Time & lhs, const Time & rhs)
+  {
+    tm tmLhs = lhs;
+    tm tmRhs = rhs;
+    return mktime(&tmLhs) + lhs.frac_sec == mktime(&tmRhs) + rhs.frac_sec;
+  }
 };
 
-OTBMetadata_EXPORT bool operator==(const Time & lhs, const Time & rhs);
 
 struct LUTAxis
 {
@@ -254,10 +259,15 @@ struct LUTAxis
   std::vector<double> Values;
   /** Export to JSON */
   std::string ToJSON(bool multiline=false) const;
-};
-
-OTBMetadata_EXPORT bool operator==(const LUTAxis & lhs, const LUTAxis & rhs);
 
+  friend bool operator==(const LUTAxis & lhs, const LUTAxis & rhs)
+  {
+    return lhs.Size == rhs.Size
+        && lhs.Origin == rhs.Origin
+        && lhs.Spacing == rhs.Spacing
+        && lhs.Values == rhs.Values;
+  }
+};
 
 template <unsigned int VDim> class LUT
 {
@@ -271,14 +281,14 @@ public:
   std::string OTBMetadata_EXPORT ToString() const;
 
   void OTBMetadata_EXPORT FromString(std::string);
-};
 
-template <unsigned int VDim>
-bool operator==(const LUT<VDim> & lhs, const LUT<VDim> & rhs)
-{
-  return std::equal(std::begin(lhs.Array), std::end(lhs.Array), std::begin(rhs.Array) ) 
-          && lhs.Array == rhs.Array;
-}
+  friend bool operator==(const LUT<VDim> & lhs, const LUT<VDim> & rhs)
+  {
+    return std::equal(std::begin(lhs.Array), std::end(lhs.Array), std::begin(rhs.Array) ) 
+            && lhs.Array == rhs.Array;
+  }
+
+};
 
 
 template <unsigned int VDim>
diff --git a/Modules/Core/Metadata/src/otbGeometryMetadata.cxx b/Modules/Core/Metadata/src/otbGeometryMetadata.cxx
index fea2819667..0dadc355c4 100644
--- a/Modules/Core/Metadata/src/otbGeometryMetadata.cxx
+++ b/Modules/Core/Metadata/src/otbGeometryMetadata.cxx
@@ -107,23 +107,5 @@ std::string RPCParam::ToJSON(bool multiline) const
   return oss.str();
 }
 
-bool operator==(const RPCParam & lhs, const RPCParam & rhs)
-{
-  return lhs.LineOffset == rhs.LineOffset
-      && lhs.SampleOffset == rhs.SampleOffset
-      && lhs.LatOffset == rhs.LatOffset
-      && lhs.LonOffset == rhs.LonOffset
-      && lhs.HeightOffset == rhs.HeightOffset
-      && lhs.LineScale == rhs.LineScale
-      && lhs.SampleScale == rhs.SampleScale
-      && lhs.LatScale == rhs.LatScale
-      && lhs.LonScale == rhs.LonScale
-      && lhs.HeightScale == rhs.HeightScale
-      && std::equal(lhs.LineNum, lhs.LineNum+20, rhs.LineNum )
-      && std::equal(lhs.LineDen, lhs.LineDen+20, rhs.LineDen )
-      && std::equal(lhs.SampleNum, lhs.SampleNum+20, rhs.SampleNum )
-      && std::equal(lhs.SampleDen, lhs.SampleDen+20, rhs.SampleDen );
-}
-
 } // end namespace Projection
 } // end namespace otb
diff --git a/Modules/Core/Metadata/src/otbMetaDataKey.cxx b/Modules/Core/Metadata/src/otbMetaDataKey.cxx
index 9808d0fec7..e55a94384e 100644
--- a/Modules/Core/Metadata/src/otbMetaDataKey.cxx
+++ b/Modules/Core/Metadata/src/otbMetaDataKey.cxx
@@ -188,15 +188,6 @@ std::istream& operator>>(std::istream& is, Time& val)
 #undef _OTB_ISTREAM_EXPECT
 
 
-
-bool operator==(const Time & lhs, const Time & rhs)
-{
-  tm tmLhs = lhs;
-  tm tmRhs = rhs;
-  return mktime(&tmLhs) + lhs.frac_sec == mktime(&tmRhs) + rhs.frac_sec;
-}
-
-
 std::string LUTAxis::ToJSON(bool multiline) const
 {
   std::ostringstream oss;
@@ -404,15 +395,6 @@ MDGeomBmType MDGeomNames = bimapGenerator<MDGeom>(std::map<MDGeom, std::string>
   {MDGeom::Adjustment,     "Adjustment"}
 });
 
-
-OTBMetadata_EXPORT bool operator==(const LUTAxis & lhs, const LUTAxis & rhs)
-{
-  return lhs.Size == rhs.Size
-      && lhs.Origin == rhs.Origin
-      && lhs.Spacing == rhs.Spacing
-      && lhs.Values == rhs.Values;
-}
-
 template<>
 std::string EnumToString(MDGeom value)
 {
diff --git a/Modules/IO/TestKernel/src/otbTestHelper.cxx b/Modules/IO/TestKernel/src/otbTestHelper.cxx
index 8bd7eb58f3..fc00fe17d6 100644
--- a/Modules/IO/TestKernel/src/otbTestHelper.cxx
+++ b/Modules/IO/TestKernel/src/otbTestHelper.cxx
@@ -1456,7 +1456,6 @@ int CompareMetadataDict( const MapType & baselineMap,
   while (first1 != last1)
   {
     if (std::find(untestedKeys.begin(), untestedKeys.end(), first1->first) == untestedKeys.end())
-    //if (first1->first != untestedKeys)
     {
       if (first1->first != first2->first)
       {
-- 
GitLab