From 0ec43e3611180bf7580da1404478900b47ff3467 Mon Sep 17 00:00:00 2001
From: Ted John <ted@brambles.org>
Date: Sat, 25 Feb 2017 16:07:08 +0000
Subject: [PATCH] Improve String::Split and add tests

---
 src/openrct2/common.h        |  3 +++
 src/openrct2/core/String.cpp | 43 ++++++++++++++++++++++--------------
 test/tests/AssertHelpers.hpp | 25 +++++++++++++++++++++
 test/tests/StringTest.cpp    | 21 ++++++++++++++++++
 test/tests/tests.vcxproj     |  1 +
 5 files changed, 76 insertions(+), 17 deletions(-)
 create mode 100644 test/tests/AssertHelpers.hpp

diff --git a/src/openrct2/common.h b/src/openrct2/common.h
index 1a1d89dafe..14209e0ef3 100644
--- a/src/openrct2/common.h
+++ b/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__)
diff --git a/src/openrct2/core/String.cpp b/src/openrct2/core/String.cpp
index b91061744e..1c96ee3304 100644
--- a/src/openrct2/core/String.cpp
+++ b/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;
-        size_t index = 0;
-        size_t nextIndex;
-        do
+        if (delimiter.empty())
         {
-            nextIndex = s.find_first_of(delimiter, index);
-            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 + 1;
+            throw std::invalid_argument(nameof(delimiter) " can not be empty.");
+        }
+
+        std::vector<std::string> results;
+        if (!s.empty())
+        {
+            size_t index = 0;
+            size_t nextIndex;
+            do
+            {
+                nextIndex = s.find(delimiter, index);
+                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;
     }
 
diff --git a/test/tests/AssertHelpers.hpp b/test/tests/AssertHelpers.hpp
new file mode 100644
index 0000000000..1098db7a2c
--- /dev/null
+++ b/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);
+}
diff --git a/test/tests/StringTest.cpp b/test/tests/StringTest.cpp
index 8644c3c19b..7280eb6200 100644
--- a/test/tests/StringTest.cpp
+++ b/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);
+}
diff --git a/test/tests/tests.vcxproj b/test/tests/tests.vcxproj
index 02dc0344b5..bbc3753f97 100644
--- a/test/tests/tests.vcxproj
+++ b/test/tests/tests.vcxproj
@@ -47,6 +47,7 @@
   </ItemDefinitionGroup>
   <!-- Files -->
   <ItemGroup>
+    <ClInclude Include="AssertHelpers.hpp" />
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="LanguagePackTest.cpp" />