Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] Results.Merge() where TValue is a collection #182

Open
forrestab opened this issue Apr 19, 2023 · 5 comments · May be fixed by #215
Open

[Question] Results.Merge() where TValue is a collection #182

forrestab opened this issue Apr 19, 2023 · 5 comments · May be fixed by #215

Comments

@forrestab
Copy link

I have a requirement to merge multiple results of type Result<IEnumerable<T>>. Here is an example of what Im trying to do:

Result<IEnumerable<SomeDto>> result = Result.Ok(Enumerable.Empty<SomeDto>());

if (someCondition)
{
    Result<IEnumerable<SomeDto>> dbResult = await CallSomeDbFunc();
    result = Result.Merge(result, dbResult);
}

if (result.IsSuccess && someCondition)
{
    // run another db function and merge with `result`
}

With the above code, Result.Merge is returning Result<IEnumerable<IEnumerable<SomeDto>>>>. What I would rather it return is the input's result type.

In order to achieve what I want, Im using ToResult to "unwrap" the return from Result.Merge.

Result.Merge(result, dbResult).ToResult(result => result.SelectMany(value => value));

Is this the recommended way or have I missed a method signature somewhere?

@Kysluss
Copy link
Contributor

Kysluss commented Jul 14, 2024

I'm interested in implementing this. One question I have is should this be a different name than Merge? Reason being once implemented and you go to merge the collections, you get an error saying that Merge is ambiguous between Merge<TValue> and Merge<IEnumerable<TValue>>. To get around the ambiguity, the method could be called something else, or you can specify the result type when calling Merge. Thinking this may be a small price to pay for keeping the public API clean.

@altmann what are your thoughts? Below is my proposal. Happy to submit a PR for this and will update docs.

var result1 = new string[] { "A", "B", "C" };
var result2 = new string[] { "D", "E", "F" };
var result3 = new string[] { "G", "H", "I" };

// Have to specify result type so compiler knows this is the IEnumerable version
var result = Result.Merge<string>(result1, result2, result3);

Console.WriteLine(String.Join(",", result.Value));

var result4 = Result.Ok("J");
var result5 = Result.Ok("K");
var result6 = Result.Ok("L");

// Do not have to specify result type
var result7 = Result.Merge(result4, result5, result6);

Console.WriteLine(String.Join(",", result7.Value));

@altmann
Copy link
Owner

altmann commented Jul 29, 2024

@Kysluss Thank you for your preparation. I think I would call the new method with a new name. Maybe MergeFlat() or only Flat(). What do you think?

If I see it correctly the first three variables (result1, result2 and result3) are not real reasults, only array of strings. I think you mean Result.Ok(new string[] { ... });

@Kysluss
Copy link
Contributor

Kysluss commented Jul 30, 2024

Thanks @altmann

You are right, I was taking advantage of the implicit conversion for ease of creating a demo. I think MergeFlat might a good name because it helps explicitly state the intention of the method. I'll work on this and should have a PR either today or tomorrow for you.

@Kysluss
Copy link
Contributor

Kysluss commented Jul 30, 2024

Okay, so slight complication on this one. I'm running into the error we discussed in #201 where you can't convert between things based on interface type. It might be a little bit before I have that PR ready, but will keep working on it.

@Kysluss Kysluss linked a pull request Jul 31, 2024 that will close this issue
@Kysluss
Copy link
Contributor

Kysluss commented Jul 31, 2024

@altmann I have added #215 with a MergeFlat method and documentation. Because of the discussion in #201, the method signature requires TValue and TArray to be able to correctly infer the type parameters. For some reason, the compiler doesn't pick them up. Below is an example usage.

var result1 = Result.Ok(new string[] { "A", "B" });
var result2 = Result.Ok(new string[] { "C", "D" });
var result3 = Result.Ok(new string[] { "E", "F" });

// Will contain ["A", "B", "C", "D", "E", "F"]
var mergedResult = Result.MergeFlat<string, string[]>(result1, result2, result3);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants