Findings from review

exasol · Oct 24, 2024 · 9d0943c · 9d0943c
1 parent 51beabe
commit 9d0943c
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 35 deletions.
diff --git a/exaudfclient/base/javacontainer/javacontainer_impl.cc b/exaudfclient/base/javacontainer/javacontainer_impl.cc
@@ -56,9 +56,8 @@ void JavaVMImpl::parseScriptOptions(std::unique_ptr<JavaScriptOptions::Extractor
 
     m_jvmOptions = std::move(extractor->moveJvmOptions());
 
-    for (set<string>::iterator it = extractor->getJarPaths().begin(); it != extractor->getJarPaths().end();
-         ++it) {
-        addJarToClasspath(*it);
+    for (const auto & jar :  extractor->getJarPaths()) {
+        addJarToClasspath(jar);
     }
 }
 

diff --git a/exaudfclient/base/javacontainer/script_options/converter.h b/exaudfclient/base/javacontainer/script_options/converter.h
@@ -14,18 +14,19 @@ class Converter {
     public:
         Converter();
 
-        virtual void convertExternalJar(const std::string & value) = 0;
-
         void convertScriptClassName(const std::string & value);
 
         void convertJvmOption(const std::string & value);
 
-        virtual const std::set<std::string> & getJarPaths() const = 0;
-
         std::vector<std::string>&& moveJvmOptions() {
             return std::move(m_jvmOptions);
         }
 
+        virtual void convertExternalJar(const std::string & value) = 0;
+
+        virtual const std::set<std::string> & getJarPaths() const = 0;
+
+
     private:
 
         std::vector<std::string> m_jvmOptions;

diff --git a/exaudfclient/base/javacontainer/script_options/converter_legacy.cc b/exaudfclient/base/javacontainer/script_options/converter_legacy.cc
@@ -1,6 +1,7 @@
 #include "base/javacontainer/script_options/converter_legacy.h"
 #include "base/javacontainer/script_options/string_ops.h"
 #include <iostream>
+#include <sstream>
 
 namespace SWIGVMContainers {
 
@@ -10,20 +11,12 @@ ConverterLegacy::ConverterLegacy()
 : Converter()
 , m_jarPaths() {}
 
-void ConverterLegacy::convertExternalJar(const std::string & value) {
-    for (size_t start = 0, delim = 0; ; start = delim + 1) {
-        delim = value.find(":", start);
-        if (delim != std::string::npos) {
-            std::string jar = value.substr(start, delim - start);
-            if (m_jarPaths.find(jar) == m_jarPaths.end())
-                m_jarPaths.insert(jar);
-        }
-        else {
-            std::string jar = value.substr(start);
-            if (m_jarPaths.find(jar) == m_jarPaths.end())
-                m_jarPaths.insert(jar);
-            break;
-        }
+void ConverterLegacy::convertExternalJar(const std::string& value) {
+    std::istringstream stream(value);
+    std::string jar;
+
+    while (std::getline(stream, jar, ':')) {
+        m_jarPaths.insert(jar); // If m_jarPaths is a Set or you make it a Set, uniqueness implicitly is taken care of
     }
 }
 

diff --git a/exaudfclient/base/javacontainer/script_options/converter_v2.cc b/exaudfclient/base/javacontainer/script_options/converter_v2.cc
@@ -1,6 +1,7 @@
 #include "base/javacontainer/script_options/converter_v2.h"
 #include "base/javacontainer/script_options/string_ops.h"
 #include <iostream>
+#include <istream>
 
 namespace SWIGVMContainers {
 
@@ -11,24 +12,14 @@ ConverterV2::ConverterV2()
 , m_jarPaths() {}
 
 void ConverterV2::convertExternalJar(const std::string & value) {
+    std::istringstream stream(value);
+    std::string jar;
 
-    for (size_t start = 0, delim = 0; ; start = delim + 1) {
-        delim = value.find(":", start);
-        if (delim != std::string::npos) {
-            std::string jar = value.substr(start, delim - start);
-            if (m_jarPaths.find(jar) == m_jarPaths.end())
-                m_jarPaths.insert(jar);
-        }
-        else {
-            std::string jar = value.substr(start);
-            if (m_jarPaths.find(jar) == m_jarPaths.end())
-                m_jarPaths.insert(jar);
-            break;
-        }
+    while (std::getline(stream, jar, ':')) {
+        m_jarPaths.insert(jar); // If m_jarPaths is a Set or you make it a Set, uniqueness implicitly is taken care of
     }
 }
 
-
 } //namespace JavaScriptOptions
 
 } //namespace SWIGVMContainers