Photo by Agence Olloweb

Introduction

Don't use exceptions for flow control!

This is a quote I stumbled on quite a bit during my first years as a programmer. Let's imagine the following scenario:

  • We have one endpoint that takes a name and returns a hamburger with that name.
  • If we can't find a hamburger with that name, we need to return 404 Not found
  • If something else goes wrong, we need to return 500 Internal Server Error.

Let's implement our first solution:

public interface IGetHamburgerQuery
{
    Hamburger Execute(string name);
}


public class InMemoryGetHamburgerQuery : IGetHamburgerQuery
{
    private static readonly IReadOnlyCollection<Hamburger> Hamburgers = new List<Hamburger>
    {
        new Hamburger("Double Cheese"),
        new Hamburger("Big Mac"),
        new Hamburger("Hamburger")
    };

    public Hamburger Execute(string name)
    {
        var hamburger = Hamburgers.FirstOrDefault(x => x.Name.Equals(name, StringComparison.OrdinalIgnoreCase));

        if (hamburger == null)
        {
            throw new NotFoundException();
        }

        return hamburger;
    }
}


[HttpGet("{name}")]
public ActionResult<Hamburger> Get(string name)
{
    try
    {
        var hamburger = _getHamburgerQuery.Execute(name);
        return new OkObjectResult(hamburger);
    }
    catch (NotFoundException)
    {
        return new NotFoundResult();
    }
    catch
    {
        return new StatusCodeResult(500);
    }
}

Nothing fancy going on here, we look in our "database" for the hamburger, if the "database" returns null, we throw a NotFoundException. In our action method we then use a try catch where we catch NotFoundException and return the correct status code.

What's "wrong" with this code then? Well, if we should "obey" the quote above, we should not rely on exceptions for controlling the flow of our program. One could also argue that not finding any hamburger in our database is in fact nothing...exceptional at all. I would rather say that it's EXPECTED. I tend to only use exceptions for errors which I can't recover from (database connection errors...).

Introducing Result.cs

I've used a modified version of Vladimir Khorikov's Result class throughout the years and it has worked great. Let's try it out. Note that I prefix Vladimirs implementation to avoid confusion with my own implementation that I will show further down.

public interface IGetHamburgerResultQuery
{
    Vladimir.Result<Hamburger> Execute(string name);
}


public class InMemoryGetHamburgerResultQuery : IGetHamburgerResultQuery
{
    private static readonly IReadOnlyCollection<Hamburger> Hamburgers = new List<Hamburger>
    {
        new Hamburger("Double Cheese"),
        new Hamburger("Big Mac"),
        new Hamburger("Hamburger")
    };

    public Vladimir.Result<Hamburger> Execute(string name)
    {
        var hamburger = Hamburgers.FirstOrDefault(x => x.Name.Equals(name, StringComparison.OrdinalIgnoreCase));

        if (hamburger == null)
        {
            return Vladimir.Result.Fail<Hamburger>($"Could not find any hamburger named '{name}'");
        }

        return Vladimir.Result.Ok<Hamburger>(hamburger);
    }
}


[HttpGet("result/{name}")]
public ActionResult<Hamburger> GetResult(string name)
{
    var hamburgerResult = _getHamburgerResultQuery.Execute(name);
    if (hamburgerResult.Success)
    {
        return new OkObjectResult(hamburgerResult.Value);
    }

    if (hamburgerResult.Error.Equals($"Could not find any hamburger named '{name}'"))
    {
        return new NotFoundResult();
    }

    return new StatusCodeResult(500);
}

Now we are not using exceptions anymore. If we can't find any hamburger we are returning Result.Fail.
We are then inspecting the Success property in our controller to see if everything went OK. If it didn't, we inspect the Error property to see if we should return a 404 or not.
This is one of the things that bothered me quite a bit, with the current implementation of the Result class, there is no easy/good way of telling WHAT went wrong. You just know that SOMETHING went wrong and you can only act on the error message. That's what I'm trying to fix with my own implementation.

My take on the Result class

I will just paste in the Result class here. Basically it boils down to the following:

  • Two classes, SuccessResult and ErrorResult.
  • When something goes OK -> return a SuccessResult
  • When something goes wrong -> return a ErrorResult OR something that inherits from ErrorResult to provide some more context.
public abstract class Result
{
    public bool Success { get; protected set; }
    public bool Failure => !Success;
}

