Skip to main content

Follow up: improving the Result type from feedback

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.

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

Popular posts from this blog

Why is the Single Responsability Principle important?

The Single Responsability Principle is one of the five S.O.L.I.D. principles in which i base my everyday programming. It tells us how a method or class should have only one responsability. Not a long time ago i was designing a reporting service with my colleague Nuno for an application module we were redoing and we had a method that was responsible for being both the factory method of a popup view and showing it to the user. You can see where this is going now... I figured out it would not be a that bad violation of the principle, so we moved on with this design. The method was called something like "ShowPrintPopup" and it took an IReport as an argument. All this was fine, but then we got to a point where we needed to have a permissions system to say if the user was able to export the report to Excel, Word, PDF, etc... The problem was the print popup would need to know beforehand if it would allow the user to export the report or not, so that it could show it's UI a...

From crappy to happy - refactoring untestable code - an introduction

I started testing my code automatically a couple of years in after starting my career. Working in a small company, there weren't really incentives for us to automate testing and we were not following any kind of best practices. Our way of working was to write the code, test it manually and then just Release It ™ , preferably on a Friday of course. I'm sure everyone can relate to this at some point in their career, because this is a lot more common than the Almighty Programming Gods of the Internet make us believe. I find that the amount of companies that actually bother writing tests for their production code is a fraction of the whole universe. I know some friends who work in pretty big companies with big names in the industry and even there the same mindset exists. Of course, at some point in time our code turned into a big pile of shit . Nobody really knew what was going on and where. We had quantum-level switcheroo that nobody really wanted to touch, and I suspect it i...

From crappy to happy - dependency what, now?

Following the introduction on this series on a previous post, we will now talk about dependency injection and how it has the effect of allowing for more testable code. Sometimes when I talk about this concept it is difficult to explain the effect that applying it might have on the tests. For that reason I think it is better to demonstrate with a near-real-world situation. Obviously, keep in mind this is not real code, so don't worry about the design or implementation details that don't contribute to the point being discussed. The code As you can see, it is simple. There's a class called ShipManager (what else?) that receives position updates for the ships. It keeps the last position reported from each ship and does some calculation to see how much the ship moved. It assigns some values to the update and finally it persists the final version of the update. How do we start testing? When you think about it, tests are dead simple. A test either passes or it doesn...