lordgamez commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1153345313
##########
libminifi/include/core/TypedValues.h:
##########
@@ -110,20 +110,26 @@ class DataSizeValue : public TransformableValue, public
state::response::UInt64V
: state::response::UInt64Value(0) {
}
-
- // Convert String to Integer
- template<typename T, typename std::enable_if<
- std::is_integral<T>::value>::type* = nullptr>
- static bool StringToInt(const std::string &input, T &output) {
+ static std::optional<uint64_t> getUnitMultiplier(std::string unit_str) {
// TODO(adebreceni): this mapping is to preserve backwards compatibility,
// we should entertain the idea of moving to standardized units in
// the configuration (i.e. K = 1000, Ki = 1024)
static std::map<std::string, int64_t> unit_map{
- {"B", 1},
- {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
- {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB",
1_PiB},
+ {"B", 1},
+ {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
+ {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB",
1_PiB},
};
+ std::transform(unit_str.begin(), unit_str.end(), unit_str.begin(),
::toupper);
+ auto unit_multiplier_it = unit_map.find(unit_str);
+ if (unit_multiplier_it == unit_map.end())
+ return std::nullopt;
+ return unit_multiplier_it->second;
Review Comment:
This may be a bit shorter:
```
return unit_map.contains(unit_str) ? unit_map[unit_str] : std::nullopt;
```
##########
libminifi/test/unit/StringUtilsTests.cpp:
##########
@@ -536,4 +536,25 @@ TEST_CASE("StringUtils::matchesSequence works correctly",
"[matchesSequence]") {
REQUIRE(StringUtils::matchesSequence("xxxabcxxxabcxxxdefxxx", {"abc", "abc",
"def"}));
REQUIRE(!StringUtils::matchesSequence("xxxabcxxxdefxxx", {"abc", "abc",
"def"}));
}
+
+TEST_CASE("StringUtils::splitToUnitAndValue tests") {
Review Comment:
I would add a test case like `1KiB` that is without spaces. It would
distinguish this from another solution that would just split the input on the
spaces and convert the first part to an integer.
I would also add a test case for invalid cases like
- an empty string
- a non-decimal number like `017 bananas`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
