Resolves ambiguity around get_converted_values

NOAA-OWP · Jul 11, 2022 · 24d1658 · 24d1658
1 parent f7d28d0
commit 24d1658
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 7 deletions.
diff --git a/include/core/mediator/UnitsHelper.hpp b/include/core/mediator/UnitsHelper.hpp
@@ -11,7 +11,7 @@ class UnitsHelper {
 
     static double get_converted_value(const std::string &in_units, const double &value, const std::string &out_units);
 
-    static double* get_converted_values(const std::string &in_units, double* values, const std::string &out_units, const size_t & count);
+    static double* convert_values(const std::string &in_units, double* values, const std::string &out_units, double* out_values, const size_t & count);
 
     private:
     static cv_converter* get_converter(const std::string &in_units, const std::string& out_units, ut_unit*& to, ut_unit*& from);

diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp
@@ -421,7 +421,7 @@ namespace realization {
                 // Convert units
                 std::string native_units = get_bmi_model()->GetVarUnits(bmi_var_name);
                 try {
-                    UnitsHelper::get_converted_values(native_units, values.data(), output_units, values.size());
+                    UnitsHelper::convert_values(native_units, values.data(), output_units, values.data(), values.size());
                     return values;
                 }
                 catch (const std::runtime_error& e){

diff --git a/src/core/mediator/UnitsHelper.cpp b/src/core/mediator/UnitsHelper.cpp
@@ -1,4 +1,5 @@
 #include "UnitsHelper.hpp"
+#include <cstring>
 
 ut_system* UnitsHelper::unit_system;
 std::once_flag UnitsHelper::unit_system_inited;
@@ -45,19 +46,25 @@ double UnitsHelper::get_converted_value(const std::string &in_units, const doubl
     return r;
 }
 
-double* UnitsHelper::get_converted_values(const std::string &in_units, double* values, const std::string &out_units, const size_t& count)
+double* UnitsHelper::convert_values(const std::string &in_units, double* in_values, const std::string &out_units, double* out_values, const size_t& count)
 {
     if(in_units == out_units){
-        return values; // Early-out optimization
+        // Early-out optimization
+        if(in_values == out_values){
+            return in_values;
+        } else {
+            memcpy(out_values, in_values, sizeof(double)*count);
+            return out_values;
+        }
     }
     std::call_once(unit_system_inited, init_unit_system);
     ut_unit* to = NULL;
     ut_unit* from = NULL;
     cv_converter* conv = get_converter(in_units, out_units, to, from);
 
-    cv_convert_doubles(conv, values, count, values);
+    cv_convert_doubles(conv, in_values, count, out_values);
     ut_free(from);
     ut_free(to);
     cv_free(conv);
-    return values;
+    return out_values;
 }
diff --git a/test/core/mediator/UnitsHelper_Tests.cpp b/test/core/mediator/UnitsHelper_Tests.cpp
@@ -28,7 +28,36 @@ TEST_F(UnitsHelper_Test, TestConvertArray){
     std::vector<double> data = {1,2,3,4};
     std::vector<double> expected = {1000, 2000, 3000, 4000};
     //Call the converter, updates data in place
-    UnitsHelper::get_converted_values("m", data.data(), "mm", data.size());
+    UnitsHelper::convert_values("m", data.data(), "mm", data.data(), data.size());
     ASSERT_EQ( expected,  data);
 
 }
+
+// For coverage completeness...
+TEST_F(UnitsHelper_Test, TestConvertArrayNoOp){
+    std::vector<double> data = {1,2,3,4};
+    std::vector<double> expected = {1, 2, 3, 4};
+    //Call the converter, updates data in place
+    UnitsHelper::convert_values("m", data.data(), "m", data.data(), data.size());
+    ASSERT_EQ( expected,  data);
+}
+
+TEST_F(UnitsHelper_Test, TestConvertArrayDontModifyInput){
+    std::vector<double> data = {1,2,3,4};
+    std::vector<double> data2 = {2,4,6,8};
+    std::vector<double> expected = {1000, 2000, 3000, 4000};
+    //Call the converter, updates data in place
+    UnitsHelper::convert_values("m", data.data(), "mm", data2.data(), data.size());
+    ASSERT_EQ( expected,  data2);
+    ASSERT_EQ( data.at(2), 3);
+}
+
+TEST_F(UnitsHelper_Test, TestConvertArrayDontModifyInputNoOp){
+    std::vector<double> data = {1,2,3,4};
+    std::vector<double> data2 = {2,4,6,8};
+    std::vector<double> expected = {1, 2, 3, 4};
+    //Call the converter, updates data in place
+    UnitsHelper::convert_values("m", data.data(), "m", data2.data(), data.size());
+    ASSERT_EQ( expected,  data2);
+    ASSERT_EQ( data.at(2), 3);
+}