feat: Add monadic functions to Result
#3957
Conversation
WalkthroughEnhancements to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
one last question @stephenswat, what is a monad? |
A monad is just a monoid in the category of endofunctors, what's the problem? More seriously, we need to understand a few concepts first. A monoid is a set A category is a set of objects connected via morphisms. In category theory there are many categories, but in programming we usually only deal with the category known as Type which contains all types. The morphisms in the category Type are functions. A functor is something that maps objects and morphisms from one category onto another. An example of a functor is Now, this is where we can start talking about the operators in this PR. Note that Now we know what a functor is, what an endofunctor is, and what a monoid is. Now we can talk about monads. A monad is simply a functor (like Now, a monad is simply a functor equipped with the ability to "strip away" those unnecessary layers, i.e. to collapse the double wrapping of itself. This function is called std::optional<T> join(std::optional<std::optional<T>> v) {
if (v.has_value()) {
return *v;
} else {
return std::nullopt;
}
}Using join we can create the std::optional<int> murderIfNotEven(int i) {
if (i % 2 == 0) {
return {i};
} else {
return std::nullopt;
}
}
std::optional<double> safeSqrt(int i) {
if (i >= 0) {
return std::sqrt(static_cast<double>(i));
} else {
return std::nullopt;
}
}
...
// returns std::nullopt, because one of the operations "fails"
std::optional<double> a = murderIfNotEven(5).and_then(safeSqrt);
// also returns std::nullopt
std::optional<double> b = murderIfNotEven(-2).and_then(safeSqrt);
// returns 1.41
std::optional<double> c = murderIfNotEven(2).and_then(safeSqrt);Note that |
stephenswat
force-pushed
the
feat/result_monad
branch
from
093fa86 to
8310bf3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
Core/include/Acts/Utilities/Result.hpp (2)
178-212: Correct implementation of value_or, hmm yes.Handle references and move semantics properly, this implementation does. A small documentation improvement suggest I must:
-///e@param[in] v The default value to use if no valid value exists. +/// @param[in] v The default value to use if no valid value exists.
258-314: Strong with the Force, this monadic implementation is.Follow the monad laws, it does. But improve the error message, we can:
- "bind must take a callable with the same error type"); + "bind must take a callable returning Result with the same error type");Tests/UnitTests/Core/Utilities/ResultTests.cpp (1)
371-389: More test cases, we should add.Good start this is, but strengthen the tests with these cases, we must:
- Test transformation that changes type (e.g., int to string)
- Test with throwing transformations
- Test with transformations that return references
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
Core/include/Acts/Utilities/Result.hpp(2 hunks)Tests/UnitTests/Core/Utilities/ResultTests.cpp(1 hunks)
🔇 Additional comments (3)
Core/include/Acts/Utilities/Result.hpp (1)
214-256: Wise implementation of transform, this is.
Follow the functor laws, it does. Handle both lvalue and rvalue cases correctly, it does. Type safety through concepts, it maintains.
Tests/UnitTests/Core/Utilities/ResultTests.cpp (2)
336-369: Thorough test coverage, you have achieved.
Test both success and failure paths, you do. Verify reference semantics correctly, you have. Miss no edge cases, you did.
391-423: Complete and balanced, these tests are.
Test all paths of the monad, you do. Handle errors with wisdom, you have. Verify type changes correctly, you did.
stephenswat
force-pushed
the
feat/result_monad
branch
from
8310bf3 to
1ee5965
Compare
In order to make the `Result` type easier to use, this commit adds three new functions: * `Result::value_or` allows the user to obtain the value or a provided default value. * `Result::transform` models functorial mapping, allowing users to modify values inside results. * `Result::and_then` models monadic binding, allowing users to build complex chains of actions on results. Implemented are lvalue and rvalue versions of these functions as well as tests.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
Core/include/Acts/Utilities/Result.hpp (1)
258-314: Powerful monadic binding, implemented you have!Follow the sacred monad laws, your implementation does:
- Left identity: return a >>= f ≡ f a
- Right identity: m >>= return ≡ m
- Associativity: (m >>= f) >>= g ≡ m >>= (\x -> f x >>= g)
Yet, improve the error message in static_assert, we could.
- "bind must take a callable with the same error type"); + "and_then requires callable returning Result with matching error type");Tests/UnitTests/Core/Utilities/ResultTests.cpp (1)
371-389: More test cases, add we should.Good foundation laid, you have. Yet enhance we could with:
- Complex transformations testing
- Chain of transformations
- Type-changing transformations
// Add these test cases auto f2 = [](int x) { return std::to_string(x); }; // type-changing auto f3 = [](int x) { return x < 0 ? Result::failure(MyError::Failure) : Result::success(x); }; // error-producing BOOST_CHECK(Result::success(5).transform(f1).transform(f2).ok()); BOOST_CHECK_EQUAL(Result::success(5).transform(f1).transform(f2).value(), "10");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
Core/include/Acts/Utilities/Result.hpp(2 hunks)Tests/UnitTests/Core/Utilities/ResultTests.cpp(1 hunks)
🔇 Additional comments (4)
Core/include/Acts/Utilities/Result.hpp (2)
178-212: Wise implementation of value_or, it is! Hmmmm.
Handle both lvalue and rvalue cases correctly, you do. Type safety through requires-clause, maintain you have. Documentation, thorough it is.
214-256: Strong with the Force, this transform implementation is!
Follow the ancient functor laws, you do:
- Identity preservation: f(x) -> x, maintain you do
- Composition: g(f(x)) -> (g ∘ f)(x), respect you have
- Error propagation: handle with grace, you do
Well implemented, this is!
Tests/UnitTests/Core/Utilities/ResultTests.cpp (2)
336-369: Thorough tests for value_or, written you have!
Cover all cases, your tests do:
- Success and failure paths
- Reference handling
- Value preservation
391-423: Well tested, the monadic binding is!
Cover the essential cases, your tests do:
- Success and failure paths
- Error propagation
- Basic chaining
|
What on earth is this CI failure? 🤔 |
|
never seen this before. maybe @benjaminhuth @paulgessinger ? |
|
Should be solved by #3967. |
|
acts-project-service
added
the
Breaks Athena build
In order to make the
Resulttype easier to use, this commit adds three new functions:Result::value_orallows the user to obtain the value or a provided default value.Result::transformmodels functorial mapping, allowing users to modify values inside results.Result::and_thenmodels monadic binding, allowing users to build complex chains of actions on results.Implemented are lvalue and rvalue versions of these functions as well as tests.
Summary by CodeRabbit
New Features
Resultclass with new methods for improved error handling and value transformation:value_or: Retrieve valid value or default.transform: Apply a function to the valid value.and_then: Chain functions based on the result's validity.Tests
Resultclass, ensuring reliability and correctness:ValueOrResult: Validates thevalue_ormethod.TransformResult: Assesses thetransformmethod.AndThenResult: Evaluates theand_thenmethod.