AK: Move to_int(), to_uint() implementations to StringUtils (#1338)

Provide wrappers in String and StringView. Add some tests for the
implementations.

AK/String.cpp

@@ -189,45 +189,12 @@ ByteBuffer String::to_byte_buffer() const
 
 int String::to_int(bool& ok) const
 {
-    bool negative = false;
+    return StringUtils::convert_to_int(this->view(), ok);
-    int value = 0;
-    size_t i = 0;
-
-    if (is_empty()) {
-        ok = false;
-        return 0;
-    }
-
-    if (characters()[0] == '-') {
-        i++;
-        negative = true;
-    }
-    for (; i < length(); i++) {
-        if (characters()[i] < '0' || characters()[i] > '9') {
-            ok = false;
-            return 0;
-        }
-        value = value * 10;
-        value += characters()[i] - '0';
-    }
-    ok = true;
-
-    return negative ? -value : value;
 }
 
 unsigned String::to_uint(bool& ok) const
 {
-    unsigned value = 0;
+    return StringUtils::convert_to_uint(this->view(), ok);
-    for (size_t i = 0; i < length(); ++i) {
-        if (characters()[i] < '0' || characters()[i] > '9') {
-            ok = false;
-            return 0;
-        }
-        value = value * 10;
-        value += characters()[i] - '0';
-    }
-    ok = true;
-    return value;
 }
 
 String String::number(unsigned long long value)

AK/String.h

@@ -112,7 +112,6 @@ public:
     static String repeated(char, size_t count);
     bool matches(const StringView& mask, CaseSensitivity = CaseSensitivity::CaseInsensitive) const;
 
-    // FIXME: These should be shared between String and StringView somehow!
     int to_int(bool& ok) const;
     unsigned to_uint(bool& ok) const;
 

AK/StringUtils.cpp

@@ -59,6 +59,63 @@ namespace StringUtils {
         return (mask_ptr == mask_end) && string_ptr == string_end;
     }
 
+    int convert_to_int(const StringView& str, bool& ok)
+    {
+        if (str.is_empty()) {
+            ok = false;
+            return 0;
+        }
+
+        bool negative = false;
+        size_t i = 0;
+        const auto characters = str.characters_without_null_termination();
+
+        if (characters[0] == '-' || characters[0] == '+') {
+            if (str.length() == 1) {
+                ok = false;
+                return 0;
+            }
+            i++;
+            negative = (characters[0] == '-');
+        }
+
+        int value = 0;
+        for (; i < str.length(); i++) {
+            if (characters[i] < '0' || characters[i] > '9') {
+                ok = false;
+                return 0;
+            }
+            value = value * 10;
+            value += characters[i] - '0';
+        }
+        ok = true;
+
+        return negative ? -value : value;
+    }
+
+    unsigned convert_to_uint(const StringView& str, bool& ok)
+    {
+        if (str.is_empty()) {
+            ok = false;
+            return 0;
+        }
+
+        unsigned value = 0;
+        const auto characters = str.characters_without_null_termination();
+
+        for (size_t i = 0; i < str.length(); i++) {
+            if (characters[i] < '0' || characters[i] > '9') {
+                ok = false;
+                return 0;
+            }
+            value = value * 10;
+            value += characters[i] - '0';
+        }
+        ok = true;
+
+        return value;
+    }
+
 }
 
 }

AK/StringUtils.h

