Optimize formatTime utility (#3777)

Adds benchmarks and unit tests for the function

Co-authored-by: Rasmus Karlsson <rasmus.karlsson@pajlada.com>

benchmarks/CMakeLists.txt

@@ -3,6 +3,7 @@ project(chatterino-benchmark)
 set(benchmark_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/main.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/Emojis.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/src/FormatTime.cpp
     # Add your new file above this line!
     )
 

benchmarks/src/FormatTime.cpp

@@ -0,0 +1,38 @@
+#include "util/FormatTime.hpp"
+
+#include <benchmark/benchmark.h>
+
+using namespace chatterino;
+
+template <class... Args>
+void BM_TimeFormatting(benchmark::State &state, Args &&...args)
+{
+    auto args_tuple = std::make_tuple(std::move(args)...);
+    for (auto _ : state)
+    {
+        formatTime(std::get<0>(args_tuple));
+    }
+}
+
+BENCHMARK_CAPTURE(BM_TimeFormatting, 0, 0);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs0, "0");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 1337, 1337);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs1337, "1337");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 623452, 623452);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs623452, "623452");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 8345, 8345);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs8345, "8345");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 314034, 314034);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs314034, "314034");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 27, 27);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs27, "27");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 34589, 34589);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs34589, "34589");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 3659, 3659);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs3659, "3659");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 1045345, 1045345);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs1045345, "1045345");
+BENCHMARK_CAPTURE(BM_TimeFormatting, 86432, 86432);
+BENCHMARK_CAPTURE(BM_TimeFormatting, qs86432, "86432");
+BENCHMARK_CAPTURE(BM_TimeFormatting, qsempty, "");
+BENCHMARK_CAPTURE(BM_TimeFormatting, qsinvalid, "asd");

src/util/FormatTime.cpp

@@ -1,12 +1,19 @@
 #include "FormatTime.hpp"
 
 namespace chatterino {
+
 namespace {
-    void appendDuration(int count, QChar &&order, QString &outString)
+
+    void appendDuration(int count, QChar &&suffix, QString &out)
     {
-        outString.append(QString::number(count));
+        if (!out.isEmpty())
-        outString.append(order);
+        {
+            out.append(' ');
         }
+        out.append(QString::number(count));
+        out.append(suffix);
+    }
+
 }  // namespace
 
 QString formatTime(int totalSeconds)
@@ -25,26 +32,14 @@ QString formatTime(int totalSeconds)
     }
     if (hours > 0)
     {
-        if (!res.isEmpty())
-        {
-            res.append(" ");
-        }
         appendDuration(hours, 'h', res);
     }
     if (minutes > 0)
     {
-        if (!res.isEmpty())
-        {
-            res.append(" ");
-        }
         appendDuration(minutes, 'm', res);
     }
     if (seconds > 0)
     {
-        if (!res.isEmpty())
-        {
-            res.append(" ");
-        }
         appendDuration(seconds, 's', res);
     }
     return res;

tests/CMakeLists.txt

@@ -18,6 +18,7 @@ set(test_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/IrcHelpers.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/TwitchPubSubClient.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/TwitchMessageBuilder.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/src/FormatTime.cpp
     # Add your new file above this line!
     )
 

tests/src/FormatTime.cpp

@@ -0,0 +1,133 @@
+#include "util/FormatTime.hpp"
+
+#include <gtest/gtest.h>
+
+using namespace chatterino;
+
+TEST(FormatTime, Int)
+{
+    struct TestCase {
+        int input;
+        QString expectedOutput;
+    };
+
+    std::vector<TestCase> tests{
+        {
+            0,
+            "",
+        },
+        {
+            1337,
+            "22m 17s",
+        },
+        {
+            623452,
+            "7d 5h 10m 52s",
+        },
+        {
+            8345,
+            "2h 19m 5s",
+        },
+        {
+            314034,
+            "3d 15h 13m 54s",
+        },
+        {
+            27,
+            "27s",
+        },
+        {
+            34589,
+            "9h 36m 29s",
+        },
+        {
+            3659,
+            "1h 59s",
+        },
+        {
+            1045345,
+            "12d 2h 22m 25s",
+        },
+        {
+            86432,
+            "1d 32s",
+        },
+    };
+
+    for (const auto &[input, expected] : tests)
+    {
+        const auto actual = formatTime(input);
+
+        EXPECT_EQ(actual, expected)
+            << qUtf8Printable(actual) << " (" << input
+            << ") did not match expected value " << qUtf8Printable(expected);
+    }
+}
+
+TEST(FormatTime, QString)
+{
+    struct TestCase {
+        QString input;
+        QString expectedOutput;
+    };
+
+    std::vector<TestCase> tests{
+        {
+            "0",
+            "",
+        },
+        {
+            "1337",
+            "22m 17s",
+        },
+        {
+            "623452",
+            "7d 5h 10m 52s",
+        },
+        {
+            "8345",
+            "2h 19m 5s",
+        },
+        {
+            "314034",
+            "3d 15h 13m 54s",
+        },
+        {
+            "27",
+            "27s",
+        },
+        {
+            "34589",
+            "9h 36m 29s",
+        },
+        {
+            "3659",
+            "1h 59s",
+        },
+        {
+            "1045345",
+            "12d 2h 22m 25s",
+        },
+        {
+            "86432",
+            "1d 32s",
+        },
+        {
+            "",
+            "n/a",
+        },
+        {
+            "asd",
+            "n/a",
+        },
+    };
+
+    for (const auto &[input, expected] : tests)
+    {
+        const auto actual = formatTime(input);
+
+        EXPECT_EQ(actual, expected)
+            << qUtf8Printable(actual) << " (" << qUtf8Printable(input)
+            << ") did not match expected value " << qUtf8Printable(expected);
+    }
+}