With Extensions (Combine) #342

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

@@ -36,6 +36,10 @@
  <ItemGroup Condition="'$(TargetFramework)'=='net40'">
    <PackageReference Include="Microsoft.Bcl.Async" Version="1.0.168" />
  </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

@@ -36,6 +36,10 @@
  <ItemGroup Condition="'$(TargetFramework)'=='net40'">
    <PackageReference Include="Microsoft.Bcl.Async" Version="1.0.168" />
  </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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

With Extensions (Combine) #342

With Extensions (Combine) #342

SuperJMN commented Oct 11, 2021 •

edited

SuperJMN commented Oct 11, 2021

hankovich commented Nov 8, 2021

hankovich Nov 8, 2021

ProphetLamb Jul 6, 2022 •

edited

hankovich Nov 8, 2021

SuperJMN Nov 8, 2021

hankovich Nov 8, 2021

SuperJMN Nov 8, 2021

ProphetLamb Jul 6, 2022 •

edited

ProphetLamb Jul 6, 2022

ProphetLamb Jul 6, 2022 •

edited

ProphetLamb Jul 6, 2022

ProphetLamb Jul 6, 2022

ProphetLamb Jul 6, 2022

With Extensions (Combine) #342

Are you sure you want to change the base?

With Extensions (Combine) #342

Conversation

SuperJMN commented Oct 11, 2021 • edited

Example

SuperJMN commented Oct 11, 2021

hankovich commented Nov 8, 2021

Choose a reason for hiding this comment

ProphetLamb Jul 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ProphetLamb Jul 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ProphetLamb Jul 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuperJMN commented Oct 11, 2021 •

edited

ProphetLamb Jul 6, 2022 •

edited

ProphetLamb Jul 6, 2022 •

edited

ProphetLamb Jul 6, 2022 •

edited