96

I sometimes end up having to write a method or property for a class library for which it is not exceptional to have no real answer, but a failure. Something cannot be determined, is not available, not found, not currently possible or there is no more data available.

I think there are three possible solutions for such a relatively non-exceptional situation to indicate failure in C# 4:

  • return a magic value that has no meaning otherwise (such as null and -1);
  • throw an exception (e.g. KeyNotFoundException);
  • return false and provide the actual return value in an out parameter, (such as Dictionary<,>.TryGetValue).

So the questions are: in which non-exceptional situation should I throw an exception? And if I should not throw: when is returning a magic value perferred above implementing a Try* method with an out parameter? (To me the out parameter seems dirty, and it is more work to use it properly.)

I'm looking for factual answers, such as answers involving design guidelines (I don't know any about Try* methods), usability (as I ask this for a class library), consistency with the BCL, and readability.


In the .NET Framework Base Class Library, all three methods are used:

Note that as Hashtable was created in the time when there were no generics in C#, it uses object and can therefore return null as a magic value. But with generics, exceptions are used in Dictionary<,>, and initially it didn't have TryGetValue. Apparently insights change.

Obviously, the Item-TryGetValue and Parse-TryParse duality is there for a reason, so I assume that throwing exceptions for non-exceptional failures is in C# 4 not done. However, the Try* methods didn't always exist, even when Dictionary<,>.Item existed.

11
  • 6
    "exceptions mean bugs". asking a dictionary for a non-existant item is a bug; asking a stream to read data when it's an EOF happens every time you use a stream. (this summarizes the long a beautifully formatted answer I didn't get a chance to submit :) )
    – KutuluMike
    Aug 1, 2012 at 19:24
  • 6
    It's not that I think your question is too broad, it's that it's not a constructive question. There's no canonical answer as the answers are already showing. Aug 1, 2012 at 21:18
  • 2
    @GeorgeStocker If the answer was straight-forward and obvious then I wouldn't have asked the question. The fact that people can argue why a given choice is preferable from any standpoint (such as performance, readability, usability, maintainability, design guidelines or consistency) makes it inherently non-canonical yet answerable to my satisfaction. All questions can be answered somewhat subjective. Apparently you expect a question to have mostly similar answers for it to be a good question. Aug 1, 2012 at 21:23
  • 3
    @Virtlink: George is a community-elected moderator who volunteers a great deal of his time to help maintain Stack Overflow. He stated why he closed the question, and the FAQ backs him up.
    – Eric J.
    Aug 1, 2012 at 22:23
  • 16
    The question belongs here, not because it is subjective, but because it is conceptual. Rule of thumb: if you're sitting in front of your IDE coding, ask it on Stack Overflow. If you're standing in front of a whiteboard brainstorming, ask it here on Programmers. Aug 2, 2012 at 3:17

8 Answers 8

60

I don't think that your examples are really equivalent. There are three distinct groups, each with it's own rationale for its behaviour.

  1. Magic value is a good option when there is an "until" condition such as StreamReader.Read or when there is a simple to use value that will never be a valid answer (-1 for IndexOf).
  2. Throw exception when the semantics of the function is that the caller is sure that it will work. In this case a non-existing key or a bad double format is truly exceptional.
  3. Use an out parameter and return a bool if the semantics is to probe if the operation is possible or not.

The examples you provide are perfectly clear for cases 2 and 3. For the magic values, it can be argued if this is a good design decision or not in all cases.

The NaN returned by Math.Sqrt is a special case - it follows the floating point standard.

3
  • 11
    Disagree with number 1. Magic values are never a good option as they require the next coder to know the significance of the magic value. They also hurt readability. I can think of no instance in which the use of a magic value is better than the Try pattern.
    – 0b101010
    Feb 1, 2016 at 17:53
  • 2
    But what about the Either<TLeft, TRight> monad?
    – sara
    May 20, 2016 at 19:04
  • 2
    @0b101010: just spent some time looking up how streamreader.read can safely return -1...
    – jmoreno
    Feb 18, 2018 at 1:16
