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
Linux Systems
Linux Systems
Power Users
Power Users
Tabletop RPGs
Tabletop RPGs
Community Proposals
Community Proposals
tag:snake search within a tag
answers:0 unanswered questions
user:xxxx search by author id
score:0.5 posts with 0.5+ score
"snake oil" exact phrase
votes:4 posts with 4+ votes
created:<1w created < 1 week ago
post_type:xxxx type of post
Search help
Notifications
Mark all as read See all your notifications »
Q&A

Welcome to Software Development on Codidact!

Will you help us build our independent community of developers helping developers? We're small and trying to grow. We welcome questions about all aspects of software development, from design to code to QA and more. Got questions? Got answers? Got code you'd like someone to review? Please join us.

Comments on Multiple catches with almost the same code.

Parent

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?

History
Why does this post require attention from curators or moderators?
You might want to add some details to your flag.
Why should this post be closed?

1 comment thread

General comments (1 comment)
Post
+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.

History
Why does this post require attention from curators or moderators?
You might want to add some details to your flag.

1 comment thread

General comments (5 comments)
General comments
Ayxan Haqverdili‭ wrote over 3 years ago · edited over 3 years ago

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.

Lundin‭ wrote over 3 years 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.

Ayxan Haqverdili‭ wrote over 3 years ago · edited over 3 years 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.

Estela‭ wrote over 3 years ago · edited over 3 years 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)?

Lundin‭ wrote over 3 years ago

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