Throwing an exception when a method does not complete or implement a work around?

Question

I have been studying this subject quite a bit, and I am unsure of what's the best way. I still have a lot to go trough (books, blogs, stack exchange, etc), but I simply can't find a consensus.

Basically here is the situation: You have a function that is expected to return a result, but the function fails (parameter is bad, exception occurs, resource is blocked / unreachable, etc), should you throw an exception or return something?

I simply don't see a best way of doing this, here is how things look:

People seem to agree that returning null and coding for null (expecting null and checking for it) is bad practice and I wholeheartedly agree. But on the other side of things people don't seem to like "vexing exceptions", for example Int32.parseInt throws an exception instead of returning null, this function has a counterpart that does not do that (tryParse) which returns a boolean but has an output parameter:

bool result = Int32.TryParse(value, out number);
if (result)
{
    Console.WriteLine("Converted '{0}' to {1}.", value, number);         
}

I personally find this method a bit vexing, I studied threading under linux and most methods were methodA(in, in, out, in) which is horrible. With a proper IDE this issue is way smaller, but it still left a bad taste in my mouth.

And in some languages (like Java) this is not exactly easy to do, since it has no out descriptor (or whatever it's called) and it passes by value.

So it comes down to this: How would this be better handled? Add exceptions to when your code can't complete (if it isn't a number, I can't parse it, I can't return a number), or should you work around the issue. In C, C++, or C# I would probably always go with the bool method(in, out) approach, but in Java this seems like a really painful way of solving the issue, and I'm not sure if it's available in other languages or not.

Answer 1

One possibility is to use an Optional generic. For example, see one from Guava:

http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Optional.html

In this case, your function becomes

Optional<Int32> parseInt(String);

Now the calling code has to explicitly decide how it wants to handle the failure case. The Guava Optional generic has a variety of methods to make common use cases easy.

In some ways, this is like a null, in that you include the error state as a return value. However, this is explicit. If you try to pass a Optional where an Int32 is expected, you'll get a compiler error. At the point where Optional becomes an Int32, you know that you've got to think about handling the failure case.

Answer 2

The modern line of thinking, which is considered as best practice by most publications and in most workplaces out there, is to never return result codes, and instead to always throw an exception if you cannot return a valid value. (But read more, there is a twist.)

The main reason why C# offers tryParse() is because in Microsoft Dot Net throwing an exception is outrageously slow, (because the DotNet runtime works in promiscuous collusion with the underlying Windows Operating System,) so you cannot really afford to have exceptions thrown and caught during normal flow of operations. By contrast, in Java throwing an exception and catching it is relatively inexpensive, (because it gets handled entirely within the VM without hitting the underlying operating system, whichever that may be,) so people do it all the time and are not afraid of it.

If you are really concerned about the performance overhead of exceptions, then you may "bend" the best practices to make them suit your needs.

Best practices dictate that any exceptional situation must be reported by means of an exception, while any non-exceptional situation must not be reported by exception. Thus, you can redefine, if you wish, what a non-exceptional situation is, as to include, for example, the case where you are unable to parse a given string. If you expect failures to be common, this is reasonable. So, in this scenario, it is permissible to indicate a parsing failure by some means other than an exception, because you have just redefined failure-to-parse as a non-exceptional situation.

So, in C#, besides the (admittedly awkward) try...() style of methods you can always return a nullable primitive (int?) or that "optional generic" suggested by Winston Ewert. In java, you can always return a boxed primitive, (Integer instead of int,) which can be null.

Answer 3

On the one hand, it all depends on the contract of the procedure. Whatever you choose, you should make sure that what you expect of the caller (e.g., preconditions) and what the caller can expect from you (e.g., postconditions) are clear, including what the caller should expect on failure. If you put the burden of checking return values on the caller, make sure the caller knows.

I favor exceptions. Exceptions are beneficial because they make it difficult for client code to silently ignore the error. In Java, for instance, non-Runtime exceptions must be explicitly caught or declared as thrown; Runtime exceptions generally shouldn't be caught and are intended to terminate the program due to a bug. Exceptions also make it easier to separate the "real" logic of solving the problem from the "error handing" logic, which make the code easier to read.

What I wouldn't suggest is mixing using exceptions with using return codes – that'll confuse anyone using your API.

Answer 4

You should throw an Exception.

People like the TryParse because otherwise you need an IsParsable function which probably does the same thing under the hood.

Personally I try to define my functions such that common exceptions can be handled within the function itself. eg

//returns empty list when no objects are found
IEnumerable<Objects> Repository.GetObjects(string search)

The thing to avoid is a situation where throwing an exception will force the calling code to use the exception as part of the logic ie.

Try {
    This();
}
catch() {
    That();
}

As this is both expensive and prone to error when unexpected exceptions are thrown.

