1

I am looking at a problem where a method can fail in a predictable way. For now I think that exceptions might not be a good way to represent some of the failures (I may be wrong on that). In any case I was thinking of returning a Result type similar to the ones in ASP.NET Identity and the ones discussed in this question - What is a good design for a method that can return several logically different results? (i.e. a bool or enum property Result and other properties which are present if the result is success). However since C# 7 is now released and it has pattern matching I wonder if this is not a better option

public abstract class ReadFileResult
{
}

public class FileNotFoundResult : ReadFileResult
{
}

public class BytesFileResult : ReadFileResult
{
    public BytesFileResult(byte[] fileData)
    {
        FileData = fileData;
    }

    public byte[] FileData { get; }
}

class Program
{
    static void Main(string[] args)
    {
        ReadFileResult readFileResult = ReadFile("test.txt");

        switch (readFileResult)
        {
            case FileNotFoundResult fileNotFound:
                {
                    Console.WriteLine("File not found");
                }
                break;
            case BytesFileResult bytesFileResult:
                {
                    Console.WriteLine(bytesFileResult.FileData.Length); //do something with the data
                }
                break;
            default:
                {
                    throw new Exception("Unexpected file result");
                }
        }
    }

    public static ReadFileResult ReadFile(string filename)
    {
        throw new NotImplementedException(); //some logic here
    }
}

Would you prefer to see the "standard" result type + properties way or the pattern matching way is clearer? If you prefer one over the other why? Can the pattern matching based solution be improved?

Edit: Just for the record this example is NOT my use case. My specific use case will need at least 3 catch blocks if I go for exceptions (which I still might do). I lean towards returning a result because I prefer if over 3 catch blocks even if I'd probably go for an exception if it was just 1 odd case. In any case my question is meant to compare the two ways to return a result not to argue about exceptions vs results. Lets assume that I am talking about ASP.NET Identity. Would they be better off if instead of this class they had a bunch of derived classes and used pattern matching (assuming that all their clients used C# 7.0)

11
  • Any specific reason why you don't use exception handling instead of your result sub-classes?
    – Staeff
    Jul 24, 2017 at 9:39
  • 3
    it's the fremen way
    – Ewan
    Jul 24, 2017 at 9:56
  • 1
    A good design is to not have a method that can return several different logical results. Methods should do only thing.
    – paparazzo
    Jul 24, 2017 at 12:04
  • @Staeff I prefer to use exceptions when I can funnel the execution in single exception case rather than use different catch blocks as a form of if. Specifically this happens when the "error" case must be shown to the user. Think of ASP.NET Identity where for example user registration returns a result where the error case might be "e-mail already exists" or "username already exist" and you must have ifs in the calling code to check for each case and be able to display the proper message to the user.
    – Stilgar
    Jul 24, 2017 at 12:08
  • Have you checked the new multi-return functionality of c#? It uses implicit tuples and it a very nice feature if used correctly.
    – T. Sar
    Jul 24, 2017 at 14:00

5 Answers 5

4

I'd say that this is literally the classical use case for exceptions. Exceptions already come with a perfectly good pattern matching mechanism. As put, there's no advantage over just letting the exception propagate upward, and catch it in your Main(). Here's what your implementation of ReadFileResult would look like:

public static ReadFileResult ReadFile(string filename)
{
  try {
     return new BytesFileResult(File.ReadAllBytes(filename));
  }
  catch (FileNotFoundException) {
     return new FileNotFoundResult();
  }
}

You could do this for the same effect, which would save you a lot of unnecessary complexity:

class Program
{
    static void Main(string[] args)
    {
        try {
            byte[] readFileResult = ReadFile("test.txt");
            Console.WriteLine(bytesFileResult.Length); 
        }
        catch (FileNotFoundException) {
            Console.WriteLine("File not found");
        }
    }

    public static ReadFileResult ReadFile(string filename)
    {
        File.ReadAllBytes(filename); //some logic here
    }
}

Don't forget that there are other IOExceptions that can happen when you read a file. Would you create ReadFileResults for those too? That's just adding an extra layer of indirection without any obvious benefit.

ASP.NET uses the multiple result pattern because you can return different kinds of result - not just content or error. You can return a View, JSON, a redirect, raw content, blank, so on. This isn't your situation.

The "error result" pattern by itself is useful when you have to serialize the result, for example, and pass it through a layer that doesn't support exceptions.

0
3

You have chosen a language, C#, which uses exceptions throughout, and your use case seem to be exactly what exceptions are supposed to be used for.

You may feel some other mechanism for error handling (e.g pattern matching on result types as is used in e.g. Rust or Haskell) have certain advantages, but this is really a choice done on the language level. Even if you decide to use a different mechanism, the BCL and all third party libraries will still use exceptions, which means you will have to support both kinds of error handling in your application then, and you are going to confuse other developers to boot. There is simply no good reason to do so.

Pattern matching on result types is a useful pattern, but should not be used as alternative to exceptions.

