Findings from review

exasol · Oct 25, 2024 · e30b172 · e30b172
1 parent 624ea3b
commit e30b172
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 11 deletions.
diff --git a/exaudfclient/base/javacontainer/script_options/converter_legacy.cc b/exaudfclient/base/javacontainer/script_options/converter_legacy.cc
@@ -16,9 +16,7 @@ void ConverterLegacy::convertExternalJar(const std::string& value) {
     std::string jar;
 
     while (std::getline(stream, jar, ':')) {
-        if (m_jarPaths.find(jar) == m_jarPaths.end()) {
-            m_jarPaths.insert(jar);
-        }
+        m_jarPaths.insert(jar);
     }
 }
 

diff --git a/exaudfclient/base/javacontainer/script_options/test/converter_test.cc b/exaudfclient/base/javacontainer/script_options/test/converter_test.cc
@@ -22,9 +22,9 @@ TEST_P(LegacyConverterJarTest, jar) {
 
 const std::vector<std::pair<std::string, std::vector<std::string>>> jar_strings =
         {
-            std::make_pair("test.jar:test2.jar", std::vector<std::string>({"test.jar", "test2.jar"})),
-            std::make_pair("test.jar:test.jar", std::vector<std::string>({"test.jar"})),
-            std::make_pair("testDEF.jar:testABC.jar", std::vector<std::string>({"testABC.jar", "testDEF.jar"})),
+            std::make_pair("test.jar:test2.jar", std::vector<std::string>({"test.jar", "test2.jar"})), //basic splitting
+            std::make_pair("test.jar:test.jar", std::vector<std::string>({"test.jar"})), //filter duplicates
+            std::make_pair("testDEF.jar:testABC.jar", std::vector<std::string>({"testABC.jar", "testDEF.jar"})), //alphabetical order
         };
 
 INSTANTIATE_TEST_SUITE_P(
@@ -49,15 +49,15 @@ TEST_P(ConverterV2JarTest, jar) {
     ASSERT_EQ(result, option_value.second);
 }
 
-const std::vector<std::pair<std::string, std::vector<std::string>>> jar_escape_sequences =
+const std::vector<std::pair<std::string, std::vector<std::string>>> jar_strings_v2 =
         {
-            std::make_pair("test.jar:test2.jar", std::vector<std::string>({"test.jar", "test2.jar"})),
-            std::make_pair("test.jar:test.jar", std::vector<std::string>({"test.jar", "test.jar"})),
-            std::make_pair("testDEF.jar:testABC.jar", std::vector<std::string>({"testDEF.jar", "testABC.jar"})),
+            std::make_pair("test.jar:test2.jar", std::vector<std::string>({"test.jar", "test2.jar"})), //basic splitting
+            std::make_pair("test.jar:test.jar", std::vector<std::string>({"test.jar", "test.jar"})), //keep duplicates
+            std::make_pair("testDEF.jar:testABC.jar", std::vector<std::string>({"testDEF.jar", "testABC.jar"})), //maintain order
         };
 
 INSTANTIATE_TEST_SUITE_P(
     Converter,
     ConverterV2JarTest,
-    ::testing::ValuesIn(jar_escape_sequences)
+    ::testing::ValuesIn(jar_strings_v2)
 );