views:

131

answers:

4

I have a large project in C# (.NET 2.0) which contains very large chunks of code generated by SubSonic. Is a try-catch like this causing a horrible performance hit?

for (int x = 0; x < identifiers.Count; x++)
        {decimal target = 0;
            try
            {
                target = Convert.ToDecimal(assets[x + identifiers.Count * 2]); // target %
            }
            catch { targetEmpty = true;  }}

What is happening is if the given field that is being passed in is not something that can be converted to a decimal it sets a flag which is then used further along in the record to determine something else.

The problem is that the application is literally throwing 10s of thousands of exceptions as I am parsing through 30k records. The process as a whole takes almost 10 minutes for everything and my overall task is to improve that time some and this seemed like easy hanging fruit if its a bad design idea.

Any thoughts would be helpful (be kind, its been a miserable day)

thanks, Chris

+3  A: 

There's a TryParse for Decimal as well. It avoids the exception and uses a bool instead to signal success/failure.

Brian Rasmussen
+5  A: 

Using exceptions for control flow is generally a bad practice (exactly because of the poor efficiency that you're observing). What type of data do you need to convert to decimal? Could you use TryParse method or some other method that doesn't throw exception if the input is not in the expected format?

Decimal.TryParse method should do the trick if you're parsing strings as it reports failure by returnning false:

decimal d;
if (Decimal.TryParse(str, out d)) 
  // Ok, use decimal 'd'
else 
  // Failed - do something else
Tomas Petricek
many thanks, I replaced the 3 places where that particular horrid code was with the above and now the whole thing finished in LESS than 30 seconds!
SomeMiscGuy
+4  A: 

That is a truly horrible looking piece of code. In general exceptions should only be used to capture unexpected results. The fact that you are getting lots of exceptions means that this is a common condition that should become part of the intrinsic logic.

decimal.TryParse would be a far better option here and I would suggest caching the value of identifiers.Count * 2 as well (not sure if compiler will optimise that)

James Westgate
A: 

It's scary looking code, and you're getting good suggestions on how to fix it.

If you want to know if those exceptions are costing you a big percentage of time, there's a simple way to find out.

Just pause it about 10 times, and each time examine the call stack. If the exceptions are costing some percentage of time like, say, 50%, then you will see it in the process of throwing or catching them on roughly that percent of pauses.

Mike Dunlavey