Communities

Writing
Writing
Codidact Meta
Codidact Meta
The Great Outdoors
The Great Outdoors
Photography & Video
Photography & Video
Scientific Speculation
Scientific Speculation
Cooking
Cooking
Electrical Engineering
Electrical Engineering
Judaism
Judaism
Languages & Linguistics
Languages & Linguistics
Software Development
Software Development
Mathematics
Mathematics
Christianity
Christianity
Code Golf
Code Golf
Music
Music
Physics
Physics

Dashboard
Notifications
Mark all as read
Q&A

Multiple catches with almost the same code.

+7
−0

I find myself often writing code like this.

try {
  // code
}
catch( FailedReadException const & ex) {
  file.close();
  std::cerr << "Read failure " << ex.readFailure() << std::endl;
  std::lock_guard<std::mutex> lock(mutexFile);
  errorState = true;
}
catch( std::runtime_error const & ex) {
  file.close();
  std::cerr << "Read failure " << ex.what() << std::endl;
  std::lock_guard<std::mutex> lock(mutexFile);
  errorState = true;
}
catch(...) {
  file.close();
  std::cerr << "Read failure " << "unexpected error" << std::endl;
  std::lock_guard<std::mutex> lock(mutexFile);
  errorState = true;
}

I'd like to make it easier to read by factoring all that common code. I could use a function for that. But that moves the code away from where it is used. I'd rather have it there in the catch for easier reading. Best I've come up with is this:

try {
  // code
}
#define CATCH(reason) {\
  file.close(); \
  std::cerr << "Read failure " << reason << std::endl; \
  std::lock_guard<std::mutex> lock(mutexFile); \
  errorState = true; \
}
catch( FailedReadException const & ex)
  CATCH(ex.readFailure())
catch( std::runtime_error const & ex)
  CATCH(ex.what())
catch(...) 
  CATCH("unexpected error")
#undef CATCH

Is there a better way to do this?

Why does this post require moderator attention?
You might want to add some details to your flag.
Why should this post be closed?

0 comments

2 answers

+5
−0

One solution would be to catch a more generic base class, like std::exception. If all your exceptions derive from that, you should be safe.

Another solution I like better is using destructors to do clean-up instead of lots of try-catches. The more clean up is done automatically by destructors, the better. I recommend you watch this CppCon talk titled Declarative Control Flow. The video shows a very convenient way of using destructors with macros like SCOPE_EXIT, SCOPE_FAIL and SCOPE_SUCCESS from folly/ScopeGuard.h. You can basically write declarative code like:

file.open();
SCOPE_EXIT{ file.close(); };
SCOPE_FAIL{ errorState = true; };

The macros create an anonymous object, which execute the block in its destructor either always or conditionally depending on the macro. This is the most natural way I tend to do error-handling. I usually don't care about the particular exception being thrown. All I want to do is to make sure code either succeeds or fails gracefully and cleans up automatically.

I'd argue that the destructor of file should handle closing, but in case you're working with some legacy API, this could be useful.

Why does this post require moderator attention?
You might want to add some details to your flag.

0 comments

+4
−0

There are many things we label as bad when programming, for various reasons. Repeating code is slightly bad. But writing function like-macros is extremely bad.

You should not try to replace something slightly bad with something extremely bad. As it turns out, the programmer putting out fire with gasoline in their attempts to avoid code repetition is a very common pitfall when we look at broken C++ program designs. Don't go out of your ways to avoid code repetition! Less "DRY" and more "KISS", for those who like design buzzwords.

The reasons why we should avoid function-like macros are many: they are type unsafe, it is easy to write subtle precedence bugs when writing them, they are hard to read, they are very hard to debug and maintain.

But mostly we should avoid them because individuals inventing their own personal, secret macro language makes the code impossible to read. Other C++ programmers know C++. They don't know your secret macro language. Therefore you should strive to write the code in C++ instead.

In your specific case, you could simply have written a function instead. Perhaps something along the lines of:

bool exception_failed (const std::string& str);

This could likely be a private member function too. If the reason why you can't do this is because you've got lots of variables scattered outside a class, then probably you should have created a class.

Cleaning up file handles and mutexes is ideal for a little local class that you can create an object of at local scope, to get all the clean-up handled by RAII. This might reduce the need for exceptions too.


Regarding exceptions, my experience is that it is always best to stick to the standard ones unless you have very specialized needs, which is rarely the case. Most often you just wish to send a string around. The best option is to rewrite the API can't be rewritten to use std::runtime_error. If that's not possible, then it might be justified to use a wrapper to standardize the exception types.

If you have some function foo throwing rotten_tomatoes, then you could create a wrapper just to get rid of the custom, cumbersome type:

void foo_wrapper (int x)
{
  try
  {
    foo();
  }
  catch(const rotten_tomatoes& ex)
  {
    throw( std::runtime_error(ex.msg()) );
  }
}

Or better yet, don't throw an exception when there's no need for it and return an error class or enum. I started this post speaking of bad things we do when programming, and exception handling is definitely one of them. They should be used with caution and only for the most severe forms of errors, if at all.

Why does this post require moderator attention?
You might want to add some details to your flag.

5 comments

rotten_tomatoes could be a subclass of std::runtime_error (or some other standard exception) so there is no need to catch and throw another exception. It's good to have std::exception as the base class for all exceptions imho. Ayxan Haqverdili‭ about 1 month ago

@Ayxan Haqverdili‭ If it is a subclass, then the original code would have handled it differently than std::runtime_error yeah? Or otherwise there's no need for a dedicated catch block. Lundin‭ about 1 month ago

@Lundin‭ that's exactly what I am saying. OP names every single exception in a specific catch block and handles them all identically. A simpler way is to catch the base class std::exception. Ayxan Haqverdili‭ about 1 month ago

Yes, the question is about exceptions which are not related by inheritance and which require almost identical code. @Lundin I agree with you regarding global macros. Do you have the same stance regarding local macros (a macro which is defined, used and immediately undefined)? Estela‭ about 1 month ago

@Estela There's a place for macros and even function-like macros, but not in this specific case. Avoid them when you can. Lundin‭ about 1 month ago

Sign up to answer this question »