Add generic ErrorList<T> that supports Combine.

[xavierjohn]

trafficstars

Added an Error object and an ErrorList object. ErrorList can combine Error objects.

I had to drop net40;net45; since I could not find the developer pack to build those unsupported frameworks.

[hankovich]

I would say I feel that we have not yet discussed Error / ErrorList from all sides, so it is too early to have them in the library.

Moreover, I have several concerns about the provided implementation. I think it's totally ok to have such error in company projects, but it isn't to generic to be located in the library itself.

[xavierjohn]

That is true. I have been thinking if we should have a companion library with commonly needed items like Email, RequiredString<>, Error, etc.

Here we could have an ErrorList<T> so that people can have their own Error object but I was not able to get Combine to work with a generic ErrorList<T> This specific error class comes from the validation course.

[xavierjohn]

I figured out the problem with the Generic Error so now we have to option of only keeping the generic version in this library.

[xavierjohn]

params T [] errors

Added.

[vkhorikov]

@hankovich Regarding the discussion about Error, my current plan is to implement it like this: https://github.com/vkhorikov/CSharpFunctionalExtensions/issues/321#issuecomment-966245392 Could you review and comment on any issues you see with it?

@xavierjohn Sorry, we need to postpone this particular PR. I plan to try out the Error class in some of my projects (with a locally built nuget package) before committing it to the library. Though after we have the Error, I don't see any reason why we can't have ErrorList too.

[MikelThief]

Instead of Error in Result<SomeType, Error> I usually go with Result<SomeType, ErrorArray> Error might be any error (like the one presented by @vkhorikov which looks versatile for different projects).

public class ErrorArray : ValueObject, IReadOnlyCollection<Error>
    {
        private ImmutableArray<Error> _errors;

        public static ErrorArray Empty => new ErrorArray();

        public int Count => _errors.Length;

        public IEnumerator<Error> GetEnumerator() => ((IEnumerable<Error>)_errors).GetEnumerator();

        IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)_errors).GetEnumerator();

        public ErrorArray() => _errors = ImmutableArray<Error>.Empty;

        public ErrorArray(Error error) => _errors = ImmutableArray.Create(error);

        public ErrorArray(ImmutableArray<Error> errors) => _errors = errors;

        public ErrorArray(ErrorArray errors) => _errors = ImmutableArray<Error>.Empty.AddRange(errors);

        public ErrorArray With(Error error) => new ErrorArray(_errors.Add(error));

        public ErrorArray With(IEnumerable<Error> errors) => new ErrorArray(_errors.AddRange(errors));

        public ErrorArray With<T>(Result<T, ErrorArray> result)
        {
            if (result.IsFailure)
            {
                return new ErrorArray(_errors.AddRange(result.Error));
            }
            return this;
        }

        protected override IEnumerable<object> GetEqualityComponents()
        {
            yield return Count;
            for (int cnt = 0; cnt < Count; cnt++)
            {
                yield return _errors[cnt];
            }
        }
    }

It might serve as some API example for errors collection (it needs to become generic though via some marking interface for errors).

I would prefer for implementation in PR use composition for storing errors rather than inheritance. Also, naming could be improved: ErrorList stores errors so HasErrors is too verbose. IsEmpty is enough since it shouldn't store anything else (so T should be limited to only errors).

[xavierjohn]

@vkhorikov I hope your new course will have a solution to this error problem that is rfc7807 compliant. https://datatracker.ietf.org/doc/html/rfc7807

Using the ProblemDetails Class in ASP.NET Core Web API https://code-maze.com/using-the-problemdetails-class-in-asp-net-core-web-api/

https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.mvc.problemdetails?view=aspnetcore-6.0

[vkhorikov]

@xavierjohn I was going to go with my usual Envelope actually :) Why do you prefer the ProblemDetails?

[xavierjohn]

I used Envelope when there was no standard. Now that HTTP API has defined RFC7807, we have to follow it. ProblemDetails is RFC7807 compliant. We also want failure to be consistent with Data Annotations. Today I have two different behaviors. When..

1. Data Annotation fails, the errors are in RFC7807.
2. Domain fails, it is ...

{
    "code": "value.is.invalid",
    "message": "updatedBy is invalid."
}

