With Extensions (Combine) by SuperJMN · Pull Request #342 · vkhorikov/CSharpFunctionalExtensions

SuperJMN · 2021-10-11T09:10:54Z

These extensions will allow you combine Results.
This is done by combining successful values using a factory + merging errors using a factory

Example

var r1 = Result.Success(1);
var r2 = Result.Success(3);
var combined = r1.With(r2, (x, y) => x + y);

combined will contain a success with a value of 4

SuperJMN · 2021-10-11T09:12:10Z

I removed the tests because I still have to think about a good way to test every overload without making a mess.

hankovich · 2021-11-08T11:09:09Z

Personally I feel it's better to use SelectMany via the fluent syntax (which I hate and it's the single case where I'm ok to use it) for such complex cases.

And, speaking honestly, I think those extensions do too much. It's hard to understand all those positional and generic parameters.

Example from tests:

await r1.WithMap(r2, (a, b) => Task.FromResult(a + b), (e1, e2) => "failure"); // I don't feel it's simple and easy to read/understand

hankovich · 2021-11-08T10:58:51Z

CSharpFunctionalExtensions/CSharpFunctionalExtensions.csproj

  </ItemGroup>
+
+  <ItemGroup Condition="$(TargetFramework.StartsWith('net4')) OR $(TargetFramework)=='netstandard2.0'">
+    <PackageReference Include="System.ValueTuple" Version="4.*" />


System.ValueTuple is shipped with the legacy framework starting from 4.7 and with netstandard starting from 2.0.

Comparing TargetFramework is considered leaky, for various good reasons, such as Mono RT, and "oh I missed a version", additionally we are not interested in the TargetFramework at all, but in its features.
Preprocessor directives express features.

Instead, consider comparing using $(DefineConstants.Contains('NET5_0_OR_GREATER')) which are supported by 3rd party runtimes and also well documented.

hankovich · 2021-11-08T10:59:55Z

CSharpFunctionalExtensions/Result/Extensions/BindError.cs

+{
+    public partial class ResultExtensions
+    {
+        public static Result<T, E2> BindError<T, E, E2>(this Result<T, E> self,


Doesn't OnFailureCompensate cover such a case?

I don't think so. OnFailureCompensate expects the same error type :) This maps from E, to E2

hankovich · 2021-11-08T11:01:11Z

CSharpFunctionalExtensions/Result/Extensions/With/AsyncResultOfT.cs

+{
+    public static partial class ResultExtensions
+    {
+        public static Task<Result<TResult>> WithMap<T1, T2, TResult>(this Result<T1> a,


I feel it can be expressed via SelectMany

If you're able to give an example, I'll be good with that :D

ProphetLamb · 2022-07-06T15:27:22Z

CSharpFunctionalExtensions/CSharpFunctionalExtensions.csproj

  </ItemGroup>
+
+  <ItemGroup Condition="$(TargetFramework.StartsWith('net4')) OR $(TargetFramework)=='netstandard2.0'">
+    <PackageReference Include="System.ValueTuple" Version="4.*" />


Comparing TargetFramework is considered leaky, for various good reasons, such as Mono RT, and "oh I missed a version", additionally we are not interested in the TargetFramework at all, but in its features.
Preprocessor directives express features.

Instead, consider comparing using $(DefineConstants.Contains('NET5_0_OR_GREATER')) which are supported by 3rd party runtimes and also well documented.

ProphetLamb · 2022-07-06T15:33:44Z

CSharpFunctionalExtensions/Result/Extensions/With/AsyncResultOfT.cs

+        {
+            var mapSuccess =
+                a.BindError(el1 => b
+                        .MapError(el2 => string.Join(Result.ErrorMessagesSeparator, el1, el2))


Use Errors.Join

ProphetLamb · 2022-07-06T15:35:38Z

CSharpFunctionalExtensions/Result/Extensions/With/Errors.cs

+    {
+        public static string Join(string e1, string e2)
+        {
+            return string.Join(Result.ErrorMessagesSeparator, e1, e2);


Use string interpolation $"{e1}{Result.ErrorMessagesSeparator}{e2}"

Would be sensible to wait for #378 with the error combining

ProphetLamb · 2022-07-06T15:40:17Z

CSharpFunctionalExtensions/Result/Extensions/With/ResultOfT.cs

+{
+    public static partial class ResultExtensions
+    {
+        public static Result<(T1, T2)> With<T1, T2>(this Result<T1> a, Result<T2> b)


Declare argument covariance in this & in for every non TAP, non closure usages accepting structs.

ProphetLamb · 2022-07-06T15:42:05Z

CSharpFunctionalExtensions/Result/Extensions/With/ResultOfT.cs

+    {
+        public static Result<(T1, T2)> With<T1, T2>(this Result<T1> a, Result<T2> b)
+        {
+            return a.WithMap(b, (x, y) => (x, y));


Use static () {} lambda in all instances where no closure allocation is required.

SuperJMN force-pushed the master branch from 80ab5ad to 7a5dbe1 Compare November 7, 2021 15:15

With (Combine)

d443c4d

SuperJMN force-pushed the master branch from 7a5dbe1 to d443c4d Compare November 7, 2021 21:11

SuperJMN marked this pull request as ready for review November 7, 2021 21:12

hankovich reviewed Nov 8, 2021

View reviewed changes

ProphetLamb reviewed Jul 6, 2022

View reviewed changes

macpak mentioned this pull request Jul 16, 2022

Result.Combine question #424

Open

Conversation

SuperJMN commented Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example

Uh oh!

SuperJMN commented Oct 11, 2021

Uh oh!

hankovich commented Nov 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ProphetLamb Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ProphetLamb Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ProphetLamb Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SuperJMN commented Oct 11, 2021 •

edited

Loading

ProphetLamb Jul 6, 2022 •

edited

Loading

ProphetLamb Jul 6, 2022 •

edited

Loading

ProphetLamb Jul 6, 2022 •

edited

Loading