Is it "bad" to use try-catch for flow control in .NET?

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

Solution 1:

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.

Solution 2:

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.

Solution 3:

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.

Solution 4:

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.

Solution 5:

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.