6
  • I should have clarified in the question that the example is not my use case. I specifically wander how to choose between the two result patterns. The result pattern is useful in C#. As pointed out ASP.NET Identity uses it a lot instead of exceptions.
    – Stilgar
    Jul 24, 2017 at 12:13
  • @Stilgar: I am a bit confused what your actual question is then? What is your use case?
    – JacquesB
    Jul 24, 2017 at 12:18
  • My use cases has to do with images similar to the ImageResizer HTTP module (resize options passed in the URL). The results are more than just success/failure and I should return several different status codes from my service layer depending on the specific situation. I am leaning towards result pattern because if I go for exceptions I would need at least 3 different catch blocks. I'd rather go for an if. In any case my specific question is about comparing the two patterns for returning results (result with properties vs types with pattern matching) not about results vs exceptions.
    – Stilgar
    Jul 24, 2017 at 12:27
  • @Stilgar: The question is if the results indicate an error condition or not. If they indicate error conditions (i.e the method was not able to complete the task indicated by its name), then you should use exceptions regardless of the number of catch blocks it may require. If you have logically different results which are not error conditions then you might use a Result-pattern, but more likely you are breaking separation of concerns (because why does the same method return unrelated types?), but this depends on the actual use case.
    – JacquesB
    Jul 24, 2017 at 12:41
  • @Stilgar: In any case you should probably update the original question, because other posters might not look through all comments to find the "the real question".
    – JacquesB
    Jul 24, 2017 at 12:44
1

Exceptions should be used only when the function results in an error. And, if a function fails, it should return an exception. Error return codes are acceptable only when performance is of absolute importance, or input validators like Int32.TryParse.

If a function need to return different types of "success" results, then pattern-matching is actually a nice approach.

1
  • Thanks for pointing that out. The .net framework design guidelines make this clear as well. Jul 24, 2017 at 22:34
1

You are asking if the return type is a good way to deal with multiple outcomes.

Usually you would like a method to only return one thing because that is the easiest thing to implement, understand and maintain.

That leaves us with either exceptions to handle errors or splitting up the service call into multiple service calls.

I am leaning towards that Exceptions would be the "correct" way for your scenario, unless you now that callers frequently are going to be calling the service with wrong parameters and that it will hurt the performance so much, so it would be a problem. (my machine can easily process 10000 exceptions per second).

In that case, splitting up the service call into multiple calls may be a good solution, pseudo-code:

public ActionResult Resize(string path, int width, int height)
{
    if (resizeService.IsValidSize(width, height) == false)
    {
        return new HttpStatusCodeResult(403, "Invalid parameters.");
    }

    if (resizeService.IsValidPath(path) == false)
    {
        return new HttpStatusCodeResult(404, "File not found.");
    }

    var resizeResult = resizeService.Resize(path, width, height);
    return Content(resizeResult);
}

The only downside in my mind by such approach is that if it the file is deleted/moved/locked, the file might be there when calling resizeService.IsValidPath, but inaccessible when the actual Resize call takes place which would then throw an Exception.

I would approximate the likelihood of that failure and if it is unlikely I would favor to keep the code clean without adding additional exception handling.

1

Some good people here have pretty much pointed out that exceptions here would be a good way to go and, personally, I agree with them. Nonetheless, for the sake of the argument, I will pretend that I completely disagree with them and will suggest one alternative approach: use polymorphism.

Instead of having a huge switch clause that you would have to expand every time you add a new type of error, you can just define a default action to be taken when the result is created. Something like this:

public class ReadFileResult
{
    public virtual void Action()
    {
        throw new Exception("Unexpected file result");
    }
}

public class FileNotFoundResult : ReadFileResult
{
    public override void Action()
    {
        Console.WriteLine("File not found");
    }
}

public class BytesFileResult : ReadFileResult
{
    public BytesFileResult(byte[] fileData)
    {
        FileData = fileData;
    }

    public byte[] FileData { get; }

    public override void Action()
    {
        Console.WriteLine(bytesFileResult.FileData.Length); //do something with the data
    }
}

class Program
{
    static void Main(string[] args)
    {
        ReadFileResult readFileResult = ReadFile("test.txt");
        readFileResult.Action();           
    }

    public static ReadFileResult ReadFile(string filename)
    {
        throw new NotImplementedException(); //some logic here
    }
}

Of course, some variations can be made here. ReadFileResult can be an abstract class or an interface, thus forcing a user to implement the type that needs to be returned. I opted here for the default implementation that matches the default block in your switch statement.

Furthermore, you will notice that Action() is a method without parameters that has no return type, and some actions might warrant a return type or some parameters. This can easily be achieved using dependency injection. A good example of this is shown in BytesFileResult class. Alternatively, you could provide a collection of objects that you could use as a container of whatever you need passed in or out of the method (not a fan of this solution, but it can be done).

Not the answer you're looking for? Browse other questions tagged or ask your own question.