public abstract class Result<T> : Result
{
    private T _data;

    protected Result(T data)
    {
        Data = data;
    }

    public T Data
    {
        get => Success ? _data : throw new Exception($"You can't access .{nameof(Data)} when .{nameof(Success)} is false");
        set => _data = value;
    }
}

public class SuccessResult : Result
{
    public SuccessResult()
    {
        Success = true;
    }
}

public class SuccessResult<T> : Result<T>
{
    public SuccessResult(T data) : base(data)
    {
        Success = true;
    }
}

public class ErrorResult : Result, IErrorResult
{
    public ErrorResult(string message) : this(message, Array.Empty<Error>())
    {
    }

    public ErrorResult(string message, IReadOnlyCollection<Error> errors)
    {
        Message = message;
        Success = false;
        Errors = errors ?? Array.Empty<Error>();
    }

    public string Message { get; }
    public IReadOnlyCollection<Error> Errors { get; }
}

public class ErrorResult<T> : Result<T>, IErrorResult
{
    public ErrorResult(string message) : this(message, Array.Empty<Error>())
    {    
    }

    public ErrorResult(string message, IReadOnlyCollection<Error> errors) : base(default)
    {
        Message = message;
        Success = false;
        Errors = errors ?? Array.Empty<Error>();
    }

    public string Message { get; set; }
    public IReadOnlyCollection<Error> Errors { get; }
}

public class Error
{
    public Error(string details) : this(null, details)
    {

    }

    public Error(string code, string details)
    {
        Code = code;
        Details = details;
    }

    public string Code { get; }
    public string Details { get; }
}

internal interface IErrorResult
{
    string Message { get; }
    IReadOnlyCollection<Error> Errors { get; }
}

Our hamburger example will now look like this:

public interface IGetHamburgerJosResultQuery
{
    Result<Hamburger> Execute(string name);
}


public class InMemoryGetHamburgerJosResultQuery : IGetHamburgerJosResultQuery
{
    private static readonly IReadOnlyCollection<Hamburger> Hamburgers = new List<Hamburger>
    {
        new Hamburger("Double Cheese"),
        new Hamburger("Big Mac"),
        new Hamburger("Hamburger")
    };

    public Result<Hamburger> Execute(string name)
    {
        var hamburger = Hamburgers.FirstOrDefault(x => x.Name.Equals(name, StringComparison.OrdinalIgnoreCase));

        if (hamburger == null)
        {
            return new NotFoundResult<Hamburger>("Could not find any hamburgers");
        }

        return new SuccessResult<Hamburger>(hamburger);
    }
}


[HttpGet("jos-result/{name}")]
public ActionResult<Hamburger> GetJosResult(string name)
{
    var hamburgerResult = _getHamburgerJosResultQuery.Execute(name);
    return hamburgerResult switch
    {
        SuccessResult<Hamburger> successResult => new OkObjectResult(successResult.Data),
        NotFoundResult<Hamburger> notFoundResult => new NotFoundResult(),
        ErrorResult<Hamburger> errorResult => new StatusCodeResult(500),
        _ => new StatusCodeResult(500)
    };
}

We are taking advantage of the switch expression and by doing so the code in our action method becomes quite nice and readable (in my opinion... :) ). Now we don't need to rely on a error message to return a 404 Not Found.

The whole idea of my implementation of the Result class is that you should create specific classes that adds some context that can help you further up in the chain to make the correct decision. By using classes instead of for example an enum the solution becomes more flexible. Now I can put the Result class in a nuget package and then use it in some other project where I might need to handle different kinds of errors. The only thing I need to do then is inherit from ErrorResult.

You need to handle validation errors?

public class ValidationErrorResult : ErrorResult
{
    public ValidationErrorResult(string message) : base(message)
    {
    }

    public ValidationErrorResult(string message, IReadOnlyCollection<ValidationError> errors) : base(message, errors)
    {
    }
}

public class ValidationError : Error
{
    public ValidationError(string propertyName, string details) : base(null, details)
    {
        PropertyName = propertyName;
    }

    public string PropertyName { get; }
}


public Result ValidateMyData(MyData data)
{
    if(data.IsValid)
    {
        return new SuccessResult();
    }
    return new ValidationErrorResult("Error when validating...", data.Errors.Select(x => new ValidationError(x.PropertyName, x.Message)))
}