I am working on Result -> ProblemDetails conversion today. Let's see how it goes.

[vkhorikov]

You just need to update the Envelope to follow the standard. You can check how to combine data annotations with a custom envelope here: https://enterprisecraftsmanship.com/posts/combining-asp-net-core-attributes-with-value-objects/

[xavierjohn]

Having to create a ValidationAttribute for each value object seems like extra work. I want to be lazier :) public sealed class EmailAttribute : ValidationAttribute

I wonder if we can add an optional 'Field' property to the error object so that the ApplicationBase class can generically process the entire ErrorList to the RFC7807 format.

A completely opposite approach would be for the Domain to validate using Data Annotations as provided by this library. https://github.com/commonsensesoftware/More/tree/master/src/More.Validation A lot of the domain validation is available out of the box with Data Annotation so it makes sense to leverage it.

[vkhorikov]

I wonder if we can add an optional 'Field' property to the error object so that the ApplicationBase class can generically process the entire ErrorList to the RFC7807 format.

I wouldn't add this field to Error itself. Errors are returned by domain classes, and those domain classes shouldn't really know about the fields in data contracts the input data is coming from.

A completely opposite approach would be for the Domain to validate using Data Annotations as provided by this library.

Yeah, that's the default approach in many projects. I don't like it because it goes against the principles of domain model encapsulation.

[xavierjohn]

Yeah, that's the default approach in many projects. I don't like it because it goes against the principles of domain model encapsulation.

How does it break domain model encapsulation? The ctor is still private so the Create method has to be called. Create method will assign the properties and call validate. Now it is clear at the property level what validation will be done.

About the field, it does have the -ve that it is using the Domain field name which may differ from the DTO but in most of the cases, the DTO field name is the same as the Domain field name.

[vkhorikov]

How does it break domain model encapsulation? The ctor is still private so the Create method has to be called. Create method will assign the properties and call validate. Now it is clear at the property level what validation will be done.

That's not what I saw in the sample. Here for example the email validation resides in the rule itself, not the domain model (e.g value object):

https://github.com/commonsensesoftware/More/blob/master/src/More.Validation/EmailRule.cs

[xavierjohn]

Yes, he is fine with creating invalid domain object but we don't have to do that. Achieve domain purity by leveraging validation code provided by the framework. I think it is the best of both worlds. His code shows how to manually validate which we will leverage in our create method.

[xavierjohn]

@vkhorikov I created a few samples using data annotations. Using your error class. https://github.com/xavierjohn/ValueObjects/blob/main/CommonValueObject/EmailAddress.cs

Using ValidationResult https://github.com/xavierjohn/ValueObjects/blob/main/CommonValueObject/EmailAddress2.cs

A more complex class https://github.com/xavierjohn/ValueObjects/blob/main/CommonValueObject/Student.cs

Failures look like this https://github.com/xavierjohn/ValueObjects/blob/main/CommonValueObjectTests/StudentTest.cs

I am no expert in Data Annotations so as I learn more, I will improve the sample.

[xavierjohn]

Lesser code in this version using private set. https://github.com/xavierjohn/ValueObjects/blob/main/CommonValueObject/Student2.cs

[vkhorikov]

@xavierjohn Was out of town during the holidays. I don't quite like this approach because it breaks the separation of concerns between domain modeling (the VOs) and application-specific validation (the attributes).

[xavierjohn]

I don't think I fully understood "the separation of concerns between domain modeling (the VOs) and application-specific validation (the attributes)". Why would we not what to use Data Annotation for Domain Model validation? So much code for reuse and seems more useful than the fluent validation library that requires us to roll out a separate validation class.

We can implement complex domain validation by implementing the interface IValidatableObject https://stackoverflow.com/questions/3400542/how-do-i-use-ivalidatableobject

[vkhorikov]

I wrote about the SoC principle in-depth here: https://khorikov.org/posts/2021-07-05-domain-model-purity-boundary/

Validation attributes themselves are fine (just as FV classes are fine also), it's just you don't want to mix them in with domain modeling concerns (value objects).

[ProphetLamb]

We could also approach the problem of collecting errors from the Result perspective. By introducing a ResultCollection: IResult which allows collecting Results via implicit operator ResultCollection +(ResultCollection, Result).

