You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/12/08 19:15:56 UTC

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

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:
users@infra.apache.org