Allowed split_string to be used on whitespace if not also trying to strip

Also improved comments (and moved to doxygen) as I assumed strip would
remove
whitespace from throughout the string but actually only does the final
string

Replace asserts in string utils

Adding tests for behaviour with whitespace delimiter

Author: thk123

src/util/string_utils.cpp

@@ -7,11 +7,16 @@ Author: Daniel Poetzl
 \*******************************************************************/
 
 #include "string_utils.h"
+#include "invariant.h"
 
 #include <cassert>
 #include <cctype>
 #include <algorithm>
 
+/// Remove all whitespace characters from either end of a string. Whitespace
+/// in the middle of the string is left unchanged
+/// \param s: the string to strip
+/// \return The stripped string
 std::string strip_string(const std::string &s)
 {
   auto pred=[](char c){ return std::isspace(c); };
@@ -30,15 +35,29 @@ std::string strip_string(const std::string &s)
   return s.substr(i, (j-i+1));
 }
 
+/// Given a string s, split into a sequence of substrings when separated by
+/// specified delimiter.
+/// \param s: The string to split up
+/// \param delim: The character to use as the delimiter
+/// \param [out] result: The sub strings. Must be empty.
+/// \param strip: If true, strip_string will be used on each element, removing
+/// whitespace from the beginning and end of each element
+/// \param remove_empty: If true, all empty-string elements will be removed.
+/// This is applied after strip so whitespace only elements will be removed if
+/// both are set to true
 void split_string(
   const std::string &s,
   char delim,
   std::vector<std::string> &result,
   bool strip,
   bool remove_empty)
 {
-  assert(result.empty());
-  assert(!std::isspace(delim));
+  PRECONDITION(result.empty());
+  if(strip && std::isspace(delim))
+  {
+    throw std::invalid_argument(
+      "delim can't be a space character if using strip");
+  }
 
   if(s.empty())
   {
@@ -47,7 +66,7 @@ void split_string(
   }
 
   std::string::size_type n=s.length();
-  assert(n>0);
+  INVARIANT(n > 0, "Empty string case should already be handled");
 
   std::string::size_type start=0;
   std::string::size_type i;
@@ -87,7 +106,11 @@ void split_string(
   std::string &right,
   bool strip)
 {
-  assert(!std::isspace(delim));
+  if(strip && std::isspace(delim))
+  {
+    throw std::invalid_argument(
+      "delim can't be a space character if using strip");
+  }
 
   std::vector<std::string> result;
 

src/util/string_utils.h

@@ -18,10 +18,10 @@ std::string strip_string(const std::string &s);
 
 void split_string(
   const std::string &s,
-  char delim, // must not be a whitespace character
+  char delim,
   std::vector<std::string> &result,
-  bool strip=false, // strip whitespace from elements
-  bool remove_empty=false); // remove empty elements
+  bool strip = false,
+  bool remove_empty = false);
 
 void split_string(
   const std::string &s,

unit/util/string_utils/split_string.cpp

@@ -134,6 +134,58 @@ SCENARIO("split_string", "[core][utils][string_utils][split_string]")
       run_on_all_variants(string, ',', expected_results);
     }
   }
+  GIVEN("A whitespace delimter")
+  {
+    std::string string = "a\nb\nc";
+    const char delimiter = '\n';
+
+    WHEN("Not stripping, not removing empty")
+    {
+      std::vector<std::string> result;
+      split_string(string, delimiter, result, false, false);
+
+      THEN("Should get expected vector")
+      {
+        std::vector<std::string> expected_result = {"a", "b", "c"};
+        REQUIRE_THAT(
+          result,
+          Catch::Matchers::Vector::EqualsMatcher<std::string>{expected_result});
+      }
+    }
+    WHEN("Not stripping, removing empty")
+    {
+      std::vector<std::string> result;
+      split_string(string, delimiter, result, false, true);
+
+      THEN("Should get expected vector")
+      {
+        std::vector<std::string> expected_result = {"a", "b", "c"};
+        REQUIRE_THAT(
+          result,
+          Catch::Matchers::Vector::EqualsMatcher<std::string>{expected_result});
+      }
+    }
+    WHEN("Stripping, not removing empty")
+    {
+      std::vector<std::string> result;
+      THEN("Should throw an exception")
+      {
+        REQUIRE_THROWS_AS(
+          split_string(string, delimiter, result, true, false),
+          std::invalid_argument);
+      }
+    }
+    WHEN("Stripping and removing empty")
+    {
+      std::vector<std::string> result;
+      THEN("Should throw an exception")
+      {
+        REQUIRE_THROWS_AS(
+          split_string(string, delimiter, result, true, true),
+          std::invalid_argument);
+      }
+    }
+  }
 }
 
 SCENARIO("split_string into two", "[core][utils][string_utils][split_string]")
@@ -155,4 +207,31 @@ SCENARIO("split_string into two", "[core][utils][string_utils][split_string]")
       }
     }
   }
+  GIVEN("A string and a whitespace delimiter")
+  {
+    std::string string = "a\nb";
+    const char delimiter = '\n';
+
+    WHEN("Splitting in two and not stripping")
+    {
+      std::string s1;
+      std::string s2;
+      split_string(string, delimiter, s1, s2, false);
+      THEN("The string should be split")
+      {
+        REQUIRE(s1 == "a");
+        REQUIRE(s2 == "b");
+      }
+    }
+    WHEN("Splitting in two and stripping")
+    {
+      THEN("An invalid argument exception should be raised")
+      {
+        std::string s1;
+        std::string s2;
+        REQUIRE_THROWS_AS(
+          split_string(string, delimiter, s1, s2, true), std::invalid_argument);
+      }
+    }
+  }
 }