Instead of storing Resultss themselves, the content is distributed into the following structure.

public abstract class ResultCollectionBase<S, F> : IResult<S, F>, ICollection<Result<S, F>>, IReadonlyCollection<Result<S, F>> {
    protected readonly List<S> _successes = new();
    protected readonly List<F> _failures = new();
    private Maybe<S> _success;
    private Maybe<F> _failure;

    public bool IsSuccess => _failures.IsEmpty && !_successes.IsEmpty;
    public bool IsFailure => _successes.IsEmpty && !_failures.IsEmpty;

    public S Value {
        get {
            if (_success.TryGetValue(out S s)) { return s; }
            If (!AggragateSuccesses().TryGetValue(out s)) { throw null; }
            _success = s;
            return s;
        }
    }

    public F Error {
        get {
            if (_failure.TryGetValue(out F f)) { return f; }
            If (!AggragateFailures().TryGetValue(out f)) { throw null;  }
            _failure = f;
            return f;
        }
    }

    protected abstract Maybe<S> AggragateSuccesses();
    protected abstract Maybe<F> AggragateFailures();

    public void Add(in Result<S, F> item) {
        if (item.TryGetFailure(out F f, out S s)) {
            _failure = default;
            AddError(f);
        }
        _success = default;
        AddValue(s);
    }

    protected virtual void AddValue(S item) => _successes.Add(item);
    protected virtual void AddError(F item) => _failures.Add(item);

    public Maybe<Result<S, F>> ToResult();
    public Result<ImmutableArray<S>, F> ToValuesResult();
    public Result<S, ImmutableArray<F>> ToErrorsResult();

    public SuccessCollection Values => new(this);
    public FailureCollection Errors => new(this);
    
    [...]
}

public sealed class DelegateResults<S, F>: ResultCollectionBase<S, F> {
    readonly Func<S, S, S> _successAggragator;
    readonly Func<F, F, F> _failureAggragator;

    protected virtual Maybe<S> AggregateSuccesses() {
        if (!_successes.TryGetFirst(out S res)) { return default; }
        for (int i = 1; i < _successes.Count; i += 1) {
            res = _successAggragator(res, _successes[i]);
        }
        return res;
    }
    [...]
}

The Good:

• Does not inflate function identities because we can default to IResult or call ToResult(): Result<S, F>.
• Allows arbitrary user defined handling of aggregation.
• Conversation to Result is possible.
• Using existing extension methods is possible, without reoccurring cost.
• Little allocation overhead only IntPtr*2+byte*2 compared to a boxed Result.
• Much quicker than Bind*n.

The Bad:

• !IsFailure does not imply IsSuccess, hence Maybe<Result<S, F>> ToResult()
• How to handle Result<S>, UnitResult<E> and Result? Duplicate implementation?
• Possibly bogus aggregating, as seen below, maybe drop IResult trade interface?

This would allow for the following syntaxes:

class PersonResults : ResultCollectionBase<Person, Error> {
    protected override Maybe<Person> AggragateSuccesses() => default;
    protected override Maybe<Error> AggragateFailures() => _failures.Aggegate(static (rhs, lhs) rhs.Combine(lhs)) ?? default;
}

public Result<Person[], Error> ReadPerson(IEnumerable<string> storyBookOfLife) {
    PersonResults people = new();
    foreach (string storyOfLife in storyBookOfLife) {
        people += TryParsePerson(storyOfLife);
    return people.ToValuesResult();
}

class LyricsResults : ResultCollectionBase<Lyrics, Lyrics> {
    protected override Maybe<Lyrics> AggragateSuccesses() => _successes.LastOrDefault();
    protected override Maybe<Lyrics> AggragateFailures() => default;
}
public Result<Lyrics, Lyrics[]> WriteSong(IEnumerable<Lyrics> songwriter) {
    LyricsResults attempts = new();
    foreach (Lyrics attempt in songwriter) {
        attempts += ProducerEvaluates(attempt);
        if (attempts.IsSuccess)
            break;
        }
    }
    return attempts.ToErrorsResult();
}

[xavierjohn]

@vkhorikov Any progress on an ErrorList or Error class?

