Return magic value, throw exception or return false on failure?

Question

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:

• return a magic value that has no meaning otherwise:
  • Collection<T>.IndexOf returns -1,
  • StreamReader.Read returns -1,
  • Math.Sqrt returns NaN,
  • Hashtable.Item returns null;
• throw an exception:
  • Dictionary<,>.Item throws KeyNotFoundException,
  • Double.Parse throws FormatException; or
• return false and provide the actual return value in an out parameter:
  • Dictionary<,>.TryGetValue,
  • Double.TryParse.

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.

Answer 1

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.

Answer 2

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.

Answer 3

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.

Answer 4

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).

Answer 5

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.

Answer 6

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.

Answer 7

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;
        }
    }

Answer 8

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));
    }