36

You're trying to communicate to the user of the API what they have to do. If you throw an exception, there's nothing forcing them to catch it, and only reading the documentation is going to let them know what all the possibilities are. Personally I find it slow and tedious to dig into the documentation to find all the exceptions that a certain method might throw (even if it's in intellisense, I still have to copy them out manually).

A magic value still requires you to read the documentation, and possibly reference some const table to decode the value. At least it doesn't have the overhead of an exception for what you call a non-exceptional occurrence.

That is why even though out parameters are sometimes frowned upon, I prefer that method, with the Try... syntax. It's canonical .NET and C# syntax. You're communicating to the user of the API that they have to check the return value before using the result. You can also include a second out parameter with a helpful error message, which again helps with debugging. That's why I vote for the Try... with out parameter solution.

Another option is to return a special "result" object, though I find this a lot more tedious:

interface IMyResult
{
    bool Success { get; }
    // Only access this if Success is true
    MyOtherClass Result { get; }
    // Only access this if Success is false
    string ErrorMessage { get; }
}

Then your function looks right because it only has input parameters and it only returns one thing. It's just that the one thing it returns is kind of a tuple.

In fact, if you're into that kind of thing, you can use the new Tuple<> classes that were introduced in .NET 4. Personally I don't like the fact that the meaning of each field is less explicit because I can't give Item1 and Item2 useful names.

6
  • 3
    Personally, I often use a result container like your IMyResult cause it is possible to communicate a more complex result than just true or false or the result value. Try*() is only useful for simple stuff like string to int conversion. Jun 2, 2015 at 7:14
  • 1
    Good post. For me, I prefer the Result structure idiom that you outlined above as opposed to splitting between "return" values and "out" parameters. Keeps it neat and tidy. Jun 1, 2016 at 8:54
  • 2
    The problem with the second out parameter is when you are 50 functions deep in a complex program, how do you then communicate that error message back to the user? Throwing an exception is much simpler than have layers of checking errors. When you get one just throw it and it doesn't matter how deep you are.
    – rollsch
    Oct 18, 2016 at 23:25
  • @rolls - When we use output objects we suppose that the immediate caller will do whatever he wants with the output: treat it locally, ignore it, or bubble up by throwing that wrapped into an exception. It's the best of both words - the caller can clearly see all possible outputs (with enums, etc), can decide what do to with the error, and don't need to try-catch on each call. If you're mostly expecting to handle the result immediately or ignore it, return objects are easier. If you want to throw all errors to upper layers, exceptions are easier.
    – drizin
    Jul 15, 2017 at 7:23
  • 2
    This is just reinventing checked exceptions in C# in a much more tedious way than checked exceptions in Java.
    – Winter
    Jul 18, 2018 at 21:34
18

As your examples already show, each such case must be evaluated separately and there is a considerable spectrum of gray between "exceptional circumstances" and "flow control", especially if your method is intended to be reusable, and might be used in quite different patterns than it was originally designed for. Do not expect us all here to agree on what "non-exceptional" means, especially if you immediately discuss the possibility of using "exceptions" to implement that.

We might also not agree on what design makes the code easiest to read and maintain, but I will assume that the library designer has a clear personal vision of that and only needs to balance it against the other considerations involved.

Short answer

Follow your gut feelings except when you are designing pretty fast methods and expect a possibility of unforeseen reuse.

Long answer

Each future caller may freely translate between error codes and exceptions as it wishes in both directions; this makes the two design approaches almost equivalent except for performance, debugger-friendliness and some restricted interoperability contexts. This usually boils down to performance, so let's focus on that.

  • As a rule of thumb, expect that throwing an exception is 200x slower than a regular return (in reality, there is a significant variance in that).

  • As another rule of thumb, throwing an exception may often allow much cleaner code compared with the crudest of magic values, because you are not relying on the programmer translating the error code into another error code, as it travels through multiple layers of client code towards a point where there is enough context for handling it in a consistent and adequate way. (Special case: null tends to fare better here than other magic values because of its tendency to automatically translating itself to a NullReferenceException in case of some, but not all types of defects; usually, but not always, quite close to the source of the defect.)

So what's the lesson?

For a function that's called just a few times during the lifetime of an application (like the app initialization), use anything that gives you cleaner, easier to understand code. Performance cannot be a concern.

For a throw-away function, use anything that gives you cleaner code. Then do some profiling (if needed at all) and change exceptions to return codes if they are among the suspected top bottlenecks based on measurements or overall program structure.

For an expensive reusable function, use anything that gives you cleaner code. If you basically always have to undergo a network roundtrip or parse an on-disk XML file, the overhead of throwing an exception is probably negligible. It is more important to not lose details of any failure, not even accidentally, than to return from a "non-exceptional failure" extra fast.

A lean reusable function requires more thought. By employing exceptions, you are forcing something like a 100 times slow down on callers who will see the exception on half of their (many) calls, if the body of the function executes very fast. Exceptions are still a design option, but you will have to provide a low overhead alternative for callers who can not afford this. Let's look at an example.

You list a great example of Dictionary<,>.Item, which, loosely speaking, changed from returning null values to throwing KeyNotFoundException between .NET 1.1 and .NET 2.0 (only if you are willing to consider Hashtable.Item to be its practical non-generic forerunner). The reason of this "change" is not without interest here. Performance optimization of value types (no more boxing) made the original magic value (null) a non-option; out parameters would just bring a small part of the performance cost back. This latter performance consideration is completely negligible in comparison with the overhead of throwing a KeyNotFoundException, but the exception design is still superior here. Why?

  • ref/out parameters incur their costs every time, not just in the "failure" case
  • Anyone who cares can call Contains before any call to the indexer, and this pattern reads completely naturally. If a developer wants to but forgets to call Contains, no performance issues can creep in; KeyNotFoundException is loud enough to be noticed and fixed.
7
  • I think 200x is being optimistic about the perf of exceptions... see blogs.msdn.com/b/cbrumme/archive/2003/10/01/51524.aspx "Performance and Trends" section just before the comments.
    – gbjbaanb
    Aug 2, 2012 at 10:20
  • @gbjbaanb - Well, maybe. That article uses a 1% failure rate to discuss the topic which isn't an entirely different ballpark. My own thoughts and vaguely remembered measurements would be from the context of table-implemented C++ (see Section 5.4.1.2 of this report, where one problem is that the first exception of its kind is likely to start with a page fault (drastic and variable but amortized one time overhead). But I'll do and post an experiment with .NET 4 and possibly tweak this ballpark value. I already stress the variance. Aug 2, 2012 at 11:51
  • Is the cost for ref/out parameters that high then? How so? And calling Contains before a call to an indexer might cause a race condition that does not have to be present with TryGetValue. Aug 2, 2012 at 20:22
  • @gbjbaanb - Experimented finished. I was lazy and used mono on Linux. Exceptions gave me ~563000 throws in 3 seconds. Returns gave me ~10900000 returns in 3 seconds. That's 1:20, not even 1:200. I still recommend thinking 1:100+ for any more realistic code. (out param variant, as I had predicted, had negligible costs - I actually suspect that the jitter probably had optimized the call away completely in my minimalistic example if there was no exception thrown, regardless of the signature.) Aug 2, 2012 at 20:49
  • @Virtlink - Agreed on thread safety in general, but given that you referred to .NET 4 in particular, use ConcurrentDictionary for multi-threaded access and Dictionary for single threaded access for optimal performance. That is, not using ``Contains` does NOT make the code thread safe with this particular Dictionary class. Aug 2, 2012 at 20:53
10

What is the best thing to do in such a relatively non-exceptional situation to indicate failure, and why?

You should not allow failure.

I know, it's hand-wavey and idealistic, but hear me out. When doing design, there are a number of cases where you have the opportunity to favor a version that has no failure modes. Instead of having a 'FindAll' that fails, LINQ uses a where clause that simply returns an empty enumerable. Instead of having an object that needs to be initialized before used, let the constructor initialize the object (or initialize when the not-initialized is detected). The key is removing the failure branch in consumer code. This is the problem, so focus on it.

Another strategy for this is the KeyNotFound sscenario. In almost every codebase I've worked on since 3.0, something like this extension method exists:

public static class DictionaryExtensions {
    public static V GetValue<K, V>(this IDictionary<K, V> arg, K key, Func<K,V> ifNotFound) {
        if (!arg.ContainsKey(key)) {
            return ifNotFound(key);
        }

        return arg[key];
    }
}

There is no real failure mode for this. ConcurrentDictionary has a similar GetOrAdd built in.

All that said, there will always be times where it's simply unavoidable. All three have their place, but I would favor the first option. Despite all that is made of null's danger, it's well known and fits into a lot of the 'item not found' or 'result is not applicable' scenarios that make up the "not exceptional failure" set. Especially when you're making nullable value types, the significance of the 'this might fail' is very explicit in code and hard to forget/screw up.

The second option is good enough when your user does something dumb. Gives you a string with the wrong format, tries to set the date to December 42nd... something that you want things to blow up quickly and spectacularly during testing so that bad code is identified and fixed.

The last option is one I increasingly dislike. Out parameters are awkward, and tend to violate some of the best practices when making methods, like focusing on one thing and not having side effects. Plus, the out param is usually only meaningful during success. That said, they are essential for certain operations that are usually constrained by concurrency concerns, or performance considerations (where you don't want to make a second trip to the DB for example).

If the return value and out param are non-trivial, then Scott Whitlock's suggestion about a result object is preferred (like Regex's Match class).

8
  • 7
    Pet peeve here: out parameters are completely orthogonal to the issue of side effects. Modifying a ref parameter is a side effect, and modifying an object's state that you pass in via an input parameter is a side effect, but an out parameter is just an awkward way of making a function return more than one value. There is no side effect, just multiple return values. Aug 2, 2012 at 12:07
  • I said tend to, because of how people use them. Like you say, they're simply multiple return values.
    – Telastyn
    Aug 2, 2012 at 12:58
  • But if you dislike out parameters and use exceptions to blow things up spectacularly when the format is wrong... how do you then handle the case where your format is user input? Then either a user can blow things up, or one incurs the performance penalty of throwing and then catching an exception. Right? Aug 2, 2012 at 20:26
  • @virtlink by having a distinct validation method. You need it anyways to provide proper messages to the UI before they submit it.
    – Telastyn
    Aug 2, 2012 at 20:52
  • 1
    There is a legitimate pattern for out parameters, and that is a function that has overloads that return different types. Overload resolution won't work for return types, but it will for out parameters. Aug 3, 2012 at 5:13
3

Always prefer to throw an exception. It's got a uniform interface amongst all functions that could fail, and it indicates failure as noisily as possible- a very desirable property.

Note that Parse and TryParse aren't really the same thing apart from failure modes. The fact that TryParse can also return the value is somewhat orthogonal, really. Consider the situation where, for example, you're validating some input. You don't actually care what the value is, as long as it's valid. And there's nothing wrong with offering a kind of IsValidFor(arguments) function. But it can never be the primary mode of operation.

5
  • 4
    If you're dealing with a large number of calls to a method, exceptions can have a huge negative effect on performance. Exceptions should be reserved for exceptional conditions, and would be perfectly acceptable for validating form input, but not for parsing numbers from large files. Aug 2, 2012 at 3:24
  • 1
    That's a more specialist need, not the general case.
    – DeadMG
    Aug 3, 2012 at 4:30
  • 2
    So you say. But you did use the word "always." :) Aug 3, 2012 at 5:14
  • @DeadMG, agreed with RobertHarvey although I think the answer was overly voted down, if it were modified to reflect "most of the time" and then point out exceptions (no pun intended) to the general case when it comes to frequently used high performance calls to consider other options. Apr 12, 2014 at 16:19
  • Exceptions are not expensive. Catching deeply thrown exceptions can be expensive as the system has to unwind the stack to the nearest critical point. But that "expensive" is relatively tiny and shouldn't be feared even in tight nested loops. Jul 2, 2019 at 7:53
2

As others have noted, the magic value (including a boolean return value) is not that great a solution, except as an "end-of-range" marker. Reason: The semantics are not explicit, even if you examine the methods of the object. You have to actually read the full documentation for the entire object right down to "oh yeah, if it returns -42 that means bla bla bla".

This solution may be used for historical reasons or for performance reasons, but should otherwise be avoided.

This leaves two general cases: Probing or exceptions.

Here the rule of thumb is that the program should not react to exceptions except as to handle when the program /unintentionally/ violates some condition. Probing should be used to ensure that this doesn't happen. Hence, an exception either means that the relevant probing was not performed in advance, or that something entirely unexpected has happened.

Example:

You want to create a file from a given path.

You should use the File object to evaluate in advance whether this path is legal for file creation or writing.

If your program somehow still ends up trying to write to a path that is illegal or otherwise not writable, you should get an excaption. This could happen due to a race condition (some other user removed the directory, or made it read-only, after you probl)

The task of handling an unexpected fail (signalled by an exception) and checking if the conditions are right for the operation in advance (probing) will usually be structured differently, and should therefore use different mechanisms.

0

I think the Try pattern is the best choice, when code just indicate what happened. I hate out param and like nullable object. I've created following class

public sealed class Bag<TValue>
{
    public Bag(TValue value, bool hasValue = true)
    {
        HasValue = hasValue;
        Value = value;
    }

    public static Bag<TValue> Empty
    {
        get { return new Bag<TValue>(default(TValue), false); }
    }

    public bool HasValue { get; private set; }
    public TValue Value { get; private set; }
}

so I can write following code

    public static Bag<XElement> GetXElement(this XElement element, string elementName)
    {
        try
        {
            XElement result = element.Element(elementName);
            return result == null
                       ? Bag<XElement>.Empty
                       : new Bag<XElement>(result);
        }
        catch (Exception)
        {
            return Bag<XElement>.Empty;
        }
    }

Looks like nullable but not only for value type

Another example

    public static Bag<string> TryParseString(this XElement element, string attributeName)
    {
        Bag<string> attributeResult = GetString(element, attributeName);
        if (attributeResult.HasValue)
        {
            return new Bag<string>(attributeResult.Value);
        }
        return Bag<string>.Empty;
    }

    private static Bag<string> GetString(XElement element, string attributeName)
    {
        try
        {
            string result = element.GetAttribute(attributeName).Value;
            return new Bag<string>(result);
        }
        catch (Exception)
        {
            return Bag<string>.Empty;
        }
    }
4
  • 3
    try catch will wreak havoc on your performance if you're calling GetXElement() and failing many times. Aug 1, 2012 at 22:19
  • sometime it's doesn't matter. Take a look on Bag call. Thanks for your observation
    – GSerjo
    Aug 1, 2012 at 22:30
  • your Bag<T> clas is almost identical to System.Nullable<T> aka "nullable object"
    – aeroson
    Feb 2, 2017 at 11:26
  • yes, almost public struct Nullable<T> where T : struct the main difference in a constraint. btw the latest version is here github.com/Nelibur/Nelibur/blob/master/Source/Nelibur.Sword/…
    – GSerjo
    Feb 2, 2017 at 16:41
0

If you are interested in the "magic value" route, yet another way to solve this is to overload the purpose of the Lazy class. Although Lazy is intended to defer instantiation, nothing really prevents you from using like a Maybe or an Option. For instance:

    public static Lazy<TValue> GetValue<TValue, TKey>(
        this IDictionary<TKey, TValue> dictionary,
        TKey key)
    {
        TValue retVal;
        if (dictionary.TryGetValue(key, out retVal))
        {
            var retValRef = retVal;
            var lazy = new Lazy<TValue>(() => retValRef);
            retVal = lazy.Value;
            return lazy;
        }

        return new Lazy<TValue>(() => default(TValue));
    }

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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