Cleanup and (mostly) document util/string.h and (very) minor refactoring

Updated: Incorporated feedback from 'kahrl'
Updated: Moved MinetestApp::boolToCStr() from game.cpp into string.h renaming it bool_to_cstr()

3 files changed, 304 insertions, 138 deletions

diff --git a/src/game.cpp b/src/game.cpp
index 48d43c9f7..a700da8cd 100644
--- a/src/game.cpp
+++ b/src/game.cpp
@@ -1386,8 +1386,6 @@ protected:
 	void showOverlayMessage(const char *msg, float dtime, int percent,
 			bool draw_clouds = true);
 
-	inline const char *boolToCStr(bool v);
-
 private:
 	InputHandler *input;
 
@@ -2472,7 +2470,7 @@ void MinetestApp::toggleFreeMove(float *statustext_time)
 	static const wchar_t *msg[] = { L"free_move disabled", L"free_move enabled" };
 
 	bool free_move = !g_settings->getBool("free_move");
-	g_settings->set("free_move", boolToCStr(free_move));
+	g_settings->set("free_move", bool_to_cstr(free_move));
 
 	*statustext_time = 0;
 	statustext = msg[free_move];
@@ -2494,7 +2492,7 @@ void MinetestApp::toggleFast(float *statustext_time)
 {
 	static const wchar_t *msg[] = { L"fast_move disabled", L"fast_move enabled" };
 	bool fast_move = !g_settings->getBool("fast_move");
-	g_settings->set("fast_move", boolToCStr(fast_move));
+	g_settings->set("fast_move", bool_to_cstr(fast_move));
 
 	*statustext_time = 0;
 	statustext = msg[fast_move];
@@ -2508,7 +2506,7 @@ void MinetestApp::toggleNoClip(float *statustext_time)
 {
 	static const wchar_t *msg[] = { L"noclip disabled", L"noclip enabled" };
 	bool noclip = !g_settings->getBool("noclip");
-	g_settings->set("noclip", boolToCStr(noclip));
+	g_settings->set("noclip", bool_to_cstr(noclip));
 
 	*statustext_time = 0;
 	statustext = msg[noclip];
@@ -3937,13 +3935,6 @@ void MinetestApp::showOverlayMessage(const char *msg, float dtime,
 }
 
 
-inline const char *MinetestApp::boolToCStr(bool v)
-{
-	static const char *str[] = { "false", "true" };
-	return str[v];
-}
-
-
 
 /****************************************************************************
  Shutdown / cleanup
diff --git a/src/test.cpp b/src/test.cpp
index 86424ad6b..cd353c0ea 100644
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -166,7 +166,7 @@ struct TestUtilities: public TestBase
 		UASSERT(is_yes("0") == false);
 		UASSERT(is_yes("1") == true);
 		UASSERT(is_yes("2") == true);
-		const char *ends[] = {"abc", "c", "bc", NULL};
+		const char *ends[] = {"abc", "c", "bc", "", NULL};
 		UASSERT(removeStringEnd("abc", ends) == "");
 		UASSERT(removeStringEnd("bc", ends) == "b");
 		UASSERT(removeStringEnd("12c", ends) == "12");
@@ -175,6 +175,30 @@ struct TestUtilities: public TestBase
 				== "%22Aardvarks%20lurk%2C%20OK%3F%22");
 		UASSERT(urldecode("%22Aardvarks%20lurk%2C%20OK%3F%22")
 				== "\"Aardvarks lurk, OK?\"");
+		UASSERT(padStringRight("hello", 8) == "hello   ");
+		UASSERT(str_equal(narrow_to_wide("abc"), narrow_to_wide("abc")));
+		UASSERT(str_equal(narrow_to_wide("ABC"), narrow_to_wide("abc"), true));
+		UASSERT(trim("  a") == "a");
+		UASSERT(trim("   a  ") == "a");
+		UASSERT(trim("a   ") == "a");
+		UASSERT(trim("") == "");
+		UASSERT(mystoi("123", 0, 1000) == 123);
+		UASSERT(mystoi("123", 0, 10) == 10);
+		std::string test_str;
+		test_str = "Hello there";
+		str_replace(test_str, "there", "world");
+		UASSERT(test_str == "Hello world");
+		test_str = "ThisAisAaAtest";
+		str_replace_char(test_str, 'A', ' ');
+		UASSERT(test_str == "This is a test");
+		UASSERT(string_allowed("hello", "abcdefghijklmno") == true);
+		UASSERT(string_allowed("123", "abcdefghijklmno") == false);
+		UASSERT(string_allowed_blacklist("hello", "123") == true);
+		UASSERT(string_allowed_blacklist("hello123", "123") == false);
+		UASSERT(wrap_rows("12345678",4) == "1234\n5678");
+		UASSERT(is_number("123") == true);
+		UASSERT(is_number("") == false);
+		UASSERT(is_number("123a") == false);
 	}
 };
 
diff --git a/src/util/string.h b/src/util/string.h
index c983668a9..f4337062e 100644
--- a/src/util/string.h
+++ b/src/util/string.h
@@ -38,142 +38,235 @@ struct FlagDesc {
 
 std::wstring narrow_to_wide(const std::string& mbs);
 std::string wide_to_narrow(const std::wstring& wcs);
+std::string translatePassword(std::string playername, std::wstring password);
+std::string urlencode(std::string str);
+std::string urldecode(std::string str);
+u32 readFlagString(std::string str, const FlagDesc *flagdesc, u32 *flagmask);
+std::string writeFlagString(u32 flags, const FlagDesc *flagdesc, u32 flagmask);
+size_t mystrlcpy(char *dst, const char *src, size_t size);
+char *mystrtok_r(char *s, const char *sep, char **lasts);
+u64 read_seed(const char *str);
+bool parseColorString(const std::string &value, video::SColor &color, bool quiet);
 
+
+/**
+ * Returns a copy of s with spaces inserted at the right hand side to ensure
+ *			that the string is len characters in length. If s is <= len then the
+ *			returned string will be identical to s.
+ */
 static inline std::string padStringRight(std::string s, size_t len)
 {
-	if(len > s.size())
+	if (len > s.size())
 		s.insert(s.end(), len - s.size(), ' ');
+
 	return s;
 }
 
-// ends: NULL- or ""-terminated array of strings
-// Returns "" if no end could be removed.
+
+/**
+ * Returns a version of the string s with the first occurrence of a string
+ * contained within ends[] removed from the end of the string.
+ *
+ * @param s
+ * @param ends A NULL- or ""- terminated array of strings to remove from s in
+ *			the copy produced. Note that once one of these strings is removed
+ *			that no further postfixes contained within this array are removed.
+ *
+ * @return If no end could be removed then "" is returned
+ */
 static inline std::string removeStringEnd(const std::string &s, const char *ends[])
 {
 	const char **p = ends;
-	for(; (*p) && (*p)[0] != '\0'; p++){
+
+	for (; *p && (*p)[0] != '\0'; p++) {
 		std::string end = *p;
 		if(s.size() < end.size())
 			continue;
 		if(s.substr(s.size()-end.size(), end.size()) == end)
 			return s.substr(0, s.size() - end.size());
 	}
+
 	return "";
 }
 
-// Tests if two strings are equal, optionally case insensitive
-inline bool str_equal(const std::wstring& s1, const std::wstring& s2,
+
+/**
+ * Check two wide strings for equivalence. If case_insensitive is true
+ * then the case of the strings are ignored (default is false).
+ *
+ * @param s1
+ * @param s2
+ * @param case_insensitive
+ * @return true if the strings match
+ */
+inline bool str_equal(const std::wstring &s1, const std::wstring &s2,
 		bool case_insensitive = false)
 {
-	if(case_insensitive)
-	{
-		if(s1.size() != s2.size())
+	if (case_insensitive) {
+		if (s1.size() != s2.size())
 			return false;
-		for(size_t i = 0; i < s1.size(); ++i)
+
+		for (size_t i = 0; i < s1.size(); ++i)
 			if(tolower(s1[i]) != tolower(s2[i]))
 				return false;
+
 		return true;
 	}
-	else
-	{
-		return s1 == s2;
-	}
+
+	return s1 == s2;
 }
 
-// Tests if the second string is a prefix of the first, optionally case insensitive
-inline bool str_starts_with(const std::wstring& str, const std::wstring& prefix,
+
+/**
+ * Check whether str begins with the string prefix. If the argument
+ *			case_insensitive == true then the check is case insensitve (default
+ *			is false; i.e. case is significant).
+ *
+ * @param str
+ * @param prefix
+ * @param case_insensitive
+ * @return	true if the str begins with prefix
+ */
+inline bool str_starts_with(const std::wstring &str, const std::wstring &prefix,
 		bool case_insensitive = false)
 {
-	if(str.size() < prefix.size())
+	if (str.size() < prefix.size())
 		return false;
-	if(case_insensitive)
-	{
-		for(size_t i = 0; i < prefix.size(); ++i)
-			if(tolower(str[i]) != tolower(prefix[i]))
+
+	if (case_insensitive) {
+		for (size_t i = 0; i < prefix.size(); ++i)
+			if (tolower(str[i]) != tolower(prefix[i]))
 				return false;
-	}
-	else
-	{
-		for(size_t i = 0; i < prefix.size(); ++i)
-			if(str[i] != prefix[i])
+	} else {
+		for (size_t i = 0; i < prefix.size(); ++i)
+			if (str[i] != prefix[i])
 				return false;
 	}
+
 	return true;
 }
 
-// Split a string using the given delimiter. Returns a vector containing
-// the component parts.
-inline std::vector<std::wstring> str_split(const std::wstring &str, wchar_t delimiter)
+
+/**
+ * Splits a string of wide characters into its component parts separated by
+ * the character delimiter.
+ *
+ * @return a std::vector<std::wstring> of the component parts
+ */
+inline std::vector<std::wstring> str_split(const std::wstring &str,
+		wchar_t delimiter)
 {
 	std::vector<std::wstring> parts;
 	std::wstringstream sstr(str);
 	std::wstring part;
-	while(std::getline(sstr, part, delimiter))
+
+	while (std::getline(sstr, part, delimiter))
 		parts.push_back(part);
+
 	return parts;
 }
 
+
+/**
+ * Splits a string into its component parts separated by the character
+ *			delimiter.
+ *
+ * @return a std::vector<std::string> of the component parts
+ */
+
 inline std::vector<std::string> str_split(const std::string &str, char delimiter) {
 
 	std::vector<std::string> parts;
 	std::stringstream sstr(str);
 	std::string part;
-	while(std::getline(sstr, part, delimiter))
+
+	while (std::getline(sstr, part, delimiter))
 		parts.push_back(part);
+
 	return parts;
 }
 
+
+/**
+ * Return a copy of s converted to all lowercase characters
+ * @param s
+ */
 inline std::string lowercase(const std::string &s)
 {
-	std::string s2 = s;
-	for(size_t i = 0; i < s.size(); i++)
-		if (isupper(s2.at(i)))
-			s2[i] = tolower(s2.at(i));
+	std::string s2;
+
+	s2.reserve(s.size());
+
+	for (size_t i = 0; i < s.size(); i++)
+		s2 += tolower(s[i]);
+
 	return s2;
 }
 
+
+/**
+ * Returns a copy of s with leading and trailing whitespace removed.
+ * @param s
+ */
 inline std::string trim(const std::string &s)
 {
 	size_t front = 0;
-	while(s[front] == ' '    ||
-	      s[front] == '\t'   ||
-	      s[front] == '\r'   ||
-	      s[front] == '\n'
-	     )
+
+	while (isspace(s[front]))
 		++front;
 
 	size_t back = s.size();
-	while(back > front &&
-	      (s[back-1] == ' '  ||
-	       s[back-1] == '\t' ||
-	       s[back-1] == '\r' ||
-	       s[back-1] == '\n'
-	      )
-	     )
+	while (back > front && isspace(s[back-1]))
 		--back;
 
 	return s.substr(front, back - front);
 }
 
+
+/**
+ * Returns true if s should be regarded as (bool) true. Leading and trailing
+ *			whitespace are ignored; case is ignored. Values that will return
+ *			true are "y", "n", "true" and any number that != 0.
+ * @param s
+ */
 inline bool is_yes(const std::string &s)
 {
 	std::string s2 = lowercase(trim(s));
-	if(s2 == "y" || s2 == "yes" || s2 == "true" || atoi(s2.c_str()) != 0)
-		return true;
-	return false;
+
+	return s2 == "y" || s2 == "yes" || s2 == "true" || atoi(s2.c_str()) != 0;
 }
 
+
+/**
+ * Converts the string s to a signed 32-bit integer. The converted value is
+ * constrained so that min <= value <= max.
+ *
+ * @see atoi(3) for limitations
+ *
+ * @param s
+ * @param min Range minimum
+ * @param max Range maximum
+ * @return The value converted to a signed 32-bit integer and constrained
+ *			within the range defined by min and max (inclusive)
+ */
 inline s32 mystoi(const std::string &s, s32 min, s32 max)
 {
 	s32 i = atoi(s.c_str());
-	if(i < min)
+
+	if (i < min)
 		i = min;
-	if(i > max)
+	if (i > max)
 		i = max;
+
 	return i;
 }
 
-inline s64 stoi64(const std::string &s) {
+
+/**
+ * Returns a 64-bit value reprensented by the string s (decimal).
+ */
+inline s64 stoi64(const std::string &s)
+{
 	std::stringstream tmp(s);
 	s64 t;
 	tmp >> t;
@@ -183,16 +276,34 @@ inline s64 stoi64(const std::string &s) {
 // MSVC2010 includes it's own versions of these
 //#if !defined(_MSC_VER) || _MSC_VER < 1600
 
+
+/**
+ * Returns a 32-bit value reprensented by the string s (decimal).
+ *
+ * @see atoi(3) for further limitations
+ */
 inline s32 mystoi(const std::string &s)
 {
 	return atoi(s.c_str());
 }
 
+
+/**
+ * Returns a 32-bit value reprensented by the wide string s (decimal).
+ *
+ * @see atoi(3) for further limitations
+ */
 inline s32 mystoi(const std::wstring &s)
 {
 	return atoi(wide_to_narrow(s).c_str());
 }
 
+
+/**
+ * Returns a float reprensented by the string s (decimal).
+ *
+ * @see atof(3)
+ */
 inline float mystof(const std::string &s)
 {
 	// This crap causes a segfault in certain cases on MinGW
@@ -209,110 +320,143 @@ inline float mystof(const std::string &s)
 #define stoi mystoi
 #define stof mystof
 
+
+/**
+ * Returns a string representing the decimal value of the 32-bit value i
+ */
 inline std::string itos(s32 i)
 {
 	std::ostringstream o;
-	o<<i;
+	o << i;
 	return o.str();
 }
 
+
+/**
+ * Returns a string representing the decimal value of i of the 64-bit value i
+ */
 inline std::string i64tos(s64 i) {
 	std::ostringstream o;
-	o<<i;
+	o << i;
 	return o.str();
 }
 
+
+/**
+ * Returns a string representing the real number (decimal) float value i
+ */
 inline std::string ftos(float f)
 {
 	std::ostringstream o;
-	o<<f;
+	o << f;
 	return o.str();
 }
 
-inline void str_replace(std::string & str, std::string const & pattern,
-		std::string const & replacement)
+
+/**
+ * Replace all occurrences of pattern in str with replacement
+ *
+ * @param str String to replace pattern with replacement within
+ * @param pattern The pattern to replace
+ * @param replacement What to replace the pattern with
+ */
+inline void str_replace(std::string &str, std::string const &pattern,
+		std::string const &replacement)
 {
 	std::string::size_type start = str.find(pattern, 0);
-	while(start != str.npos)
-	{
+	while (start != str.npos) {
 		str.replace(start, pattern.size(), replacement);
 		start = str.find(pattern, start+replacement.size());
 	}
 }
 
-inline void str_replace_char(std::string & str, char from, char to)
+
+/**
+ * Replace all occurrances of the character from in str with to.
+ *
+ * @param str The string to (potentially) modify
+ * @param from The character in str to replace
+ * @param to The replacement character
+ */
+inline void str_replace_char(std::string &str, char from, char to)
 {
-	for(unsigned int i=0; i<str.size(); i++)
-	{
-		if(str[i] == from)
+	for (size_t i = 0; i < str.size(); i++)
+		if (str[i] == from)
 			str[i] = to;
-	}
 }
 
-/*
-	Checks if a string contains only supplied characters
-*/
+
+/**
+ * Check that a string only contains whitelisted characters. This is the
+ * opposite of string_allowed_blacklist().
+ *
+ * @param s The string to be checked.
+ * @param allowed_chars A string containing permitted characters.
+ * @return true if the string is allowed, otherwise false.
+ *
+ * @see string_allowed_blacklist()
+ */
 inline bool string_allowed(const std::string &s, const std::string &allowed_chars)
 {
-	for(u32 i=0; i<s.size(); i++)
-	{
-		bool confirmed = false;
-		for(u32 j=0; j<allowed_chars.size(); j++)
-		{
-			if(s[i] == allowed_chars[j])
-			{
-				confirmed = true;
-				break;
-			}
-		}
-		if(confirmed == false)
+	for (size_t i = 0; i < s.size(); i++)
+		if (allowed_chars.find(s[i]) == std::string::npos)
 			return false;
-	}
+
 	return true;
 }
 
-/*
-	Checks if a string contains no blacklisted characters (opposite
-	function of string_allowed())
-*/
-inline bool string_allowed_blacklist(const std::string & s, const std::string & blacklisted_chars)
+
+/**
+ * Check that a string contains no blacklisted characters. This is the
+ * opposite of string_allowed().
+ *
+ * @param s The string to be checked.
+ * @param blacklisted_chars A string containing prohibited characters.
+ * @return true if the string is allowed, otherwise false.
+
+ * @see string_allowed()
+ */
+inline bool string_allowed_blacklist(const std::string &s,
+		const std::string &blacklisted_chars)
 {
-	for(unsigned int i = 0; i < s.length(); i++)
-	{
-		bool invalid = false;
-		for(unsigned int j = 0; j < blacklisted_chars.length(); j++)
-		{
-			if(s[i] == blacklisted_chars[j])
-			{
-				invalid = true;
-				break;
-			}
-		}
-		if(invalid)
+	for (size_t i = 0; i < s.size(); i++)
+		if (blacklisted_chars.find(s[i]) != std::string::npos)
 			return false;
-	}
+
 	return true;
 }
 
-/*
-	Forcefully wraps string into rows using \n
-	(no word wrap, used for showing paths in gui)
-*/
+
+/**
+ * Create a string based on 'from' where a newline is forcefully inserted every
+ * 'rowlen' characters.
+ *
+ * @note This function does not honour word wraps and blindy inserts a newline
+ *			every rowlen characters whether it breaks a word or not. It is
+ *			intended to be used, for example, showing paths in the GUI
+ *
+ * @param from The string to be wrapped into rows.
+ * @param rowlen The row length (in characters).
+ * @return A new string with the wrapping applied.
+ */
 inline std::string wrap_rows(const std::string &from, u32 rowlen)
 {
 	std::string to;
-	for(u32 i=0; i<from.size(); i++)
-	{
-		if(i != 0 && i%rowlen == 0)
+
+	for (size_t i = 0; i < from.size(); i++) {
+		if(i != 0 && i % rowlen == 0)
 			to += '\n';
 		to += from[i];
 	}
+
 	return to;
 }
 
-/*
-	Removes all \\ from a string that had been escaped (FormSpec strings)
-*/
+
+/**
+ * Removes all \\ from a string that had been escaped (FormSpec strings)
+ *
+ */
 inline std::string unescape_string(std::string &s)
 {
 	std::string res;
@@ -326,26 +470,33 @@ inline std::string unescape_string(std::string &s)
 	return res;
 }
 
-inline bool is_number(const std::string& tocheck)
+
+/**
+ * Checks that all characters in tocheck are a decimal digits
+ *
+ * @param tocheck
+ * @return true if tockcheck is not empty and all characters in tocheck are
+ *			decimal digits, otherwise false
+ */
+inline bool is_number(const std::string &tocheck)
 {
-	std::string::const_iterator iter = tocheck.begin();
+	for (size_t i = 0; i < tocheck.size(); i++)
+	    if (!isdigit(tocheck[i]))
+	        return false;
 
-	while (iter != tocheck.end() && std::isdigit(*iter)) {
-		++iter;
-	}
+	return !tocheck.empty();
+}
 
-	return ((!tocheck.empty()) && (iter == tocheck.end()));
+
+/**
+ * Returns a C-string, either "true" or "false", corresponding to v
+ *
+ * @return If v == true, then "true" is returned, otherwise "false"
+ */
+inline const char *bool_to_cstr(bool v)
+{
+	return v ? "true" : "false";
 }
 
-std::string translatePassword(std::string playername, std::wstring password);
-std::string urlencode(std::string str);
-std::string urldecode(std::string str);
-u32 readFlagString(std::string str, const FlagDesc *flagdesc, u32 *flagmask);
-std::string writeFlagString(u32 flags, const FlagDesc *flagdesc, u32 flagmask);
-size_t mystrlcpy(char *dst, const char *src, size_t size);
-char *mystrtok_r(char *s, const char *sep, char **lasts);
-u64 read_seed(const char *str);
-bool parseColorString(const std::string &value, video::SColor &color, bool quiet);
 
 #endif
-