Encapsulate std::cerr under a single point of control (#656)

* Encapsulate std::cerr under a single point of control When integrating this library into a larger application, downstream code doesn't always want to spew onto cerr. To allow customization, we'll refactor to encapsulate the destination inside of a function. Future work can extent this with a mechanism to redirect it (e.g., to send it to spdlog instead of cerr). --------- Signed-off-by: Jeremy Nimmer <[email protected]>
gazebosim · Jan 6, 2025 · ebd0060 · ebd0060
1 parent 76e7db2
commit ebd0060
Show file tree

Hide file tree

Showing 11 changed files with 209 additions and 82 deletions.
diff --git a/BUILD.bazel b/BUILD.bazel
@@ -170,6 +170,7 @@ cc_library(
     includes = ["include"],
     deps = [
         ":Config",
+        ":Error",
         ":Helpers",
         ":Vector3",
     ],
@@ -217,6 +218,7 @@ cc_library(
     deps = [
         ":Angle",
         ":Config",
+        ":Error",
         ":Helpers",
         ":Vector3",
         "@gz-utils//:ImplPtr",
@@ -303,6 +305,17 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "Error",
+    srcs = ["src/Error.cc"],
+    hdrs = ["include/gz/math/detail/Error.hh"],
+    includes = ["include"],
+    deps = [
+        ":Config",
+        ":Export",
+    ],
+)
+
 cc_test(
     name = "Ellipsoid_TEST",
     srcs = ["src/Ellipsoid_TEST.cc"],
@@ -398,6 +411,7 @@ cc_library(
     includes = ["include"],
     deps = [
         ":Config",
+        ":Error",
         ":Helpers",
         "@gz-utils//:NeverDestroyed",
     ],
@@ -472,6 +486,7 @@ cc_library(
     includes = ["include"],
     deps = [
         ":Config",
+        ":Error",
         ":Graph",
         ":Helpers",
     ],
@@ -577,6 +592,7 @@ cc_library(
     includes = ["include"],
     deps = [
         ":Config",
+        ":Error",
         ":Helpers",
         ":Rand",
         ":Vector3",
@@ -869,6 +885,7 @@ cc_library(
     includes = ["include"],
     deps = [
         ":Config",
+        ":Error",
         ":Region3",
         ":Vector3",
     ],
@@ -1103,6 +1120,7 @@ cc_library(
     includes = ["include"],
     deps = [
         ":Config",
+        ":Error",
         ":Helpers",
         "@gz-utils//:ImplPtr",
     ],
@@ -1176,6 +1194,7 @@ cc_library(
         ":Angle",
         ":Config",
         ":CoordinateVector3",
+        ":Error",
         ":Helpers",
         ":Matrix3",
         ":Vector3",

diff --git a/include/gz/math/PiecewiseScalarField3.hh b/include/gz/math/PiecewiseScalarField3.hh
@@ -18,14 +18,15 @@
 #define GZ_MATH_PIECEWISE_SCALAR_FIELD3_HH_
 
 #include <algorithm>
-#include <iostream>
 #include <limits>
+#include <sstream>
 #include <utility>
 #include <vector>
 
 #include <gz/math/Region3.hh>
 #include <gz/math/Vector3.hh>
 #include <gz/math/config.hh>
+#include <gz/math/detail/Error.hh>
 
 namespace gz::math
 {
@@ -77,21 +78,23 @@ namespace gz::math
       {
         if (pieces[i].region.Empty())
         {
-          std::cerr << "Region #" << i << " (" << pieces[i].region
-                    << ") in piecewise scalar field definition is empty."
-                    << std::endl;
+          std::ostringstream errStream;
+          errStream << "Region #" << i << " (" << pieces[i].region
+                    << ") in piecewise scalar field definition is empty.";
+          detail::LogErrorMessage(errStream.str());
         }
         for (size_t j = i + 1; j < pieces.size(); ++j)
         {
           if (pieces[i].region.Intersects(pieces[j].region))
           {
-            std::cerr << "Detected overlap between regions in "
+            std::ostringstream errStream;
+            errStream << "Detected overlap between regions in "
                       << "piecewise scalar field definition: "
                       << "region #" << i << " (" << pieces[i].region
                       << ") overlaps with region #" << j << " ("
                       << pieces[j].region << "). Region #" << i
-                      << " will take precedence when overlapping."
-                      << std::endl;
+                      << " will take precedence when overlapping.";
+            detail::LogErrorMessage(errStream.str());
           }
         }
       }

diff --git a/include/gz/math/detail/Error.hh b/include/gz/math/detail/Error.hh
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2024 Open Source Robotics Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+*/
+#ifndef GZ_MATH_DETAIL_ERROR_HH_
+#define GZ_MATH_DETAIL_ERROR_HH_
+
+#include <string>
+
+#include <gz/math/Export.hh>
+#include <gz/math/config.hh>
+
+namespace gz::math
+{
+  // Inline bracket to help doxygen filtering.
+  inline namespace GZ_MATH_VERSION_NAMESPACE {
+  namespace detail {
+    /// Prints the given error message to std::cerr, followed by a newline.
+    /// (In the future, we might provide a function to change the destination.)
+    void GZ_MATH_VISIBLE LogErrorMessage(const std::string& message);
+  }  // namespace detail
+  }  // namespace GZ_MATH_VERSION_NAMESPACE
+}  // namespace gz::math
+#endif  // GZ_MATH_DETAIL_ERROR_HH_
diff --git a/include/gz/math/graph/Graph.hh b/include/gz/math/graph/Graph.hh
@@ -18,14 +18,16 @@
 #define GZ_MATH_GRAPH_GRAPH_HH_
 
 #include <cassert>
-#include <iostream>
 #include <map>
+#include <ostream>
 #include <set>
+#include <sstream>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include <gz/math/config.hh>
+#include "gz/math/detail/Error.hh"
 #include "gz/math/graph/Edge.hh"
 #include "gz/math/graph/Vertex.hh"
 
@@ -113,16 +115,17 @@ namespace graph
       {
         if (!this->AddVertex(v.Name(), v.Data(), v.Id()).Valid())
         {
-          std::cerr << "Invalid vertex with Id [" << v.Id() << "]. Ignoring."
-                    << std::endl;
+          std::ostringstream errStream;
+          errStream << "Invalid vertex with Id [" << v.Id() << "]. Ignoring.";
+          detail::LogErrorMessage(errStream.str());
         }
       }
 
       // Add all edges.
       for (auto const &e : _edges)
       {
         if (!this->AddEdge(e.vertices, e.data, e.weight).Valid())
-          std::cerr << "Ignoring edge" << std::endl;
+          detail::LogErrorMessage("Ignoring edge");
       }
     }
 
@@ -146,8 +149,9 @@ namespace graph
         // No space for new Ids.
         if (id == kNullId)
         {
-          std::cerr << "[Graph::AddVertex()] The limit of vertices has been "
-                    << "reached. Ignoring vertex." << std::endl;
+          detail::LogErrorMessage(
+              "[Graph::AddVertex()] The limit of vertices has been reached. "
+              "Ignoring vertex.");
           return NullVertex<V>();
         }
       }
@@ -159,8 +163,9 @@ namespace graph
       // The Id already exists.
       if (!ret.second)
       {
-        std::cerr << "[Graph::AddVertex()] Repeated vertex [" << id << "]"
-                  << std::endl;
+        std::ostringstream errStream;
+        errStream << "[Graph::AddVertex()] Repeated vertex [" << id << "]";
+        detail::LogErrorMessage(errStream.str());
         return NullVertex<V>();
       }
 
@@ -215,8 +220,9 @@ namespace graph
       // No space for new Ids.
       if (id == kNullId)
       {
-        std::cerr << "[Graph::AddEdge()] The limit of edges has been reached. "
-                  << "Ignoring edge." << std::endl;
+        detail::LogErrorMessage(
+            "[Graph::AddEdge()] The limit of edges has been reached. "
+            "Ignoring edge.");
         return NullEdge<E, EdgeType>();
       }
 

diff --git a/include/gz/math/graph/GraphAlgorithms.hh b/include/gz/math/graph/GraphAlgorithms.hh
@@ -26,6 +26,7 @@
 #include <vector>
 
 #include <gz/math/config.hh>
+#include "gz/math/detail/Error.hh"
 #include "gz/math/graph/Graph.hh"
 #include "gz/math/Helpers.hh"
 
@@ -217,15 +218,19 @@ namespace graph
     // Sanity check: The source vertex should exist.
     if (allVertices.find(_from) == allVertices.end())
     {
-      std::cerr << "Vertex [" << _from << "] Not found" << std::endl;
+      std::ostringstream errStream;
+      errStream << "Vertex [" << _from << "] Not found";
+      detail::LogErrorMessage(errStream.str());
       return {};
     }
 
     // Sanity check: The destination vertex should exist (if used).
     if (_to != kNullId &&
         allVertices.find(_to) == allVertices.end())
     {
-      std::cerr << "Vertex [" << _from << "] Not found" << std::endl;
+      std::ostringstream errStream;
+      errStream << "Vertex [" << _from << "] Not found";
+      detail::LogErrorMessage(errStream.str());
       return {};
     }
 

diff --git a/src/Color.cc b/src/Color.cc
@@ -16,9 +16,9 @@
  */
 #include <cmath>
 #include <algorithm>
-#include <iostream>
 
 #include "gz/math/Color.hh"
+#include "gz/math/detail/Error.hh"
 
 using namespace gz;
 using namespace math;
@@ -574,6 +574,6 @@ void Color::Clamp()
   if (clamped)
   {
     // TODO(azeey) Use spdlog when we have it's available.
-    std::cerr << "Color values were clamped\n";
+    detail::LogErrorMessage("Color values were clamped");
   }
 }
diff --git a/src/CoordinateVector3.cc b/src/CoordinateVector3.cc
@@ -19,10 +19,10 @@
 #include "gz/math/CoordinateVector3.hh"
 #include "gz/math/Vector3.hh"
 #include "gz/math/Helpers.hh"
+#include "gz/math/detail/Error.hh"
 #include "gz/utils/ImplPtr.hh"
 
 #include <cmath>
-#include <iostream>
 #include <optional>
 #include <variant>
 
@@ -220,14 +220,16 @@ CoordinateVector3 CoordinateVector3::operator+(
   {
     if (this->IsMetric())
     {
-      std::cerr << "Spherical coordinates cannot be added to metric. "
-                   "Returning NaN." << std::endl;
+      detail::LogErrorMessage(
+          "Spherical coordinates cannot be added to metric. "
+          "Returning NaN.");
       return Metric(NAN_D, NAN_D, NAN_D);
     }
     else
     {
-      std::cerr << "Metric coordinates cannot be added to spherical. "
-                   "Returning NaN." << std::endl;
+      detail::LogErrorMessage(
+          "Metric coordinates cannot be added to spherical. "
+          "Returning NaN.");
       return Spherical(NAN_D, NAN_D, NAN_D);
     }
   }
@@ -258,14 +260,16 @@ const CoordinateVector3& CoordinateVector3::operator+=(
     if (this->IsMetric())
     {
       this->dataPtr->X() = this->dataPtr->Y() = NAN_D;
-      std::cerr << "Spherical coordinates cannot be added to metric. "
-                   "Setting the result to NaN." << std::endl;
+      detail::LogErrorMessage(
+          "Spherical coordinates cannot be added to metric. "
+          "Setting the result to NaN.");
     }
     else
     {
       this->dataPtr->Lat() = this->dataPtr->Lon() = NAN_D;
-      std::cerr << "Metric coordinates cannot be added to spherical. "
-                   "Setting the result to NaN." << std::endl;
+      detail::LogErrorMessage(
+          "Metric coordinates cannot be added to spherical. "
+          "Setting the result to NaN.");
     }
     return *this;
   }
@@ -308,14 +312,16 @@ CoordinateVector3 CoordinateVector3::operator-(
   {
     if (this->IsMetric())
     {
-      std::cerr << "Spherical coordinates cannot be subtracted from metric. "
-                   "Returning NaN." << std::endl;
+      detail::LogErrorMessage(
+          "Spherical coordinates cannot be subtracted from metric. "
+          "Returning NaN.");
       return Metric(NAN_D, NAN_D, NAN_D);
     }
     else
     {
-      std::cerr << "Metric coordinates cannot be subtracted from spherical. "
-                   "Returning NaN." << std::endl;
+      detail::LogErrorMessage(
+          "Metric coordinates cannot be subtracted from spherical. "
+          "Returning NaN.");
       return Spherical(NAN_D, NAN_D, NAN_D);
     }
   }
@@ -346,14 +352,16 @@ const CoordinateVector3& CoordinateVector3::operator-=(
     if (this->IsMetric())
     {
       this->dataPtr->X() = this->dataPtr->Y() = NAN_D;
-      std::cerr << "Spherical coordinates cannot be subtracted from metric. "
-                   "Setting the result to NaN." << std::endl;
+      detail::LogErrorMessage(
+          "Spherical coordinates cannot be subtracted from metric. "
+          "Setting the result to NaN.");
     }
     else
     {
       this->dataPtr->Lat() = this->dataPtr->Lon() = NAN_D;
-      std::cerr << "Metric coordinates cannot be subtracted from spherical. "
-                   "Setting the result to NaN." << std::endl;
+      detail::LogErrorMessage(
+          "Metric coordinates cannot be subtracted from spherical. "
+          "Setting the result to NaN.");
     }
     return *this;
   }