fgerlits commented on code in PR #1659:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1659#discussion_r1328928477
##########
libminifi/include/utils/expected.h:
##########
@@ -31,28 +29,42 @@ inline constexpr bool is_expected_v = false;
template<typename V, typename E>
inline constexpr bool is_expected_v<nonstd::expected<V, E>> = true;
-// map implementation
-template<typename Expected, typename F, typename =
std::enable_if_t<is_expected_v<utils::remove_cvref_t<Expected>>>>
-auto operator|(Expected&& object, map_wrapper<F> f) {
- using value_type = typename utils::remove_cvref_t<Expected>::value_type;
- using error_type = typename utils::remove_cvref_t<Expected>::error_type;
- if constexpr (std::is_same_v<value_type, void>) {
- using function_return_type =
std::decay_t<decltype(std::invoke(std::forward<F>(f.function)))>;
+template<typename T>
+concept expected = is_expected_v<std::remove_cvref_t<T>>;
+
+template<typename T>
+inline constexpr bool is_valid_unexpected_type_v = std::is_same_v<T,
std::remove_cvref_t<T>> && std::is_object_v<T> && !std::is_array_v<T>;
+
+template<typename T>
+inline constexpr bool is_valid_unexpected_type_v<nonstd::unexpected_type<T>> =
false;
Review Comment:
This `T` is different from the `T` parameters in lines 35 and 41. Can we
rename it to something else, eg. `E`, please?
##########
libminifi/test/unit/ExpectedTest.cpp:
##########
@@ -484,3 +484,63 @@ TEST_CASE("expected valueOrElse",
"[expected][valueOrElse]") {
REQUIRE_THROWS_AS(ex | utils::valueOrElse([](const std::string&) -> int {
throw std::exception(); }), std::exception);
REQUIRE_THROWS_AS(std::move(ex) | utils::valueOrElse([](std::string&&) ->
int { throw std::exception(); }), std::exception);
}
+
+TEST_CASE("expected transformError", "[expected][transformError]") {
+ auto mul2 = [](int a) { return a * 2; };
+
+ {
+ nonstd::expected<int, int> e = nonstd::make_unexpected(21);
+ auto ret = e | utils::transformError(mul2);
+ REQUIRE(!ret);
+ REQUIRE(ret.error() == 42);
+ }
+
+ {
+ const nonstd::expected<int, int> e = nonstd::make_unexpected(21);
+ auto ret = e | utils::transformError(mul2);
+ REQUIRE(!ret);
+ REQUIRE(ret.error() == 42);
+ }
+
+ {
+ nonstd::expected<int, int> e = nonstd::make_unexpected(21);
+ auto ret = std::move(e) | utils::transformError(mul2);
+ REQUIRE(!ret);
+ REQUIRE(ret.error() == 42);
+ }
+
+ {
+ const nonstd::expected<int, int> e = nonstd::make_unexpected(21);
+ auto ret = std::move(e) | utils::transformError(mul2); //
NOLINT(performance-move-const-arg)
+ REQUIRE(!ret);
+ REQUIRE(ret.error() == 42);
+ }
Review Comment:
I think `clang-tidy` has a point here: this test doesn't make much sense,
since we can't move from a `const` variable. I would remove this test case,
and the others marked with `NOLINT(performance-move-const-arg)`, too. Maybe
they could be replaced with more realistic tests where the `expected` comes
from the return value of a function.
##########
libminifi/include/utils/expected.h:
##########
@@ -61,56 +73,54 @@ auto operator|(Expected&& object, map_wrapper<F> f) {
}
}
-// flatMap
-template<typename Expected, typename F, typename =
std::enable_if_t<is_expected_v<utils::remove_cvref_t<Expected>>>>
-auto operator|(Expected&& object, flat_map_wrapper<F> f) {
- using value_type = typename utils::remove_cvref_t<Expected>::value_type;
- if constexpr (std::is_same_v<value_type, void>) {
- using function_return_type =
std::decay_t<decltype(std::invoke(std::forward<F>(f.function)))>;
+// andThen
+template<expected Expected, typename F>
+auto operator|(Expected&& object, and_then_wrapper<F> f) {
+ using value_type = typename std::remove_cvref_t<Expected>::value_type;
+ if constexpr (!std::is_void_v<value_type>) {
Review Comment:
Why did you flip this? Before, we always dealt with the `void` case first
and the non-`void` case second. Now, it is the other way round in this one
instance.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
