ardalis / Result

A result abstraction that can be mapped to HTTP response codes if needed.
MIT License
847 stars 100 forks source link

Expose ValidationErrors as IEnumerable to Prevent Side Effects #169

Closed KyleMcMaster closed 6 months ago

KyleMcMaster commented 6 months ago

Sample unit test showing behavior

[Fact]
public void DoesNotAddValidationErrorToValidationErrors()
{
    var result = Result.Invalid(new List<ValidationError>());

    var errors = result.ValidationErrors.ToList();
    errors.Add(new ValidationError());

    result.Status.Should().Be(ResultStatus.Invalid);
    result.ValidationErrors.Should().HaveCount(0);
    errors.Should().HaveCount(1);
}
github-actions[bot] commented 6 months ago

Code Coverage

Package Line Rate Branch Rate Complexity Health
Ardalis.Result.Sample.Core 18% 18% 63
Ardalis.Result.FluentValidation 86% 50% 6
Ardalis.Result 21% 0% 82
Summary 23% (95 / 416) 17% (9 / 54) 151
ardalis commented 6 months ago

Should we have more options for including correlation ids? And if so, rather than having separate methods for all of them, would it make sense to bake it into a single parameter object, like:

public record ErrorList(IEnumerable<string> ErrorMessages, string? CorrelationId);

?

Then the factory methods would just take in (ErrorList errorList) and we wouldn't need any overloads or alternate methods.

KyleMcMaster commented 6 months ago

Should we have more options for including correlation ids? And if so, rather than having separate methods for all of them, would it make sense to bake it into a single parameter object, like:

public record ErrorList(IEnumerable<string> ErrorMessages, string? CorrelationId);

?

Then the factory methods would just take in (ErrorList errorList) and we wouldn't need any overloads or alternate methods.

I agree we should try to pass CorrelationId whenever we can, I'll update the methods in a separate PR.