This post is a follow up on the previous post. It presents an approach on how to return values from a method.
I got some great feedback both good and bad from other people, and with that I will present now the updated code taking that feedback into account. Here is the original:
And the modified version:
Following is some of the most important feedback which led to this.
The point of this is that now we avoid allocating memory in high usage scenarios. This was a problem of scale, easily solvable.
Using a tuple instead of a named type feels like it would be more complicated at least to communicate between a team. Instead of a method returning a Result, it could return a (bool success, string code, string message) for sure, but what about when you want to return multiple results? Would you return a List<(bool success, string code, string message)>?
I don't know, it feels a little bit worse to me, but it comes down to opinion. Having said that, I totally wanted to take advantage of deconstruction that C# now offers!
I love this, it feels very functional and it is so, so simple. One of the most useful tips!
Result from Rust
Try in Scala
Result in F#
Either in Haskell
Now that I know about them, and particularly the Result in F#, the intention is very much the same after all: to have a way of handling application flow. Of course this is a half-baked implementation. It is much simpler, has no intention of being as broadly used and doesn't rely on mechanism such as discriminated unions (which doesn't exist in C#) that gives a lot of terseness. I could maybe try to emulate it using inheritance? But I think complexity might be much higher. Instead of checking for success would I check for a OkResult type?
The client of this code wants to know which nodes were synchronized and which ones weren't. When creating the Result type I can't assume a lot of how people will use the type itself, but this seems a reasonable way and is an expected one.
So no, I don't think I would gain a lot from this.
I won't step on that minefield, because it would just be the words of random people against each other.
The context of this post is exclusively for flow control and not exceptional cases. Exceptions should be used in exceptional cases, that is what I believe. Maybe I gave the idea that I was trying to eradicate Exceptions, but that might have been poor wording on my part. It is not the case.
My option is based on research and here are some links to well-respected sources:
Microsoft - Patterns and Practices Developer Center - they designed C#, so might have something to say
Microsoft has analyzer warnings for it
Martin Fowler the Notification Pattern - This is an alternative approach which I appreciate. Martin Fowler is a well respected software architect, no need for introductions.
There are other resources. Oh, I'm sure there are also great resources advocating the opposite, but what can you do? Eventually you need to chose.
As always, the approach you follow depends on a lot of factors. Depends on your projects, on the target you want to achieve, how general do you want it to be, etc. There is no need to advocate for a single approach to solve all the problems. We need to adapt.
I got some great feedback both good and bad from other people, and with that I will present now the updated code taking that feedback into account. Here is the original:
And the modified version:
Following is some of the most important feedback which led to this.
Make it an immutable struct
This was a useful one. I can't say that I have ever found a problem with having the Result type as a class, but that is just a matter of scale.The point of this is that now we avoid allocating memory in high usage scenarios. This was a problem of scale, easily solvable.
Return a tuple instead of using a dedicated Result type
The initial implementation comes from a long time ago, when C# did not have (good) support for tuples and deconstruction wasn't heard of. You would have to deal with the Tuple type, which was a bit of a hassle. I feel it would complicate the matter at that time.Using a tuple instead of a named type feels like it would be more complicated at least to communicate between a team. Instead of a method returning a Result, it could return a (bool success, string code, string message) for sure, but what about when you want to return multiple results? Would you return a List<(bool success, string code, string message)>?
I don't know, it feels a little bit worse to me, but it comes down to opinion. Having said that, I totally wanted to take advantage of deconstruction that C# now offers!
Status code should be an Enum
This would be a good idea, but I'll have to pass on this one. The Result type is defined in a base library, so the Enum would need to be closed in that same library. Sure there are other ways of modelling the problem, but if the usage for these Results follows the previous approach of enumerating all possible results in a static class then it means that we have a closed set of codes that we can use for comparison.Add a Visit extension method
Here is the method with an example of usage:I love this, it feels very functional and it is so, so simple. One of the most useful tips!
This is a half-baked Either type from other languages
Well that wasn't the intention because I didn't even knew about that type. Here are some of the links that were sent to me:Result from Rust
Try in Scala
Result in F#
Either in Haskell
Now that I know about them, and particularly the Result in F#, the intention is very much the same after all: to have a way of handling application flow. Of course this is a half-baked implementation. It is much simpler, has no intention of being as broadly used and doesn't rely on mechanism such as discriminated unions (which doesn't exist in C#) that gives a lot of terseness. I could maybe try to emulate it using inheritance? But I think complexity might be much higher. Instead of checking for success would I check for a OkResult type?
Throw an exception when accessing the Object if Success is false
That assumes a Result cannot have an Object if it is false. I'm not sure if I created that impression with the post, but that is not the point. Imagine I have an operation which synchronizes records for multiple nodes. Synchronization can succeed or fail:The client of this code wants to know which nodes were synchronized and which ones weren't. When creating the Result type I can't assume a lot of how people will use the type itself, but this seems a reasonable way and is an expected one.
So no, I don't think I would gain a lot from this.
Localization is not possible
In the implementation, I'm not sure what prevents any kind of localization. It is indeed possible and I have done it before. There are a lot of techniques to do it.Use a type that encloses the results
The idea in general is to have a Results class that inside has a collection of Errors. This seems to be similar to what ASP.Net does with ModelState object, where you can add multiple errors as you go. My approach is just to return a List<Result> and have some extension methods over it. These are two different approaches and both work. I can see myself using one or the other and I think it is a matter of preference.THROW EXCEPTIONS, MORON
I got some passionate (the kind of passion that Reddit can bring out on people) feedback that pointed me in the direction of throwing exceptions. I did my research about this topic years ago and there seems to be a big division in whether you should throw exceptions for application flow logic. Should we throw an exception if the Node.Name is null?I won't step on that minefield, because it would just be the words of random people against each other.
The context of this post is exclusively for flow control and not exceptional cases. Exceptions should be used in exceptional cases, that is what I believe. Maybe I gave the idea that I was trying to eradicate Exceptions, but that might have been poor wording on my part. It is not the case.
My option is based on research and here are some links to well-respected sources:
Microsoft - Patterns and Practices Developer Center - they designed C#, so might have something to say
Microsoft has analyzer warnings for it
Martin Fowler the Notification Pattern - This is an alternative approach which I appreciate. Martin Fowler is a well respected software architect, no need for introductions.
There are other resources. Oh, I'm sure there are also great resources advocating the opposite, but what can you do? Eventually you need to chose.
Thank you for the feedback
This is exactly why I try to share with as many people as possible, in multiple websites. I got some great feedback and that helped me learn a lot and improve on the original.As always, the approach you follow depends on a lot of factors. Depends on your projects, on the target you want to achieve, how general do you want it to be, etc. There is no need to advocate for a single approach to solve all the problems. We need to adapt.
Comments
Post a Comment