Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
RobinTF committed Feb 1, 2024
1 parent ceeef50 commit 2c62035
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 11 deletions.
20 changes: 11 additions & 9 deletions src/util/MemorySize/MemorySize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <absl/strings/charconv.h>
#include <absl/strings/str_cat.h>

#include <cctype>
#include <charconv>
#include <ctre-unicode.hpp>
#include <string_view>
Expand Down Expand Up @@ -59,30 +60,31 @@ std::string MemorySize::asString() const {

// _____________________________________________________________________________
MemorySize MemorySize::parse(std::string_view str) {
if (auto matcher = ctre::match<
"(?<amount>\\d+(?:\\.\\d+)?) ?(?<unit>[kKmMgGtT][bB]?|[bB])">(str)) {
constexpr ctll::fixed_string regex =
"(?<amount>\\d+(?:\\.\\d+)?)\\s*(?<unit>[kKmMgGtT][bB]?|[bB])";
if (auto matcher = ctre::match<regex>(str)) {
auto amountString = matcher.get<"amount">().to_view();
// Versions after CTRE v3.8.1 should support to_number()
// with double values if the compilers support it.
double amount;
absl::from_chars(amountString.begin(), amountString.end(), amount);
auto unitString = matcher.get<"unit">().to_view();
switch (unitString.at(0)) {
switch (std::tolower(unitString.at(0))) {
case 'b':
case 'B':
if (ad_utility::contains(amountString, '.')) {
break;
throw std::runtime_error(absl::StrCat(
"'", str,
"' could not be parsed as a memory size. When using bytes as "
"units only unsigned integers are allowed."));
}
return MemorySize::bytes(static_cast<size_t>(amount));
case 'k':
case 'K':
return MemorySize::kilobytes(amount);
case 'm':
case 'M':
return MemorySize::megabytes(amount);
case 'g':
case 'G':
return MemorySize::gigabytes(amount);
case 't':
case 'T':
return MemorySize::terabytes(amount);
default:
// Whatever this is, it is false.
Expand Down
10 changes: 8 additions & 2 deletions test/MemorySizeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ TEST(MemorySize, Parse) {
doExceptionTest("4.2 B");
doExceptionTest("-4.2 B");

// Nothing should work with negativ numbers.
// Nothing should work with negative numbers.
std::ranges::for_each(generalAsStringTestCases(), doExceptionTest,
[](const MemorySizeAndStringRepresentation& testCase) {
return absl::StrCat("-",
Expand Down Expand Up @@ -288,7 +288,7 @@ TEST(MemorySize, Parse) {
{42_TB, "42 t"}},
doTest);

// Check if string is truly optional
// Check if whitespace between unit and amount is truly optional
std::ranges::for_each(
std::vector<MemorySizeAndStringRepresentation>{{42_B, "42B"},
{42_B, "42b"},
Expand Down Expand Up @@ -321,6 +321,12 @@ TEST(MemorySize, Parse) {
{42_TB, "42t"}},
doTest);

// Test if multiple spaces are fine too
std::ranges::for_each(
std::vector<MemorySizeAndStringRepresentation>{{42_kB, "42 K"},
{42_kB, "42 k"}},
doTest);

// We only take memory units up to `TB`. Not further.
std::ranges::for_each(std::vector{"42 P", "42 PB"}, doExceptionTest);
}
Expand Down

0 comments on commit 2c62035

Please sign in to comment.