commit e47c09d3cce9391b96453778caa98deb97e88988
author Nan Zhou <nanzhoumails@gmail.com> Tue Oct 25 00:06:41 2022 +0000
committer Nan Zhou <nanzhoumails@gmail.com> Tue Nov 01 18:16:30 2022 +0000
tree f06bc5c84b696d3275c9a5b91a02e609624b3d61
parent c5382dbf79e42e1426cf5a9d6aa7e328974ca3d9

getCSVFromVector: optimize and add tests

Optimization: avoid copy; simpifiled unnessary branches; reduced
temperary string being created when doing string concatenation.

Added unit test for this function.

Tested: unit test

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I8c334dc4f087e262bb3e8613e86cfc2a93d525a0

test/user_mgr_test.cpp

diff --git a/test/user_mgr_test.cpp b/test/user_mgr_test.cpp
index a7801ae..22a8837 100644
--- a/test/user_mgr_test.cpp
+++ b/test/user_mgr_test.cpp
@@ -1,4 +1,5 @@
 #include "mock_user_mgr.hpp"
+#include "user_mgr.hpp"
 
 #include <sdbusplus/test/sdbus_mock.hpp>
 #include <xyz/openbmc_project/Common/error.hpp>
@@ -146,5 +147,18 @@
     EXPECT_EQ(true, std::get<bool>(userInfo["RemoteUser"]));
     EXPECT_EQ("", std::get<std::string>(userInfo["UserPrivilege"]));
 }
+
+TEST(GetCSVFromVector, EmptyVectorReturnsEmptyString)
+{
+    EXPECT_EQ(getCSVFromVector({}), "");
+}
+
+TEST(GetCSVFromVector, ElementsAreJoinedByComma)
+{
+    EXPECT_EQ(getCSVFromVector(std::vector<std::string>{"123"}), "123");
+    EXPECT_EQ(getCSVFromVector(std::vector<std::string>{"123", "456"}),
+              "123,456");
+}
+
 } // namespace user
 } // namespace phosphor

user_mgr.cpp

diff --git a/user_mgr.cpp b/user_mgr.cpp
index 95fb421..7ee424d 100644
--- a/user_mgr.cpp
+++ b/user_mgr.cpp
@@ -44,6 +44,10 @@
 #include <fstream>
 #include <numeric>
 #include <regex>
+#include <span>
+#include <string>
+#include <string_view>
+#include <vector>
 
 namespace phosphor
 {
@@ -130,29 +134,18 @@
     return stdOutput;
 }
 
-static std::string getCSVFromVector(std::vector<std::string> vec)
+std::string getCSVFromVector(std::span<const std::string> vec)
 {
-    switch (vec.size())
+    if (vec.empty())
     {
-        case 0:
-        {
-            return "";
-        }
-        break;
-
-        case 1:
-        {
-            return std::string{vec[0]};
-        }
-        break;
-
-        default:
-        {
-            return std::accumulate(
-                std::next(vec.begin()), vec.end(), vec[0],
-                [](std::string a, std::string b) { return a + ',' + b; });
-        }
+        return "";
     }
+    return std::accumulate(std::next(vec.begin()), vec.end(), vec[0],
+                           [](std::string&& val, std::string_view element) {
+                               val += ',';
+                               val += element;
+                               return val;
+                           });
 }
 
 static bool removeStringFromCSV(std::string& csvStr, const std::string& delStr)

user_mgr.hpp

diff --git a/user_mgr.hpp b/user_mgr.hpp
index 23278ed..5c1553b 100644
--- a/user_mgr.hpp
+++ b/user_mgr.hpp
@@ -21,8 +21,11 @@
 #include <xyz/openbmc_project/User/AccountPolicy/server.hpp>
 #include <xyz/openbmc_project/User/Manager/server.hpp>
 
+#include <span>
+#include <string>
 #include <unordered_map>
 #include <variant>
+#include <vector>
 
 namespace phosphor
 {
@@ -59,6 +62,8 @@
 
 using DbusUserObj = std::map<DbusUserObjPath, DbusUserObjValue>;
 
+std::string getCSVFromVector(std::span<const std::string> vec);
+
 /** @class UserMgr
  *  @brief Responsible for managing user accounts over the D-Bus interface.
  */