Result<E> implementation

[bitbehz]

Hi,
Is there any plans for implementing Result class?
(Result with no value but error class)

[ReneWiersma]

I ran into this as well. It's holding me back from using the package in my production software.

[ngoov]

One more vote, I have solved this currently with 2 extension methods, to serialize an array of errors as json and back...

[hankovich]

Usually we add something like

public readonly struct Unit
{
    public static Unit Instance => default;
}

to represent a missing value and use Result<Unit, Error> for cases you've mentioned.

[space-alien]

A while back I created a lightweight Result library - initially as a clean-slate experimental/sandbox project, with the goal of feeding back my experiences here. I wanted to see how the codebase could evolve without the need for backwards compatibility, and it was much easier to try new things without every change breaking the large existing codebase.

I ended up implementing:
UnitResult<E> (the one we are missing here)
Result<T, E>

Plus, for convenience, the string-error types:
UnitResult (equivalent to Result here)
Result<T>

In this way, client code benefits from simpler method definitions (rather than Result<Unit, MyError>). I had no need to implement a new Unit type.

The naming rationale: UnitResult indicates there is no return value. The other option was Result/ValueResult. I find most methods have a return value, hence the preference for Result to be the type with a value. However, Result/ValueResult is perhaps more "obvious"...

I really want to backport my work here, but other commitments have prevented this. In any case, I have only implemented the bare minimum functionality to meet my project requirements, so only a fraction of all the methods.

