From 414846370e1e3d8d9dc3e69702af5ea23d632abc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Del=20Pino?= <stephane.delpino44@gmail.com>
Date: Thu, 30 Jun 2022 08:11:20 +0200
Subject: [PATCH] Change string conversion strategy

Do not use std::to_string anymore for arithmetic data. This is more
coherent with R^d and R^dxd conversions and aesthetically cleaner)
---
 .../node_processor/AffectationProcessor.hpp   | 89 +++++++------------
 tests/test_AffectationToStringProcessor.cpp   | 71 +++++----------
 tests/test_AffectationToTupleProcessor.cpp    | 68 +++++---------
 tests/test_ListAffectationProcessor.cpp       | 32 +++----
 4 files changed, 90 insertions(+), 170 deletions(-)

diff --git a/src/language/node_processor/AffectationProcessor.hpp b/src/language/node_processor/AffectationProcessor.hpp
index af8482a22..4fc2bc90b 100644
--- a/src/language/node_processor/AffectationProcessor.hpp
+++ b/src/language/node_processor/AffectationProcessor.hpp
@@ -96,27 +96,25 @@ class AffectationExecutor final : public IAffectationExecutor
   affect(ExecutionPolicy&, DataVariant&& rhs)
   {
     if constexpr (m_is_defined) {
+      auto stringify = [](auto&& value) {
+        std::ostringstream os;
+        os << std::boolalpha << value;
+        return os.str();
+      };
+
       if constexpr (not std::is_same_v<DataT, ZeroType>) {
         if constexpr (std::is_same_v<ValueT, std::string>) {
           if constexpr (std::is_same_v<OperatorT, language::eq_op>) {
             if constexpr (std::is_same_v<std::string, DataT>) {
               m_lhs = std::get<DataT>(rhs);
-            } else if constexpr (std::is_arithmetic_v<DataT> and not std::is_same_v<DataT, bool>) {
-              m_lhs = std::to_string(std::get<DataT>(rhs));
             } else {
-              std::ostringstream os;
-              os << std::boolalpha << std::get<DataT>(rhs);
-              m_lhs = os.str();
+              m_lhs = std::move(stringify(std::get<DataT>(rhs)));
             }
           } else {
             if constexpr (std::is_same_v<std::string, DataT>) {
               m_lhs += std::get<std::string>(rhs);
-            } else if constexpr (std::is_arithmetic_v<DataT> and not std::is_same_v<DataT, bool>) {
-              m_lhs += std::to_string(std::get<DataT>(rhs));
             } else {
-              std::ostringstream os;
-              os << std::boolalpha << std::get<DataT>(rhs);
-              m_lhs += os.str();
+              m_lhs += std::move(stringify(std::get<DataT>(rhs)));
             }
           }
         } else {
@@ -183,15 +181,8 @@ class AffectationExecutor final : public IAffectationExecutor
                 std::visit(
                   [&](auto&& v) {
                     if constexpr (is_std_vector_v<std::decay_t<decltype(v)>>) {
-                      using Vi_T = typename std::decay_t<decltype(v)>::value_type;
                       for (size_t i = 0; i < v.size(); ++i) {
-                        if constexpr (std::is_arithmetic_v<Vi_T> and not std::is_same_v<DataT, bool>) {
-                          m_lhs[i] = std::move(std::to_string(v[i]));
-                        } else {
-                          std::ostringstream os;
-                          os << std::boolalpha << v[i];
-                          m_lhs[i] = std::move(os.str());
-                        }
+                        m_lhs[i] = std::move(stringify(v[i]));
                       }
                     } else {
                       // LCOV_EXCL_START
@@ -275,13 +266,7 @@ class AffectationExecutor final : public IAffectationExecutor
                                          std::is_convertible_v<T, ValueContentT>) {
                       tuple_value[i] = static_cast<ValueContentT>(child_value);
                     } else if constexpr (std::is_same_v<std::string, ValueContentT>) {
-                      if constexpr (std::is_arithmetic_v<T> and not std::is_same_v<T, bool>) {
-                        tuple_value[i] = std::to_string(child_value);
-                      } else {
-                        std::ostringstream os;
-                        os << std::boolalpha << child_value;
-                        tuple_value[i] = os.str();
-                      }
+                      tuple_value[i] = std::move(stringify(child_value));
                     } else if constexpr (is_tiny_vector_v<ValueContentT>) {
                       if constexpr (std::is_arithmetic_v<T>) {
                         if constexpr (std::is_same_v<ValueContentT, TinyVector<1>>) {
@@ -331,13 +316,7 @@ class AffectationExecutor final : public IAffectationExecutor
                                        std::is_convertible_v<T, ValueContentT>) {
                     tuple_value[0] = static_cast<ValueContentT>(child_value);
                   } else if constexpr (std::is_same_v<std::string, ValueContentT>) {
-                    if constexpr (std::is_arithmetic_v<T> and not std::is_same_v<DataT, bool>) {
-                      tuple_value[0] = std::to_string(child_value);
-                    } else {
-                      std::ostringstream os;
-                      os << std::boolalpha << child_value;
-                      tuple_value[0] = os.str();
-                    }
+                    tuple_value[0] = std::move(stringify(child_value));
                   } else if constexpr (is_tiny_vector_v<ValueContentT>) {
                     if constexpr (std::is_arithmetic_v<T>) {
                       if constexpr (std::is_same_v<ValueContentT, TinyVector<1>>) {
@@ -464,6 +443,12 @@ class AffectationToTupleProcessor final : public AffectationToDataVariantProcess
   DataVariant
   execute(ExecutionPolicy& exec_policy)
   {
+    auto stringify = [](auto&& value) {
+      std::ostringstream os;
+      os << std::boolalpha << value;
+      return os.str();
+    };
+
     DataVariant value = m_rhs_node.execute(exec_policy);
 
     std::visit(
@@ -474,13 +459,7 @@ class AffectationToTupleProcessor final : public AffectationToDataVariantProcess
         } else if constexpr (std::is_arithmetic_v<ValueT> and std::is_convertible_v<T, ValueT>) {
           *m_lhs = std::vector{std::move(static_cast<ValueT>(v))};
         } else if constexpr (std::is_same_v<std::string, ValueT>) {
-          if constexpr (std::is_arithmetic_v<T> and not std::is_same_v<T, bool>) {
-            *m_lhs = std::vector{std::move(std::to_string(v))};
-          } else {
-            std::ostringstream os;
-            os << std::boolalpha << v;
-            *m_lhs = std::vector<std::string>{os.str()};
-          }
+          *m_lhs = std::vector{std::move(stringify(v))};
         } else if constexpr (is_tiny_vector_v<ValueT> or is_tiny_matrix_v<ValueT>) {
           if constexpr (std::is_same_v<ValueT, TinyVector<1>> and std::is_arithmetic_v<T>) {
             *m_lhs = std::vector<TinyVector<1>>{TinyVector<1>{static_cast<double>(v)}};
@@ -519,6 +498,12 @@ class AffectationToTupleFromListProcessor final : public AffectationToDataVarian
   void
   _copyAggregateDataVariant(const AggregateDataVariant& children_values)
   {
+    auto stringify = [](auto&& value) {
+      std::ostringstream os;
+      os << std::boolalpha << value;
+      return os.str();
+    };
+
     std::vector<ValueT> tuple_value(children_values.size());
     for (size_t i = 0; i < children_values.size(); ++i) {
       std::visit(
@@ -529,13 +514,7 @@ class AffectationToTupleFromListProcessor final : public AffectationToDataVarian
           } else if constexpr (std::is_arithmetic_v<ValueT> and std::is_convertible_v<T, ValueT>) {
             tuple_value[i] = static_cast<ValueT>(child_value);
           } else if constexpr (std::is_same_v<std::string, ValueT>) {
-            if constexpr (std::is_arithmetic_v<T> and not std::is_same_v<T, bool>) {
-              tuple_value[i] = std::to_string(child_value);
-            } else {
-              std::ostringstream os;
-              os << std::boolalpha << child_value;
-              tuple_value[i] = os.str();
-            }
+            tuple_value[i] = std::move(stringify(child_value));
           } else if constexpr (is_tiny_vector_v<ValueT>) {
             if constexpr (std::is_arithmetic_v<T>) {
               if constexpr (std::is_same_v<ValueT, TinyVector<1>>) {
@@ -582,6 +561,12 @@ class AffectationToTupleFromListProcessor final : public AffectationToDataVarian
   void
   _copyVector(const std::vector<DataType>& values)
   {
+    auto stringify = [](auto&& value) {
+      std::ostringstream os;
+      os << std::boolalpha << value;
+      return os.str();
+    };
+
     std::vector<ValueT> v(values.size());
     if constexpr (std::is_same_v<ValueT, DataType>) {
       for (size_t i = 0; i < values.size(); ++i) {
@@ -592,16 +577,8 @@ class AffectationToTupleFromListProcessor final : public AffectationToDataVarian
         v[i] = static_cast<DataType>(values[i]);
       }
     } else if constexpr (std::is_same_v<ValueT, std::string>) {
-      if constexpr (std::is_arithmetic_v<DataType>) {
-        for (size_t i = 0; i < values.size(); ++i) {
-          v[i] = std::to_string(values[i]);
-        }
-      } else {
-        for (size_t i = 0; i < values.size(); ++i) {
-          std::ostringstream sout;
-          sout << values[i];
-          v[i] = sout.str();
-        }
+      for (size_t i = 0; i < values.size(); ++i) {
+        v[i] = std::move(stringify(values[i]));
       }
     } else {
       // LCOV_EXCL_START
diff --git a/tests/test_AffectationToStringProcessor.cpp b/tests/test_AffectationToStringProcessor.cpp
index 040692a9e..6b20e7ca8 100644
--- a/tests/test_AffectationToStringProcessor.cpp
+++ b/tests/test_AffectationToStringProcessor.cpp
@@ -51,61 +51,38 @@
 
 TEST_CASE("ASTAffectationToStringProcessor", "[language]")
 {
+  auto stringify = [](auto&& value) {
+    std::ostringstream os;
+    os << std::boolalpha << value;
+    return os.str();
+  };
+
   SECTION("Affectations")
   {
     CHECK_AFFECTATION_RESULT(R"(let s : string; s = "foo";)", "s", std::string("foo"));
-    CHECK_AFFECTATION_RESULT(R"(let n : N, n = 2; let s : string; s = n;)", "s", std::to_string(2ul));
-    CHECK_AFFECTATION_RESULT(R"(let s : string; s = -1;)", "s", std::to_string(-1l));
-    {
-      std::ostringstream os;
-      os << std::boolalpha << true;
-      CHECK_AFFECTATION_RESULT(R"(let s : string; s = true;)", "s", os.str());
-    }
-    CHECK_AFFECTATION_RESULT(R"(let s : string; s = 2.3;)", "s", std::to_string(2.3));
-    {
-      std::ostringstream os;
-      os << TinyVector<1>{13};
-      CHECK_AFFECTATION_RESULT(R"(let x : R^1, x = 13; let s : string; s = x;)", "s", os.str());
-    }
-    {
-      std::ostringstream os;
-      os << TinyVector<2>{2, 3};
-      CHECK_AFFECTATION_RESULT(R"(let x : R^2, x = [2,3]; let s : string; s = x;)", "s", os.str());
-    }
-    {
-      std::ostringstream os;
-      os << TinyVector<3>{1, 2, 3};
-      CHECK_AFFECTATION_RESULT(R"(let x : R^3, x = [1,2,3]; let s : string; s = x;)", "s", os.str());
-    }
+    CHECK_AFFECTATION_RESULT(R"(let n : N, n = 2; let s : string; s = n;)", "s", stringify(2ul));
+    CHECK_AFFECTATION_RESULT(R"(let s : string; s = -1;)", "s", stringify(-1l));
+    CHECK_AFFECTATION_RESULT(R"(let s : string; s = true;)", "s", stringify(true));
+    CHECK_AFFECTATION_RESULT(R"(let s : string; s = 2.3;)", "s", stringify(2.3));
+    CHECK_AFFECTATION_RESULT(R"(let x : R^1, x = 13; let s : string; s = x;)", "s", stringify(TinyVector<1>{13}));
+    CHECK_AFFECTATION_RESULT(R"(let x : R^2, x = [2,3]; let s : string; s = x;)", "s", stringify(TinyVector<2>{2, 3}));
+    CHECK_AFFECTATION_RESULT(R"(let x : R^3, x = [1,2,3]; let s : string; s = x;)", "s",
+                             stringify(TinyVector<3>{1, 2, 3}));
   }
 
   SECTION("+=")
   {
     CHECK_AFFECTATION_RESULT(R"(let s : string, s = "foo"; s += "bar";)", "s", std::string("foobar"));
     CHECK_AFFECTATION_RESULT(R"(let n : N, n = 2; let s : string, s = "foo"; s += n;)", "s",
-                             (std::string("foo") + std::to_string(2ul)));
-    CHECK_AFFECTATION_RESULT(R"(let s : string, s = "foo"; s += -1;)", "s", (std::string("foo") + std::to_string(-1l)));
-    {
-      std::ostringstream os;
-      os << "foo" << std::boolalpha << true;
-      CHECK_AFFECTATION_RESULT(R"(let s : string, s = "foo"; s += true;)", "s", os.str());
-    }
-    CHECK_AFFECTATION_RESULT(R"(let s : string, s = "foo"; s += 2.3;)", "s",
-                             (std::string("foo") + std::to_string(2.3)));
-    {
-      std::ostringstream os;
-      os << "foo" << TinyVector<1>{13};
-      CHECK_AFFECTATION_RESULT(R"(let x : R^1, x = 13; let s : string, s="foo"; s += x;)", "s", os.str());
-    }
-    {
-      std::ostringstream os;
-      os << "foo" << TinyVector<2>{2, 3};
-      CHECK_AFFECTATION_RESULT(R"(let x : R^2, x = [2,3]; let s : string, s="foo"; s += x;)", "s", os.str());
-    }
-    {
-      std::ostringstream os;
-      os << "foo" << TinyVector<3>{1, 2, 3};
-      CHECK_AFFECTATION_RESULT(R"(let x : R^3, x = [1,2,3]; let s : string, s="foo"; s += x;)", "s", os.str());
-    }
+                             (std::string("foo") + stringify(2ul)));
+    CHECK_AFFECTATION_RESULT(R"(let s : string, s = "foo"; s += -1;)", "s", (std::string("foo") + stringify(-1l)));
+    CHECK_AFFECTATION_RESULT(R"(let s : string, s = "foo"; s += true;)", "s", (std::string("foo") + stringify(true)));
+    CHECK_AFFECTATION_RESULT(R"(let s : string, s = "foo"; s += 2.3;)", "s", (std::string("foo") + stringify(2.3)));
+    CHECK_AFFECTATION_RESULT(R"(let x : R^1, x = 13; let s : string, s="foo"; s += x;)", "s",
+                             (std::string("foo") + stringify(TinyVector<1>{13})));
+    CHECK_AFFECTATION_RESULT(R"(let x : R^2, x = [2,3]; let s : string, s="foo"; s += x;)", "s",
+                             (std::string("foo") + stringify(TinyVector<2>{2, 3})));
+    CHECK_AFFECTATION_RESULT(R"(let x : R^3, x = [1,2,3]; let s : string, s="foo"; s += x;)", "s",
+                             (std::string("foo") + stringify(TinyVector<3>{1, 2, 3})));
   }
 }
diff --git a/tests/test_AffectationToTupleProcessor.cpp b/tests/test_AffectationToTupleProcessor.cpp
index 9871cf1d0..479139870 100644
--- a/tests/test_AffectationToTupleProcessor.cpp
+++ b/tests/test_AffectationToTupleProcessor.cpp
@@ -51,6 +51,12 @@
 
 TEST_CASE("ASTAffectationToTupleProcessor", "[language]")
 {
+  auto stringify = [](auto&& value) {
+    std::ostringstream os;
+    os << std::boolalpha << value;
+    return os.str();
+  };
+
   SECTION("Affectations from value")
   {
     CHECK_AFFECTATION_RESULT(R"(
@@ -66,36 +72,24 @@ let s :(R); s = 2;
     CHECK_AFFECTATION_RESULT(R"(
 let s :(string); s = 2.;
 )",
-                             "s", (std::vector<std::string>{std::to_string(2.)}));
-
-    const std::string x_string = []() -> std::string {
-      std::ostringstream os;
-      os << TinyVector<3, double>{1, 2, 3};
-      return os.str();
-    }();
+                             "s", (std::vector<std::string>{stringify(2.)}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let x :R^3, x = [1,2,3];
 let s :(string); s = x;
 )",
-                             "s", (std::vector<std::string>{x_string}));
+                             "s", (std::vector<std::string>{stringify(TinyVector<3, double>{1, 2, 3})}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let s :(R^1); s = 1.3;
 )",
                              "s", (std::vector<TinyVector<1>>{TinyVector<1>{1.3}}));
 
-    const std::string A_string = []() -> std::string {
-      std::ostringstream os;
-      os << TinyMatrix<2>{1, 2, 3, 4};
-      return os.str();
-    }();
-
     CHECK_AFFECTATION_RESULT(R"(
 let A :R^2x2, A = [[1,2],[3,4]];
 let s :(string); s = A;
 )",
-                             "s", (std::vector<std::string>{A_string}));
+                             "s", (std::vector<std::string>{stringify(TinyMatrix<2>{1, 2, 3, 4})}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let s :(R^1x1); s = 1.3;
@@ -113,25 +107,20 @@ let t :(R); t = (2.,3);
     CHECK_AFFECTATION_RESULT(R"(
 let s :(string); s = (2.,3);
 )",
-                             "s", (std::vector<std::string>{std::to_string(2.), std::to_string(3)}));
+                             "s", (std::vector<std::string>{stringify(2.), stringify(3)}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let s :(string); s = (2.,3,"foo");
 )",
-                             "s",
-                             (std::vector<std::string>{std::to_string(2.), std::to_string(3), std::string{"foo"}}));
-
-    const std::string x_string = []() -> std::string {
-      std::ostringstream os;
-      os << TinyVector<2, double>{1, 2};
-      return os.str();
-    }();
+                             "s", (std::vector<std::string>{stringify(2.), stringify(3), std::string("foo")}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let x : R^2, x = [1,2];
 let s : (string); s = (2.,3, x);
 )",
-                             "s", (std::vector<std::string>{std::to_string(2.), std::to_string(3), x_string}));
+                             "s",
+                             (std::vector<std::string>{stringify(2.), stringify(3),
+                                                       stringify(TinyVector<2, double>{1, 2})}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let x : R^2, x = [1,2];
@@ -160,17 +149,13 @@ let t :(R^1); t = (x,2);
 )",
                              "t", (std::vector<TinyVector<1>>{TinyVector<1>{1}, TinyVector<1>{2}}));
 
-    const std::string A_string = []() -> std::string {
-      std::ostringstream os;
-      os << TinyMatrix<2>{1, 2, 3, 4};
-      return os.str();
-    }();
-
     CHECK_AFFECTATION_RESULT(R"(
 let A : R^2x2, A = [[1,2],[3,4]];
 let s : (string); s = (2.,3, A);
 )",
-                             "s", (std::vector<std::string>{std::to_string(2.), std::to_string(3), A_string}));
+                             "s",
+                             (std::vector<std::string>{stringify(2.), stringify(3),
+                                                       stringify(TinyMatrix<2>{1, 2, 3, 4})}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let A : R^2x2, A = [[1,2],[3,4]];
@@ -202,36 +187,23 @@ let t :(R^1x1); t = (x,2);
 
   SECTION("Affectations from tuple")
   {
-    const std::string x_string = []() -> std::string {
-      std::ostringstream os;
-      os << TinyVector<3, double>{1, 2, 3};
-      return os.str();
-    }();
-
     CHECK_AFFECTATION_RESULT(R"(
 let x :(R^3), x = [1,2,3];
 let s :(string); s = x;
 )",
-                             "s", (std::vector<std::string>{x_string}));
-
-    const std::string A_string = []() -> std::string {
-      std::ostringstream os;
-      os << TinyMatrix<3>{1, 2, 3, 4, 5, 6, 7, 8, 9};
-      return os.str();
-    }();
+                             "s", (std::vector<std::string>{stringify(TinyVector<3, double>{1, 2, 3})}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let A :(R^3x3), A = ([[1,2,3],[4,5,6],[7,8,9]]);
 let s :(string); s = A;
 )",
-                             "s", (std::vector<std::string>{A_string}));
+                             "s", (std::vector<std::string>{stringify(TinyMatrix<3>{1, 2, 3, 4, 5, 6, 7, 8, 9})}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let x :(R), x = (1,2,3);
 let s :(string); s = x;
 )",
-                             "s",
-                             (std::vector<std::string>{std::to_string(1.), std::to_string(2.), std::to_string(3.)}));
+                             "s", (std::vector<std::string>{stringify(1.), stringify(2.), stringify(3.)}));
 
     CHECK_AFFECTATION_RESULT(R"(
 let n :(N), n = (1,2,3);
diff --git a/tests/test_ListAffectationProcessor.cpp b/tests/test_ListAffectationProcessor.cpp
index d02ee365f..55f99e665 100644
--- a/tests/test_ListAffectationProcessor.cpp
+++ b/tests/test_ListAffectationProcessor.cpp
@@ -49,6 +49,12 @@
 
 TEST_CASE("ListAffectationProcessor", "[language]")
 {
+  auto stringify = [](auto&& value) {
+    std::ostringstream os;
+    os << std::boolalpha << value;
+    return os.str();
+  };
+
   SECTION("ListAffectations")
   {
     SECTION("R*R^2*R^2x2*string")
@@ -66,25 +72,13 @@ TEST_CASE("ListAffectationProcessor", "[language]")
     SECTION("compound with string conversion")
     {
       CHECK_AFFECTATION_RESULT(R"(let z:R, z = 3; let (x,u,s):R*R^2*string, (x,u,s) = (1.2, [2,3], z);)", "s",
-                               std::to_string(double{3}));
-      {
-        std::ostringstream os;
-        os << TinyVector<1>{7};
-        CHECK_AFFECTATION_RESULT(R"(let v:R^1, v = 7; let  (x,u,s):R*R^2*string, (x,u,s) = (1.2, [2,3], v);)", "s",
-                                 os.str());
-      }
-      {
-        std::ostringstream os;
-        os << TinyVector<2>{6, 3};
-        CHECK_AFFECTATION_RESULT(R"(let v: R^2, v = [6,3]; let (x,u,s):R*R^2*string, (x,u,s) = (1.2, [2,3], v);)", "s",
-                                 os.str());
-      }
-      {
-        std::ostringstream os;
-        os << TinyVector<3>{1, 2, 3};
-        CHECK_AFFECTATION_RESULT(R"(let v:R^3, v = [1,2,3]; let (x,u,s):R*R^2*string, (x,u,s) = (1.2, [2,3], v);)", "s",
-                                 os.str());
-      }
+                               stringify(double{3}));
+      CHECK_AFFECTATION_RESULT(R"(let v:R^1, v = 7; let  (x,u,s):R*R^2*string, (x,u,s) = (1.2, [2,3], v);)", "s",
+                               stringify(TinyVector<1>{7}));
+      CHECK_AFFECTATION_RESULT(R"(let v: R^2, v = [6,3]; let (x,u,s):R*R^2*string, (x,u,s) = (1.2, [2,3], v);)", "s",
+                               stringify(TinyVector<2>{6, 3}));
+      CHECK_AFFECTATION_RESULT(R"(let v:R^3, v = [1,2,3]; let (x,u,s):R*R^2*string, (x,u,s) = (1.2, [2,3], v);)", "s",
+                               stringify(TinyVector<3>{1, 2, 3}));
     }
 
     SECTION("compound R^d from '0'")
-- 
GitLab