From e30b172cf51c37ee2b7cea4b1793a1b4e3242f1d Mon Sep 17 00:00:00 2001
From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com>
Date: Fri, 25 Oct 2024 06:09:41 -0300
Subject: [PATCH] Findings from review

---
 .../script_options/converter_legacy.cc           |  4 +---
 .../script_options/test/converter_test.cc        | 16 ++++++++--------
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/exaudfclient/base/javacontainer/script_options/converter_legacy.cc b/exaudfclient/base/javacontainer/script_options/converter_legacy.cc
index 2aeaadc6..b0fe5011 100644
--- 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
index ecc96c07..dc2725bc 100644
--- 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)
 );