Or you need access to the http status code to see if you should retry?

public class HttpErrorResult : ErrorResult
{
    public HttpStatusCode StatusCode { get; }

    public HttpErrorResult(string message, HttpStatusCode statusCode) : base(message)
    {
        StatusCode = statusCode;
    }

    public HttpErrorResult(string message, IReadOnlyCollection<Error> errors, HttpStatusCode statusCode) : base(message, errors)
    {
        StatusCode = statusCode;
    }
}


...
var result = GetResult();
if(result is HttpErrorResult httpErrorResult)
{
   if(httpErrorResult.StatusCode == HttpStatusCode.BadGateway)
   {
       Retry();
   }
}
....

Usage

In 99% of the cases you will use it like this:

var result = GetData();
if(result.Success)
{
   // Do something
}
else
{
   // Handle error
}

But as soon as you need more context/info about what went wrong you will use it like this:

var result = GetData();

return result switch
{
    SuccessResult<Hamburger> successResult => HandleSuccess(successResult),
    NotFoundResult<Hamburger> notFoundResult => HandleNotFoundResult(notFoundResult),
    ErrorResult<Hamburger> errorResult => HandleErrorResult(errorResult),
        _ => result.MissingPatternMatch();
};

Or like this:

var result = GetData();

if(result.Failure)
{
    if(result is NotFoundResult notFoundResult)
    {
       // Handle not found result
    }
    // Handle "unknown" error
}

This gives you great flexibility imo!

Handling exceptions

The following code can still (let's pretend at least) throw exceptions:

public class InMemoryGetHamburgerJosResultQuery : IGetHamburgerJosResultQuery
{
    private static readonly IReadOnlyCollection<Hamburger> Hamburgers = new List<Hamburger>
    {
        new Hamburger("Double Cheese"),
        new Hamburger("Big Mac"),
        new Hamburger("Hamburger")
    };

    public Result<Hamburger> Execute(string name)
    {
        var hamburger = Hamburgers.FirstOrDefault(x => x.Name.Equals(name, StringComparison.OrdinalIgnoreCase));

        if (hamburger == null)
        {
            return new NotFoundResult<Hamburger>("Could not find any hamburgers");
        }

        return new SuccessResult<Hamburger>(hamburger);
    }
}

I usually wrap my queries in a try catch like this:

public class InMemoryGetHamburgerJosResultQuery : IGetHamburgerJosResultQuery
{
    private static readonly IReadOnlyCollection<Hamburger> Hamburgers = new List<Hamburger>
    {
        new Hamburger("Double Cheese"),
        new Hamburger("Big Mac"),
        new Hamburger("Hamburger")
    };

    public Result<Hamburger> Execute(string name)
    {
        try
        {
            var hamburger = Hamburgers.FirstOrDefault(x => x.Name.Equals(name, StringComparison.OrdinalIgnoreCase));

            if (hamburger == null)
            {
                return new NotFoundResult<Hamburger>("Could not find any hamburgers");
            }

            return new SuccessResult<Hamburger>(hamburger);
        }
        catch (SomeRecoverableDatabaseException e)
        {
            return new DatabaseErrorResult(e);
        }

        return new ErrorResult<Hamburger>("Error when trying to fetch hamburger");
    }
}

Note that I only tend to catch exceptions that I can recovery from. This is just a matter of taste though, it's completely fine to catch all exceptions and return a Result as well, the important thing is that you handle the error.

Closing thoughts

This was just some quick thoughts about how I usually work with errors in my applications. I find that I produce fewer bugs since I'm forced to handle all different scenarios:

Compare this...

public string GetName()
{
    var data = GetName();
    return data.Name; // Says boom if data is null;
}

...to this

public string GetName()
{
    var dataResult = GetName();
    if(dataResult is SuccessResult<Data> successResult)
    {
        return successResult.Data.Name;
    }
    return string.Empty; // Or throw, or null, or maybe return a RESULT... :)
}

In the first example I don't know if GetName has succeeded. In the second example I need to check if it was successful before accessing the .Data property.

Using a Result class is not a silver bullet when it comes to error handling, you will still need to handle exceptions but I find that I tend to handle them much closer to the source. Gone are all try catchs in my controllers that catches some exception that happened in the 35:th layer of code... :)

All code in this post can be found here.