Assuming the name UnitResult<E> is desirable (of course we can't use Result<E>), this raises a semantic conflict as the existing Result type does not hold a value. Obviously, it needs to remain for backwards compatibility. This is probably not such a big deal for existing projects/users, but I think maybe less desirable in the case of new projects, where it would be nice to push users towards consistent naming.

The other backwards-compatible option is to implement Unit and continue to advise users to use Result<Unit, CustomError> where needed. No need to implement new extension methods. Frankly, I'm not even sure of the value of providing a Unit implementation in this case. It may be better to simply provide sample code as part of the documentation.

[vkhorikov]

I like the idea of using UnitResult<E> instead of Result<Unit, E> as it's less verbose. It's more work from the development perspective, but that's what libraries are for -- to shield the users from unnecessary complexity. @hankovich what are your thoughts on this? Looks like you've been using Unit for a long time.

Assuming the name UnitResult<E> is desirable (of course we can't use Result<E>), this raises a semantic conflict as the existing Result type does not hold a value.

Why does the existing Result type not hold value? Looks like a useful shortcut for UnitResult<string>.

[space-alien]

Why does the existing Result type not hold value? Looks like a useful shortcut for UnitResult<string>.

No, no, I said: "the existing Result type does not hold a value" 😂

[vkhorikov]

Oh, makes sense, I thought it was a typo :) So what would be an alternative to Result then? UnitResult?

[vkhorikov]

@space-alien Could you please make the lightweight Result library public? I will work on backporting it here. It's time to introduce Result<E> in one way or another, its absence has prevented myself from using CSharpFunctionalExtensions too in some of my projects.

[space-alien]

Apologies for the slow response; I am travelling at the moment so not a lot of time for detailed replies.

Implementing UnitResult<E> is a major undertaking, because of the number of factory/extension methods and unit tests that need to be added to provide parity with the existing types.

It will also significantly increase the maintenance and documentation burden of the library.

Its addition will also increase complexity for users (e.g. navigating IntelliSense, in particular where UnitResult<E> interacts with Result<T, E> in method signatures.

So this is quite a big thing, and before embarking upon it I think we should make sure it's the right way forward (and if so, that the name is chosen correctly).

So what would be an alternative to Result then? UnitResult?

Yes, but I don't think this should be added to the current codebase (deprecating Result would not be pleasant!)

Could you please make the lightweight Result library public?

I'll try to publish/share it soon.

[space-alien]

Didn't mean that to sound negative by the way.. I agree that this is a gap, just keen to make sure we get it right :)

[vkhorikov]

Yeah, I don't know what the right is either. And the fact that I haven't used the library's Result in my last couple projects doesn't help it. @hankovich If you could share your opinion on the topic of Result<Unit, Error> vs UnitResult<Error>, that would be great.

In any case, I can start working on backporting it, and it should become clearer whether this is the right way forward as I go along.

[hankovich]

@vkhorikov yeap, I've posted some of my thoughts here and will duplicate them here as well.
I feel like we should make UnitResult<TError> a first-class citizen and provide some handy overloads for it.

Example:

resultUnitError.Map(unit => 42) // this call does not look clean enough for me since we can't do anything with unit anyway
unitResultError.Map(() => 42) // looks clean indeed

Nobody says we should add both UnitResult and all extensions/overloads for it at the same time, it will be incremental work. Since we came up to this problem 4 years after the 'launch' of the library, I don't expect a lots of users will use Unit/UnitResult.

But I want to see handy extensions for those who will (like me).

Does this make sense to you guys?

[space-alien]

Yes, I agree in principle, especially with the incremental approach.

Before starting, I think we should plan the work carefully and perhaps add a tracking issue with a list work items so that everyone can see the status of the implementation.

In the meantime, I have pushed a new branch with an implementation UnitResult<E> that is consistent with the existing Result<T, E>.

https://github.com/vkhorikov/CSharpFunctionalExtensions/blob/unitresult/CSharpFunctionalExtensions/Result/UnitResultE.cs

[hankovich]

@space-alien at that time I see this draft PR was updated by @c17r and it now contains its own implementation of UnitResult 😊

[space-alien]

@hankovich @c17r Ah right, I didn't review the code since the discussion on the PR changed direction away from interfaces.

I will take a look.

[vkhorikov]

I also agree, and I like the idea with the incremental approach. I also like the idea to come up with a specific work plan and a tracking issue. I guess GitHub's Projects tab would be a good fit here. Any ideas of how to best organize it?

[Chris197]

Are there any updates to this? We have started using this package in our product with a custom error interface IMyError, and using the Result<Unit,IMyError> workaround, but previously clean statements all become quite a bit more convoluted now. Where previously return Result.Success() was sufficient, I now need to write return Result.Success<Unit, IMyError>(default). Not a big problem, but it does make an otherwise elegant approach loose its shine a bit..

[vkhorikov]

@Chris197 This is still in the plans. I plan to do a bare-bones version within the next month or two (with basic extension methods) and hopefully people will be able to fill in missing extension methods eventually.

[vkhorikov]

@Chris197 Could you share your approach with the IMyError interface? Seeing the existing use cases will help cater the library to those use cases' needs.

[Chris197]

@vkhorikov Sure, I've put together a rough example of how we are more or less approaching this:
https://github.com/Chris197/error-sample

A few things to note:

• The main reason we have a custom IErrorMessage type is so we can communicate meta data about an error instead of a fixed string, like Type (invalid input), Code and Properties so that the UI can map this to its own user-friendly, localized error message.
• We renamed Unit to Empty since the term Unit was clashing with multiple other libraries.
• Returning Empty.Instance is already an improvement over returning Result.Success<Empty, IErrorMessage>(default), although that does rely on implicit casting of course.
• I created a custom Combine() function to produce a Result<Empty, IErrorMessage>instead of the default Result<Empty[], IErrorMessage>.

Any comments are welcome of course.

[vkhorikov]

Thanks @Chris197 . The main comment I have is that handling the input field names like in

return Result.Failure<Empty, IErrorMessage>(MainErrors.RequiredFieldError("last name"));

is not the error's responsibility because they are application / data contract concerns. That would be fine on its own (as a small concession of purity toward simpler code) but it doesn't work if the same value object is represented by different field names in data contracts.

[Chris197]

@vkhorikov Yes, agreed, not quite sure what the best approach is yet. I could initially leave field empty, but that would mean creating an incomplete error object, relying on the caller to fill it in, which is not great. Alternatively I could remove the field name from the error object and then have the caller wrap it in a new 'ErrorWithField' model afterwards. Any suggestions?

[rathga]

I use a similar concept in a recent project where I have a KeyedError class that has key/error pairs and can hold multiple errors. The keys are obviously specific to the function producing the Result. My way of handling the 'purity' issue is that it is the responsibility of the calling function to map the keys to ones that make sense for it's own context.

e.g.

public static Result<Something> CreateSomething(SomethingDto dto) {
 return FailureIf(dto.Name == null, new KeyedError(nameof(dto.Name), Errors.CannotBeNull))...
}

public Result CreateAndSaveSomething(string name) {
  var dto = new SomethingDto(name);
  return CreateSomething(dto).MapKey(nameof(dto.Name), nameof(name)).Tap(s => repo.Save(s));
}

The ultimate caller (perhaps an asp.net form or API endpoint) than has a validation response for fields in its own context.

Interested to know thoughts on this approach?

(Note in practice MapKey would take in some sort of dictionary/mapping function for larger objects)

[vkhorikov]

@Chris197 yes, mapping of the error to a specific field in the dto/data contract should be done by the app layer (controller). In ASP.NET, you can usually rely on the built-in mapping mechanism. Check out this article: https://enterprisecraftsmanship.com/posts/combining-asp-net-core-attributes-with-value-objects/

[vkhorikov]

@rathga looks fine given that CreateSomething is a method in a controller. But I'm not a fan of manually tying all these errors to the input fields. It's best to automate as much of it as possible: https://enterprisecraftsmanship.com/posts/combining-asp-net-core-attributes-with-value-objects/

[rathga]

@vkhorikov Thanks for that! What about cases of more complex validation/business logic beyond value objects that require loading an aggregate? e.g. checking an amount is below a deposit balance for customers that are not permitted credit. Would you then revert to something like the mapping mechanism I described?

[vkhorikov]

@rathga Yes, for more complex validations (that are handled by the controller), you'd need to do the field mapping manually in the controller.

[AnthonySalon]

Any updates on the UnitResult? Or how should I return a Result with no value but error class? Thanks.

[vkhorikov]

Still working on this. I was hoping to release it with all the extension methods, but will probably do it in chunks. Will publish the class itself this weekend.

[AnthonySalon]

Still working on this. I was hoping to release it with all the extension methods, but will probably do it in chunks. Will publish the class itself this weekend.

Great! It is a good idea to do it in chunks. Just the class itself is enough for me right now.

I knew CSharpFunctionalExtensions from your excellent Pluralsight courses. I like the idea that returning a Result class instead of a bool or string, and having a specific Error class for each error. If I understand the courses correctly, if a method is a query then I should return Result<T, E> and if a method is a command I should return Result, but of course, there is no Result. That was why I found this GitHub issue. So if a method is a command I should return UnitResult (or Result<Unit, E>, depending on which way you choose), right?

[vkhorikov]

Yes, that's correct. Queries return Result<T, E>, commands -- Result<Unit, E> or UnitResult. Note that if you are working with decorators (ala CQRS), it would be easier to work with Result<Unit, E> over UnitResult because of the "compatibility" between Result<T, E> and Result<Unit, E>.

Result is just a shortcut for Result<Unit, string>

[AnthonySalon]

Thanks for clarifying that. Another question is that the CSharpFunctionalExtensions has evolved a lot since you released your courses on Pluralsight. Are there any articles or courses that introduce how to use this library in the correct way, especially all those extension methods?

[vkhorikov]

I plan to do an extensive documentation for that, but no ETA for that just yet. I'm also going to update the Pluralsight course at some point.

[AnthonySalon]

I plan to do an extensive documentation for that, but no ETA for that just yet. I'm also going to update the Pluralsight course at some point.

Sounds great! Looking forward to them!

[vkhorikov]

I've added a basic implementation of UnitResult (with no extension methods). This will be published as v2.19.0 shortly.