Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
3  
I wrote an answer then realized my answer was becoming "subjective and argumentative", which clearly indicates the question is not a good fit for the Q&A format. –  Eric J. Aug 1 '12 at 19:10
3  
"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 :) ) –  Michael Edenfield Aug 1 '12 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. –  George Stocker Aug 1 '12 at 21:18
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 '12 at 22:23
6  
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. –  Robert Harvey Aug 2 '12 at 3:17
show 6 more comments

migrated from stackoverflow.com Aug 2 '12 at 2:17

This question came from our site for professional and enthusiast programmers.

7 Answers

up vote 11 down vote accepted

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.

share|improve this answer
add comment

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.

share|improve this answer
    
This is the answer. –  Den Nov 11 '13 at 21:47
add comment

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

share|improve this answer
3  
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. –  Scott Whitlock Aug 2 '12 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 '12 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? –  Virtlink Aug 2 '12 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 '12 at 20:52
    
@Telastyn I agree, but how do then validate, for example, a user-entered date without Parse (due to the exception penalty) or TryParse (due to the out parameter)? Any thoughts? –  Virtlink Aug 2 '12 at 21:11
show 2 more comments

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.
share|improve this answer
    
+1 for KeyNotFoundExceptions. –  Wyatt Barnett Aug 2 '12 at 2:47
    
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 '12 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. –  Jirka Hanika Aug 2 '12 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. –  Virtlink Aug 2 '12 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.) –  Jirka Hanika Aug 2 '12 at 20:49
show 3 more comments

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.

share|improve this answer
add comment

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;
        }
    }
share|improve this answer
3  
try catch will wreak havoc on your performance if you're calling GetXElement() and failing many times. –  Robert Harvey Aug 1 '12 at 22:19
    
sometime it's doesn't matter. Take a look on Bag call. Thanks for your observation –  GSerjo Aug 1 '12 at 22:30
add comment

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.

share|improve this answer
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. –  Robert Harvey Aug 2 '12 at 3:24
    
That's a more specialist need, not the general case. –  DeadMG Aug 3 '12 at 4:30
1  
So you say. But you did use the word "always." :) –  Robert Harvey Aug 3 '12 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. –  Gerald Davis Apr 12 at 16:19
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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