.NET Return value vs thrown Exception Design question

Question

Let's assume we have a method handling operations in a tree hierarchical data structure located in a class handling such a structure.

Let's look closer at one of those methods:

void MoveNode(Node currentNode, Node newParentNode)
{
    /* check if new parent isn't current's child already */
     if (newParentNode.LMargin < currentNode.LMargin && newParentNode.RMargin > currentNode.RMargin)
     {
        //DO WORK
     }
     else throw new ArgumentException("New parent node cannot be current's own child");
}

MSDN states: do not throw Exceptions to control the flow !

My question: is this use of ArgumentException alright in your opinion, or would you use some kind of a return value. If so, how would you supply the error/validation message.

Answer 1

Since the exception being thrown indicates a bug here, and it thus won't be ever thrown in a correctly working program and exception is the right choice.

What you should not do is:

try
{
   MoveNode(...)
   //Do something
}
catch(ArgumentException e)
{
  //Do something else
}

In that example you expect the exception being thrown regularly and use it to control the control flow. Catching an ArgumentException in the caller is almost always a bad idea. This kind of exception should only be caught in the top-level handler, if at all.

Personally I don't like you throwing the exception in the else clause. I prefer doing my parameter checking at the beginning of the function and throw the exception immediately afterwards. That prevents nesting the non error code within multiple if blocks.

There are three types of exceptions

1. Asynchronous exception like StackOverflow, OutOfMemory and ThreadAborted. They can happen anywhere and can't really be handled
2. Bug exceptions like ArgumentException, log them in the top level handler and fix the bug
3. Expected exceptions indicating an error that can be handles locally. Typically you use these when errors are uncommon, and you can't know beforehand that an operation will cause an error. IO errors are a typical example of this.
   The cause is typically external. For example an inaccessible file, a network failure or invalid data in a file you're trying to parse.

Eric Lippert talk about these kinds of exception in a blog entry: Vexing exceptions

When to use the third kind of exception, and when to use return values is a judgment call.

Answer 2

I don't think this is the case of "not to use exception for control the flow".

This is argument validation. The method cannot work input parameters aren't valid, so, using an exception is the best way to tell the caller that it's trying to do a bad business.

Answer 3

If it is a unexpected runtime error, as here, it's an exception, otherwise it should be a return value. Model your decision on int.Parse (throws) / int.TryParse (return value), the first is for circumstances where you know things must be int (parsing a typed structure for example), the other is for validating user input (typing errors are expected from the user).

Exception handling is runtime expensive when thrown, it should avoided in a loop.

Answer 4

Rather than ask whether something should return a value or throw an exception, one should ask what a function is promising to do. If a function promises to move a node, it should throw an exception if it can't. If a function promises to move a node if possible, or indicate via return value that it can't be moved, but only throw an exception if the data structure is fundamentally corrupt in some way beyond that implied by the ability to move the node, it should do that. Sometimes it can be useful to provide functions of both the "do it" and "try it" varieties.

As for what type of exception to throw, I frankly dislike the concept of throwing most built-in exception types from user code, since there's no nice programmatic way to tell whether an ArgumentException was thrown from your routine, or from some routine that was called by your routine, and most exceptions say nothing about the quality of the underlying data structure.

If one is trying to e.g. parse a file from disk and integrate it with an existing data structure and an exception is thrown, it doesn't matter whether the exception was an ArgumentException, or a SubscriptOutOfBoundsException, or a DiskReadErrorException, or whatever. The most important thing is whether the parse attempt was rolled back in such a fashion as to leave the data structure valid; of secondary importance is whether it might be possible to parse the file another way or under other circumstances. The type of exception really only matters to the extent that it can answer those first two questions.