using try-catch for flow control (.NET)

Question

I just found in a project:

try
{
    myLabel.Text = school.SchoolName;
}
catch
{
    myPanel.Visible = false;
}

I want to talk to the developer than wrote this, saying that incurring the null exception (because school might theoretically be null, not myLabel) would virtually make the computer beep three times and sleep for two seconds. However, I wonder if I'm misremembering the rule about that. Obviously, this isn't the intended use for try/catch, but is this bad because it defies intention, or bad because of performance considerations? I feel like it's just bad, but I want to say more than "that's really bad".

Answer 1

I think this is bad because it is coding against an exception for one and it will also inherit unnecessary overhead. Exceptions should only be caught if they are going to be handled in a specific way.

Exceptions should be caught specifically for Exceptional cases that you can not predict, in this case it is a simple check to see if school can be null, in fact it is expected that school might be null (since the label is set nothing). If school was null and it should not have been than it should throw its own ArgumentNullException.

Answer 2

I never like using exceptions for flow control. Exceptions are expensive, and it is difficult to determine what the actual flow of a program with exceptions being thrown to reach other places in code. To me this is like using GoTo. This doesn't mean that you should avoid exceptions, but rather an exception should be just that, an exception to what should normally happen in the program.

I think a worse part of the code, is that it's not even doing anything with the exception. There is no logging or even an explanation as to why the exception is being thrown.

Answer 3

You should not use exceptions for control flow simply because it is bad design. It doesn't make sense. Exceptions are for exceptional cases, not for normal flow. Performance probably won't be an issue in this situation because for most modern applications on modern hardware, you could throw exceptions all day long and the user wouldn't notice a performance hit. However, if this is a high performance application processing a lot of data or doing a lot of some sort of work, then yes, performance would be a concern.

Answer 4

You are absolutely right that this is bad. It is bad because it defies intention and because it hurts performance.

I realize there is room for different programming styles, but personally, I think that even though this works, and I can see what the code is attempting to do, it also hurts readability and code clarity, making it that much more difficult for the maintenance programmers to follow. An if statement is much more appropriate here.

Answer 5

Throwing exceptions does have a negative impact on performance, see http://msdn.microsoft.com/en-us/library/ms229009%28VS.80%29.aspx

Answer 6

Exceptions do incur runtime overhead, but it's probably negligible here. There will be a difference running in the debugger, but the built binaries should run at pretty much the same speed.

Tell your developer that any chimp can make code the machine can read. Good code is written for human beings, not machines. If a null exception is the only thing you're worried about, then it's probably a bug in the user's code -- noone should ever try to assign null to anything that way. Use an Assert() statement instead.

Answer 7

In my opinion this is poor because it could be made much more clear with an if statement:

if (school != null) {
    myLabel.Text = school.SchoolName;
}
else {
    myPanel.Visible = false;
}

That will certainly avoid using exception handling unnecessarily and make the code's meaning very obvious.

Answer 8

Check out this post on "Why not use exceptions as regular flow of control?"

Answer 9

I agree with everyone here--it's a horrible idea.

There are a few cases in Java (I think they are mostly gone now, but there may still be some in external libraries) where you were required to catch an exception for certain "non-exception" cases.

In general, when writing library code (well, any class actually), avoid using exceptions for ANYTHING that could possibly be avoided. If it's possible that a name field isn't set and that should cause an exception in a write() method, be sure to add an isValid() method so that you don't actually HAVE to catch the exception around the write to know there is a problem.

(Bad Java code addendum): this "good" programming style virtually eliminates any need for checked exceptions in Java, and checked exceptions in Java are the Suck.