CODEWITHSUNDEEP
c#.nettry-catch
Christopher Jones
Christopher Jones

Reputation: 257

Using Catch Blocks to Return, Bad Idea?

This may be a stupid question, but I'm interested in the performance of using try/catch blocks.

I have a DataGrid that assigns a Converter to the background property of a DataGridCell. In the converter, I compare the value of this year's data to last year's data; if this year's data is > 3%, I return a Green background; if it's > 0% and < 3%, I return a Yellow; and if it's < 0%, I return Red:

string x = values[0].ToString().Replace("$", "").Replace(",", ""); //This year's number
string y = values[1].ToString().Replace("$", "").Replace(",", ""); //Last year's

result = (((float.Parse(x) * 100) / float.Parse(y)) - 1) * 100;

if (result >= 3)
    return Brushes.LimeGreen;
else if (result >= 0)
    return Brushes.Yellow;
else
    return Brushes.Red;

However, in some cases, the cell will NOT have a value of last year; as you can guess, dividing by 0 (or some text as the Converter seems to receive when the Cell is empty) is a pretty bad idea and will throw an exception. So, I decided the easiest way to deal with this was:

try
{
    result = (((float.Parse(x) * 100) / float.Parse(y)) - 1) * 100;
}
catch
{
    return Brushes.DarkOrange;
}

So if the exception is thrown (whereby there is no value to compare to), return an Orange and call it a day. (edit: yes, I do wish to return an Orange when there is no value to compare to.

Currently, I can predict that it will only happen to one row of data for now, so it only catches about 10 cells when it's populated. But as the future goes on, it has the possibility of having it happen more times.

The try/catch block is the easiest and quickest way to handle this(as far as I can tell), but it is obviously not the only way, especially since I know the error. So Is using a try/catch block a bad idea in this case? And by bad idea, I mean will it slow performance since it is iterated over many, many times? Given that I know what the error will be, should I preempt it, or is using a try/catch block fine?

Upvotes: 1

Views: 292

Answers (7)

Cronan
Cronan

Reputation: 193

I believe Rico Mariani has the last word, as usual:

The True Cost of .NET Exceptions

Yes, using a Try/Catch block is a very bad idea.

Upvotes: 2

jason
jason

Reputation: 241769

This is a bad idea. It's not an exceptional situation, so don't handle it like it's one. The fact that you're worried about performance and exception handling is a smell that you're doing something wrong. There is a really simple way to handle this, so I don't even see why you ever thought to instead handle it using exception handling.

Yes, it will impact your performance but that shouldn't be your concern here. Your concern should be what is the clearest way to write this logic, and using exceptions is clearly not the way.

Here's that simple way:

double value;
if(!Float.TryParse(y, out value) || value == 0f) {
    return Brushes.DarkOrange;
}
else {
    double result = (((float.Parse(x) * 100) / value) - 1) * 100;
    if (result >= 3) {
        return Brushes.LimeGreen;
    }
    else if (result >= 0) {
       return Brushes.Yellow;
    }
    else {
        return Brushes.Red;
    }
}

Upvotes: 10

Justin Rusbatch
Justin Rusbatch

Reputation: 4052

Using the catch block in the way you described in your question is a bad idea. Try-catch blocks themselves do not have a huge performance impact, but exceptions do have a large performance cost.

The best option is to use the TryParse method on the float class instead. There is a reason it exists.

Upvotes: 2

Ignacio Soler Garcia
Ignacio Soler Garcia

Reputation: 21855

The try catch has one of the worst performances on the .Net framework. Look here When an exception is raised the stacktrace has to be checked to find in which method there is a catch for that specific exception and this is a costly operation.

Never do things like that with exceptions if you have performance in mind (and I would say even if you don't have performance on mind).

Upvotes: 3

steelshark
steelshark

Reputation: 644

Catch statements should only be used for catching Exceptions..Not altering the program flow, So to answer your question, yes this is a bad idea. And I can't imagine it to be performant, because a new Exception object will be created and most likely java will do some logging of the event, so your better of just using an if statement.

Upvotes: 2

Quickhorn
Quickhorn

Reputation: 1181

I am not familiar enough with the intricacies of c# to tell you whether an if, or a tryparse statement or a try catch are going to be faster. What I can offer you is the idea of thinking about calls.

If you're worried about performance on something as simple as implementation of the same process, think about the number of calls as well. If you check every single number if it's 0, or call try-parse on every single number then you're adding that call to every iteration, not just the iterations that fail. If this implementation works for you, it is a good approach as it only gets into the catch block when it fails.

For many programs, this sort of implementation wouldn't work, because they want to do additional processing in the catch block. For those programs, you'd want to check for the 0, or do the tryparse. Since you're just returning, I believe this makes for easily readable code.

Upvotes: 1

goric
goric

Reputation: 11905

In general, exceptions should be used for truly exceptional cases - cases which are not expected. In a situation like this, where you're well aware that there could be no prior year, I'd suggest using simple if() checks to return your default values.

To answer your performance question, Microsoft says that "Throwing exceptions can negatively impact performance".

Upvotes: 7

Related Questions