Answer 5

In general, throw an exception, do not implement a work around.

The basic principle is to write the method such that it is clear what the result will be from calling the function, and for the caller to expect that the function will succeed in its purpose.

The Int.TryParse() stretches this a little (but not beyond breaking point) in that it is clearly implied that the return from this function returns true/false on whether the string can be parsed into an integer. The return value of false implies that the function successfully determined that the string parameter was not a correctly formatted Integer.

The key, is that it is not instead of exceptions. This function will still throw exceptions (e.g. ThreadAbortException, InvalidArgException etc...). Exceptions are not being hidden by a return value of 'false'.

When to use the Try Implementation?

I would typically use the Int.Parse() in normal use.

The exception to this, is if I was writing the actual parsing function for a larger data set. Perhaps a configuration file, or validating user data entered in a UI. In these situations the actual purpose of the code is to do a series of validation checks on the data, and I am at the right point in the code to implement different actions based on data formatting.

If my only other option was to do:

try
{
   ...
}
catch(ParseException ex)
{
   //  do this instead....
}

and I would swallow the exception, then using the TryParse() makes sense.

Answer 6

It depends, but if you are in doubt, throw an exception.

The one thing you should never do, its to return an error indicator value which is the same type as a legitimate result. Like having IndexOf() returning -1 if the substring is not found. This is bad because you easily forget to check for this special value, so you might hide errors without handling them, which leads to hard-to-find bugs. Returning null also have this issue because null is a valid value for any object type, so it leads to the same problem.

The TryParse() pattern is somewhat better, since it is explicit in that it returns a flag which indicates if the operation succeeded. You can of course decide to ignore the return value, but you are not likely to do it by mistake. Furthermore, the TryParse()-method has the counterpart Parse(), which thrown an exception if parsing fails. So if your code is not able to recover form a parse error you just use the Parse() version anyway. If TryParse() didn't have the Parse() counterpart, you would run the risk of people ignoring the error code and hiding errors, but this is not likely when it is simply easier to use Parse().

So even if you implement a Try...-pattern method, you should have a counterpart which throws on error. Which leads back to the point that you as as default should implement the version which throws on error.

Answer 7

I just completed a project where I was wrapping practically ALL my methods that do this sort of work in a "message" type class, which (in C#) has a concrete boolean for success/fail, along with error/success messaging string collections. There's a generic <T> version that inherits from the non-generic version, so a method might return a plain TransactionResult (which just signals if a data call failed or succeeded, with extra messaging as needed), or a TransactionResult<string> which is the former, plus a string field which holds the return value (in this case, a string).

Now I can just inspect the returned object's 'Success' field, without having to know that "-1" means "string not found". I could even wrap my "ResultObject" property with a backing private field and check it for null when setting (if I was so inclined). No more error handling for common stuff, and overall its been quite easy to implement.

public class TransactionResult
    {
        public bool Success { get; set; }
        public ValidationResult[] TransactionErrors { get; private set; }
        public string[] SuccessMessages { get; private set; } // in case you need to signal a partial success, or a custom result message

        public TransactionResult()
        {
            TransactionErrors = new ValidationResult[0];
            SuccessMessages = new string[0];
            Success = true; // Easier to let it be 'true' by default, and 'false' only when you add an error or set it manually!
        }

        public void AddTransactionError(string errorMessage, string[] memberNames = null)
        {
            var newError = new ValidationResult(errorMessage, memberNames ?? new[] { "" });
            TransactionErrors = TransactionErrors.Concat(new[] { newError }).ToArray();
            Success = false;
        }

        public void MergeWithErrorsFrom(TransactionResult transaction)
        {
            if (transaction == null || !transaction.TransactionErrors.Any())
                return;

            SuccessMessages = SuccessMessages.Concat(transaction.SuccessMessages).ToArray();
            TransactionErrors = TransactionErrors.Concat(transaction.TransactionErrors).ToArray();
            if (TransactionErrors.Any())
                Success = false;
        }

        public void AddSuccessMessage(string message)
        {
            SuccessMessages = SuccessMessages.Concat(new[] { message }).ToArray();
        }
    }

    public class TransactionResult<T> : TransactionResult
    {
        public T ResultObject { get; set; }

        public void MergeWithResultsFrom(TransactionResult<T> transaction)
        {
            MergeWithErrorsFrom(transaction);
            ResultObject = transaction.ResultObject;
        }
    }

The above class uses the System.ComponentModel.DataAnnotations.ValidationResult class as its sorta backing "error" class, since I'm working in MVC4, which means my results can bind to my UI pretty much automatically, but you could roll your own version of that error as well. I wouldn't mind working in a language where this sort of structure was automatically part of the language/framework instead of having to roll my own, but that's pretty opinionated.