szaszm commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r538697896



##########
File path: libminifi/include/utils/GeneralUtils.h
##########
@@ -49,6 +49,14 @@ constexpr T intdiv_ceil(T numerator, T denominator) {
       : numerator / denominator + (numerator % denominator != 0));
 }
 
+// from https://stackoverflow.com/questions/15202474
+struct identity {
+    template<typename U>
+    constexpr auto operator()(U&& v) const noexcept -> 
decltype(std::forward<U>(v)) {
+        return std::forward<U>(v);
+    }
+};
+

Review comment:
       Consider transparently falling back to 
[`std::identity`](https://en.cppreference.com/w/cpp/utility/functional/identity)
 when compiled on >=C++20

##########
File path: libminifi/src/utils/StringUtils.cpp
##########
@@ -59,13 +63,21 @@ std::vector<std::string> StringUtils::split(const 
std::string &str, const std::s
       break;
     }
     auto next = std::find_if(curr, end, is_func);
-    result.push_back(std::string(curr, next));
+    result.push_back(transformation(std::string(curr, next)));

Review comment:
       I would take `transformation` by value. You could also forward it to the 
call, but this doesn't make sense when it's called more than once, because the 
move case would call a moved-from function object.
   
   If the caller needs to keep state, they can use `std::ref` and pass by value.

##########
File path: libminifi/test/unit/StringUtilsTests.cpp
##########
@@ -50,6 +50,16 @@ TEST_CASE("TestStringUtils::split4", "[test split 
classname]") {
   REQUIRE(expected == 
StringUtils::split(org::apache::nifi::minifi::core::getClassName<org::apache::nifi::minifi::utils::StringUtils>(),
 "::"));
 }
 
+TEST_CASE("TestStringUtils::split5", "[test split delimiter not specified]") {

Review comment:
       What was the failure? Most languages split between each character on 
empty delimiter.
   
![2020-12-08-193418_485x47_scrot](https://user-images.githubusercontent.com/1170582/101526035-6c4b2200-398c-11eb-90df-b4bbc57196aa.png)
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to