Improve String::Split and add tests

src/openrct2/common.h

@@ -113,6 +113,9 @@ typedef uint8 colour_t;
 #endif // __GNUC__
 #endif // __cplusplus
 
+// Gets the name of a symbol as a C string
+#define nameof(symbol) #symbol
+
 #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
 #include <unistd.h>
 #define STUB() log_warning("Function %s at %s:%d is a stub.\n", __PRETTY_FUNCTION__, __FILE__, __LINE__)

src/openrct2/core/String.cpp

@@ -15,6 +15,7 @@
 #pragma endregion
 
 #include <cwctype>
+#include <stdexcept>
 #include <vector>
 
 extern "C"
@@ -356,25 +357,33 @@ namespace String
 
     std::vector<std::string> Split(const std::string &s, const std::string &delimiter)
     {
-        std::vector<std::string> results;
+        if (delimiter.empty())
-        size_t index = 0;
-        size_t nextIndex;
-        do
         {
-            nextIndex = s.find_first_of(delimiter, index);
+            throw std::invalid_argument(nameof(delimiter) " can not be empty.");
-            std::string value;
+        }
-            if (nextIndex == std::string::npos)
+
-            {
+        std::vector<std::string> results;
-                value = s.substr(index);
+        if (!s.empty())
-            }
+        {
-            else
+            size_t index = 0;
-            {
+            size_t nextIndex;
-                value = s.substr(index, nextIndex - index);
+            do
-            }
+            {
-            results.push_back(value);
+                nextIndex = s.find(delimiter, index);
-            index = nextIndex + 1;
+                std::string value;
+                if (nextIndex == std::string::npos)
+                {
+                    value = s.substr(index);
+                }
+                else
+                {
+                    value = s.substr(index, nextIndex - index);
+                }
+                results.push_back(value);
+                index = nextIndex + delimiter.size();
+            }
+            while (nextIndex != SIZE_MAX);
         }
-        while (nextIndex != SIZE_MAX);
         return results;
     }
 

test/tests/AssertHelpers.hpp

@@ -0,0 +1,25 @@
+#pragma once
+
+#include <initializer_list>
+#include <vector>
+#include <gtest/gtest.h>
+
+template<typename T, typename TExpected>
+void AssertVector(std::vector<T> actual, TExpected expected)
+{
+    ASSERT_EQ(actual.size(), expected.size()) <<
+        "Expected vector of size " << expected.size() << ", but was " << actual.size();
+    size_t i = 0;
+    for (auto item : expected)
+    {
+        EXPECT_EQ(actual[i], item) <<
+            "Element at index " << i << " did not match";
+        i++;
+    }
+}
+
+template<typename T>
+void AssertVector(std::vector<T> actual, std::initializer_list<T> expected)
+{
+    AssertVector<T, std::initializer_list<T>>(actual, expected);
+}

test/tests/StringTest.cpp

@@ -2,6 +2,7 @@
 #include <tuple>
 #include <gtest/gtest.h>
 #include <openrct2/core/String.hpp>
+#include "AssertHelpers.hpp"
 
 using TCase = std::tuple<std::string, std::string>;
 
@@ -28,3 +29,23 @@ TEST_P(StringTest, Trim)
     std::string actual = String::Trim(input);
     ASSERT_EQ(expected, actual);
 }
+
+TEST_F(StringTest, Split_ByComma)
+{
+    auto actual = String::Split("a,bb,ccc,dd", ",");
+    AssertVector<std::string>(actual, { "a", "bb", "ccc", "dd" });
+}
+TEST_F(StringTest, Split_ByColonColon)
+{
+    auto actual = String::Split("a::bb:ccc:::::dd", "::");
+    AssertVector<std::string>(actual, { "a", "bb:ccc", "", ":dd" });
+}
+TEST_F(StringTest, Split_Empty)
+{
+    auto actual = String::Split("", ".");
+    AssertVector<std::string>(actual, { });
+}
+TEST_F(StringTest, Split_ByEmpty)
+{
+    EXPECT_THROW(String::Split("string", ""), std::invalid_argument);
+}

test/tests/tests.vcxproj

@@ -47,6 +47,7 @@
   </ItemDefinitionGroup>
   <!-- Files -->
   <ItemGroup>
+    <ClInclude Include="AssertHelpers.hpp" />
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="LanguagePackTest.cpp" />