@@ -12,6 +12,8 @@ enum class CaseSensitivity {
 namespace StringUtils {
 
     bool matches(const StringView& str, const StringView& mask, CaseSensitivity = CaseSensitivity::CaseInsensitive);
+    int convert_to_int(const StringView&, bool& ok);
+    unsigned convert_to_uint(const StringView&, bool& ok);
 
 }
 

AK/StringView.cpp

@@ -174,45 +174,12 @@ StringView StringView::substring_view_starting_after_substring(const StringView&
 
 int StringView::to_int(bool& ok) const
 {
-    bool negative = false;
+    return StringUtils::convert_to_int(*this, ok);
-    int value = 0;
-    size_t i = 0;
-
-    if (is_empty()) {
-        ok = false;
-        return 0;
-    }
-
-    if (characters_without_null_termination()[0] == '-') {
-        i++;
-        negative = true;
-    }
-    for (; i < length(); i++) {
-        if (characters_without_null_termination()[i] < '0' || characters_without_null_termination()[i] > '9') {
-            ok = false;
-            return 0;
-        }
-        value = value * 10;
-        value += characters_without_null_termination()[i] - '0';
-    }
-    ok = true;
-
-    return negative ? -value : value;
 }
 
 unsigned StringView::to_uint(bool& ok) const
 {
-    unsigned value = 0;
+    return StringUtils::convert_to_uint(*this, ok);
-    for (size_t i = 0; i < length(); ++i) {
-        if (characters_without_null_termination()[i] < '0' || characters_without_null_termination()[i] > '9') {
-            ok = false;
-            return 0;
-        }
-        value = value * 10;
-        value += characters_without_null_termination()[i] - '0';
-    }
-    ok = true;
-    return value;
 }
 
 unsigned StringView::hash() const

AK/StringView.h

@@ -77,9 +77,8 @@ public:
     // following newline.".
     Vector<StringView> lines(bool consider_cr = true) const;
 
-    // FIXME: These should be shared between String and StringView somehow!
-    unsigned to_uint(bool& ok) const;
     int to_int(bool& ok) const;
+    unsigned to_uint(bool& ok) const;
 
     // Create a new substring view of this string view, starting either at the beginning of
     // the given substring view, or after its end, and continuing until the end of this string

AK/Tests/TestStringUtils.cpp

@@ -41,4 +41,81 @@ TEST_CASE(matches_case_insensitive)
     EXPECT(!AK::StringUtils::matches("acdcb", "a*c?b"));
 }
 
+TEST_CASE(convert_to_int)
+{
+    bool ok = false;
+    AK::StringUtils::convert_to_int(StringView(), ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_int("", ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_int("a", ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_int("+", ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_int("-", ok);
+    EXPECT(!ok);
+
+    int actual = actual = AK::StringUtils::convert_to_int("0", ok);
+    EXPECT(ok && actual == 0);
+
+    actual = AK::StringUtils::convert_to_int("1", ok);
+    EXPECT(ok && actual == 1);
+
+    actual = AK::StringUtils::convert_to_int("+1", ok);
+    EXPECT(ok && actual == 1);
+
+    actual = AK::StringUtils::convert_to_int("-1", ok);
+    EXPECT(ok && actual == -1);
+
+    actual = AK::StringUtils::convert_to_int("01", ok);
+    EXPECT(ok && actual == 1);
+
+    actual = AK::StringUtils::convert_to_int("12345", ok);
+    EXPECT(ok && actual == 12345);
+
+    actual = AK::StringUtils::convert_to_int("-12345", ok);
+    EXPECT(ok && actual == -12345);
+}
+
+TEST_CASE(convert_to_uint)
+{
+    bool ok = false;
+    AK::StringUtils::convert_to_uint(StringView(), ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_uint("", ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_uint("a", ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_uint("+", ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_uint("-", ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_uint("+1", ok);
+    EXPECT(!ok);
+
+    AK::StringUtils::convert_to_uint("-1", ok);
+    EXPECT(!ok);
+
+    uint actual = AK::StringUtils::convert_to_uint("0", ok);
+    EXPECT(ok && actual == 0u);
+
+    actual = AK::StringUtils::convert_to_uint("1", ok);
+    EXPECT(ok && actual == 1u);
+
+    actual = AK::StringUtils::convert_to_uint("01", ok);
+    EXPECT(ok && actual == 1u);
+
+    actual = AK::StringUtils::convert_to_uint("12345", ok);
+    EXPECT(ok && actual == 12345u);
+}
+
 TEST_MAIN(StringUtils)