[vkhorikov]

@xavierjohn Apology for delays, I plan to work this out before the year end.

[xavierjohn]

I added it to this library https://github.com/xavierjohn/CSharpFunctionalExtensions.Errors

[jeffward01]

I added it to this library https://github.com/xavierjohn/CSharpFunctionalExtensions.Errors

Hello!

I just wanted to share this library with you as I had a thought - it would be cool to combine the two somehow: https://github.com/altmann/FluentResults

FluentResults has a concept of IReason<> and IError<>, and it might be more versitile than just having a sole Error, just a thought.

I'll also give it some thoughts in how they would be merged.

Thanks for the very cool extension library @xavierjohn !

By funny coincidence, the author of Fluent Results was greatly inspired by vkhorikov and has quoted vkhorikov many times: https://github.com/altmann/FluentResults#interesting-resources-about-result-pattern

[xavierjohn]

There is this YouTube material https://www.youtube.com/watch?v=fhM0V2N1GpY&list=PLzYkqgWkHPKBcDIP5gzLfASkQyTdy0t4k that I found pretty interesting that it shows developing a site using DDD. I followed that sample but updated it to use the CSharpFunctionalExtensions. The library, from the perspective of a Pluralsight course, does fine using strings as errors, but for production code, we probably need a real Error object. Using FluentErrors could be an option. The other problem with the current library is we can't chain multiple creates and get a combined error. With the current code, this is the best I can do.

    public static Result<RegisterCommand, ErrorList> Create(string firstName, string lastName, string email, string password)
    {
        var rFirstName = FirstName.Create(firstName);
        var rLastName = LastName.Create(lastName);
        var rEmail = EmailAddress.Create(email);

        return ErrorList.Combine(rFirstName, rLastName, rEmail)
            .Map(x => new RegisterCommand(rFirstName.Value, rLastName.Value, rEmail.Value, password));
    }

Here is the link to the sample. https://github.com/xavierjohn/BuberDinner/blob/76411327665afbe67824a297174d24ed6e1d526c/BuberDinner.Application/src/Services/Authentication/Commands/RegisterCommand.cs#L13

To get to the style I want to code; it will be a breaking change to CSharpFunctionalExtensions by dropping ErrorT and just using a fixed Error object. Also, I am not sure why we need UnitResult type vs just return Result<Unit> for void functions. By removing UnitResult I was able to delete a lot of code and create my own experimental Functional DDD library https://github.com/xavierjohn/FunctionalDDD

This library allows me to write code like below which I wanted for a long time.

    public static Result<RegisterCommand> Create(string firstName, string lastName, string email, string password) =>
        FirstName.Create(firstName)
            .Combine(LastName.Create(lastName))
            .Combine(EmailAddress.Create(email))
            .Map((firstName, lastName, email) => new RegisterCommand(firstName, lastName, email, password));

I updated the BuberDinner to use my library. https://github.com/xavierjohn/BuberDinner/tree/functionalddd

I also added async to async functions because when I was working with CSharpFunctionalExtensions , if I had the wrong parameters, the compiler would get confused between sync and async function so the current code looks like this.

    [HttpPost("register")]
    public async Task<ActionResult<AuthenticationResult>> Register(RegisterRequest request) =>
        await RegisterCommand.Create(request.FirstName, request.LastName, request.Email, request.Password)
            .BindAsync(command => _sender.Send(command))
            .FinallyAsync(result => MapToActionResult(result));

If validation fails, we will get proper error

HTTP/1.1 400 Bad Request
Connection: close
Content-Type: application/problem+json; charset=utf-8
Date: Wed, 21 Dec 2022 01:50:25 GMT
Server: Kestrel
Transfer-Encoding: chunked

{
  "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "00-633fe129950d2b560d90491c950edce1-33b0b067872b1e20-00",
  "errors": {
    "Email": [
      "The Email field is required."
    ],
    "LastName": [
      "The LastName field is required."
    ]
  }
}

I also want to bring the concept of Accepted (202) and Created (201) to the domain level. In real life, if we place an order for a cake, the domain can only accept the request, so I am thinking of some domain way of returning Ok, Accepted & Created. The API layer will then be able to automatically translate it to HTTP status codes.