3

So I'm working on this code base, and every single method is contained within a try-catch block, which just logs the exception to a logging file.

I'm considering waving my hands around and trying to get this changed, but I'm gone in a month, and I'm not sure how much it really slows the code down. Yeah, it's horrible practice, but that's par for the course. Yeah, it makes errors more difficult to debug, but it's "easier".

The deciding factor for people here is speed. So I'm wondering how much slower would this really make the code? I'm asking for a ballpark estimation from someone who knows compilers much better than I do.

I know there's a lot of duplicate questions about whether or not exceptions slow things down, and that it differs depending on compiler versions and the like, but I'm looking for more of a factor of/ some advice here. I'm also just plain curious.

NathanTempelman
  • 1,301
  • 9
  • 34
  • 7
    Why not doing some benchmarking? – Hossein Narimani Rad Dec 05 '13 at 19:10
  • Like @HosseinNarimaniRad says, take the Try Catches out, run a batch of operations. Then put them back in and run the same batch. – David Pilkington Dec 05 '13 at 19:13
  • 2
    This is likely going to end up falling in the "micro-optimizing" category. If you're looking to improve performance significantly, I'm sure you'll find better places to look. I'd start with database access and indexes, and move on to things like looping through large collections unnecessarily, small jobs than can be batched together, things like that. – Joe Enos Dec 05 '13 at 19:13
  • It inserts some additional IL so yeah it's obviously slower, but who cares. It's more of a style issue and like @Joe Enos says there are probably bigger fish in that pond. – scottheckel Dec 05 '13 at 19:16
  • @HosseinNarimaniRad Its kind of a massive code base. I did it with a few methods and ran them a few hundred times either way, and it seemed fairly negligable. But I was thinking someone with some expertise could give me a hand wavey overview. – NathanTempelman Dec 05 '13 at 19:16
  • 1
    Do the catch blocks in every method swallow the exception, or rethrow it? – hatchet - done with SOverflow Dec 05 '13 at 19:16
  • @hatchet Swallow. Just writes it to one big log file. – NathanTempelman Dec 05 '13 at 19:19
  • 6
    _every single method is contained within a try-catch block_ means that you have code-quality, maintenance and probably correctness problems. I wouldn't worry about speed so much. – H H Dec 05 '13 at 19:19
  • 4
    @NathanTempelman: "Swallow. Just writes it to one big log file." If your try/catch blocks are simply _swallowing_ exceptions and letting the application fail in undefined ways silently, I'd suggest you have bigger problems than try/catch performance. – Chris Sinclair Dec 05 '13 at 19:20
  • 2
    If they swallow the exceptions, then removing the try catch blocks would substantially alter the application behavior. I would avoid making that change for that reason unless you're willing to put an awful lot of work into the conversion. BTW...what a mess. – hatchet - done with SOverflow Dec 05 '13 at 19:21
  • See http://thedailywtf.com/Articles/Exceptionally-Hard-to-Swallow.aspx for a cautionary tale.. – stuartd Dec 05 '13 at 19:44
  • @HenkHolterman Yeah, I'm very aware, but speed seems to be the only thing people care about. That's probably the reason it's so slow.. Kind of funny when you think about it. – NathanTempelman Dec 05 '13 at 21:10

3 Answers3

6

It's probably not going to slow down the running application, since it doesn't actually do anything until an exception is thrown. (Unless you're really trying to squeeze every bit of performance, and most applications don't need to do that. Not to mention that this coding style implies very heavily that this application probably has lots of other more prevalent problems.)

What it is doing is polluting the code with tons of unnecessary exception catching. (Note the difference between catching and meaningfully handling. I can guarantee you that this code is doing the former, not the latter.) It's slowing down development, and hours of developer time is a lot more expensive than milliseconds of system time.

Yeah, it's horrible practice, but that's par for the course.

Sounds like you've already left :)

Yeah, it makes errors more difficult to debug, but it's "easier".

Those are two mutually-exclusive statements. It can't be both more difficult and easier. Someone somewhere thought it would be easier. They were mistaken.

How much would wrapping every method in try-catch blocks slow a program down?

Measured in development time, a lot.

David
  • 208,112
  • 36
  • 198
  • 279
  • Yeah, this is basically what I figured. Without a huge speed boost to point to, I don't think these points would have much sway. Thanks for the answer though, it was well written and intelligent. – NathanTempelman Dec 05 '13 at 20:38
3

Putting a try/catch in every method to catch the general exception ends up being a pain for reasons other than performance. Namely, the application continues to "function" regardless of the state of the application. In other words, not all Exceptions can or should be handled. E.G. What happens when the database goes down, the email server, a bad transaction (or something that should have been transactional but wasn't because poor design) etc...?

I've worked in similar environments. The concern ultimately was not one of performance. I fear places that throw around "performance reasons" as a general vague, catch-all reason to make otherwise arbritrary decisions...I digress.

However, if you are out in a month then I caution you to consider whether the argument is warranted. The development shop has proven to below your standards. It's important not to burn a bridge as a bad reference can cost you a future position.

P.Brian.Mackey
  • 43,228
  • 68
  • 238
  • 348
  • 1
    `"if you are out in a month there's little reason to worry about it"` - I was hovering over the up-vote button until I read that... – David Dec 05 '13 at 19:20
  • @David - I only say that because some battles are not worth fighting. In my experience, making such a drastic change can easily result in "burning a bridge". That's not a wise political move for references. It's important to be mindful of one's career as well as technical decisions. If the OP feels they can make such a change without "incurring the wrath of the boss" then by all means make the proper technical decision. – P.Brian.Mackey Dec 05 '13 at 19:22
  • A valid point, but still an unprofessional approach. If the employer *insists* on terrible software practices, they're not a bridge to keep around anyway. Take it as an opportunity to research a subject and present a cogent argument. Sure, it may be a complete loss. But giving up and just staying with the status quo regardless of one's own principles is *not* good career development. – David Dec 05 '13 at 19:24
  • The OP shows initiative and a desire to improve simply by participating on SO. I call that good career development. I have suffered through situations where I presented many arguments that were ignored and it cost me my position. Considering the OP has already decided to quit I suspect they are in a similar predicament. In which case it is a bad move because references do matter. – P.Brian.Mackey Dec 05 '13 at 19:27
  • @David You both have good points, so thanks for those. I think I might mention it and see where it goes, give me a chance to get my coherent argument on. That said, I don't think I'm going to push very hard. We'll call it a compromise. I also found a home brew add element to array method that resizes the array on every element add, and it's used thousands of times. I figure using linked lists instead should have quite the performance boost, so maybe that'll give me a peace offering if the discussion goes awry. – NathanTempelman Dec 05 '13 at 20:46
2

Code within a try block is limited in how it can be optimized by the compilers. You would, in effect, limit optimizations if you wrapped every method with a try/catch. See http://msmvps.com/blogs/peterritchie/archive/2007/06/22/performance-implications-of-try-catch-finally.aspx and http://msmvps.com/blogs/peterritchie/archive/2007/07/12/performance-implications-of-try-catch-finally-part-two.aspx

The degree to which performance would be affected would be application-specific.

Peter Ritchie
  • 35,463
  • 9
  • 80
  • 98