From 0ad66c0af46f5f5163845068de3756177800f354 Mon Sep 17 00:00:00 2001 From: Kasia Date: Sun, 29 May 2022 13:54:42 +0200 Subject: [PATCH] Optimize formatTime utility (#3777) Adds benchmarks and unit tests for the function Co-authored-by: Rasmus Karlsson --- benchmarks/CMakeLists.txt | 1 + benchmarks/src/FormatTime.cpp | 38 ++++++++++ src/util/FormatTime.cpp | 25 +++---- tests/CMakeLists.txt | 1 + tests/src/FormatTime.cpp | 133 ++++++++++++++++++++++++++++++++++ 5 files changed, 183 insertions(+), 15 deletions(-) create mode 100644 benchmarks/src/FormatTime.cpp create mode 100644 tests/src/FormatTime.cpp diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 6435f139..b9fd4a9f 100644 --- a/benchmarks/CMakeLists.txt +++ b/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! ) diff --git a/benchmarks/src/FormatTime.cpp b/benchmarks/src/FormatTime.cpp new file mode 100644 index 00000000..cf63e4ca --- /dev/null +++ b/benchmarks/src/FormatTime.cpp @@ -0,0 +1,38 @@ +#include "util/FormatTime.hpp" + +#include + +using namespace chatterino; + +template +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"); diff --git a/src/util/FormatTime.cpp b/src/util/FormatTime.cpp index ba66031b..6d6f2e52 100644 --- a/src/util/FormatTime.cpp +++ b/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)); - outString.append(order); + if (!out.isEmpty()) + { + 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; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 58db955e..e065dd3a 100644 --- a/tests/CMakeLists.txt +++ b/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! ) diff --git a/tests/src/FormatTime.cpp b/tests/src/FormatTime.cpp new file mode 100644 index 00000000..3c4b5e8c --- /dev/null +++ b/tests/src/FormatTime.cpp @@ -0,0 +1,133 @@ +#include "util/FormatTime.hpp" + +#include + +using namespace chatterino; + +TEST(FormatTime, Int) +{ + struct TestCase { + int input; + QString expectedOutput; + }; + + std::vector 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 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); + } +}