FluentResults
FluentResults copied to clipboard
Better return types for lists that aren't really mutable
IResultBase
exposes mutable lists for the Errors
and Successes
properties, however, when implemented in ResultsBase
, these Lists aren't really mutable in the sense that each time you access the property you are in fact getting a new list. This violates Principle of Least Surprise, for the simple fact that you can create a ResultBase
derived instance, add to the Errors
list and on subsequent access the list appears empty (in fact it's a different reference). Example:
var result = new Result();
result.Errors.Add(new Error("Something failed"));
result.Errors.Count == 0; // Evaluates to true;
This PR updates some of the return types to better communicate the intent of the IResultBase
contract.
Good point! Thanks for the feedback. This is a big breaking change - if i merge it then I have to make a new major version of the lib. Please let me think about it.
I agree with @nukefusion this is very confusing for a new user of this framework. The code that is in the example here is almost identical to the first thing some of our developers tried, and I can't really blame them. You have to read the implementation of these properties in order to understand that Errors and Successes isn't actually something you should work with in that manner, whilst Reasons.Add
actually works exactly as expected.
I also agree with @nukefusion and I'm the perfect exemplar of 'new user' of the library @Tobbe1974 mentioned. It is common to develop using the following pattern:
- Evaluate all the errors from a method
- Store them in a variable
- Return them all at once. In this way the client (or the user) can call the method just once and know everything is wrong, let me exemplify with some code:
static void Main(string[] args)
{
var res = Validar();
Console.WriteLine("IsSuccess?" + res.IsSuccess); // Should evaluate to false but is returning true!
}
public static Result<Customer> Validar()
{
Result<Customer> result = new Result<Customer>();
if (SomethingIsWrong())
result.Errors.Add(new Error("Error 1"));
if (AnotherThingIsWrong())
result.Errors.Add(new Error("Error 2"));
return result;
}
I believe this changing would improve this lib A LOT! Is there any plans to merge and publish this PR?
Can we merge this? @altmann