Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Try with empty Catches [duplicate]

Say I have a try statement with and empty catch is that bad practice? For example say I have 2 try separate trys where it is possible for one to fail but the other to succeed or both succeed or any possible combination of such. Is that bad practice to handle code like that?

Example

if( mode == Modes.EDIT ){
  try {user = userBo.findById(id).get(0); }
  catch(Exception e) { }
  try{
    result = this.initializeEntityById(accountDao, id);
    if( !result.equals(SUCCESS) ){
      return result;
    }   
  }
  catch(Exception e){ }
}

In this example the variable in concern is 'id' where I'm not sure if the value coming in is valid and on the front end it doesn't really matter because the code handles whatever comes in and provides correct display.

So the question really is:

  1. Is this a bad practice with the empty catch's?
  2. Is there any potential instability that could occur that I'm not realizing?
  3. Is there a better way to achieve what I'm looking to get at?
like image 980
gcalex5 Avatar asked Jul 24 '13 15:07

gcalex5


2 Answers

  1. Yes it's always bad practice since you have no idea what went wrong where. You should at least log the exception.
  2. The instability is that when something goes wrong, you have no idea what is wrong with your code.
  3. It's never desirable to use exceptions for control-flow. The performance is atrocious, and exceptions should only be used for exceptional circumstances. Here's what you can do:
    1. Make the exception part of the method signature and let a higher level handle it.
    2. Catch the exception, but wrap it in a semantically-appropriate exception and rethrow it so a higher level can handle it.
    3. Handle it. This can also be done in multiple ways:
      1. Don't let the exception happen: instead of catching the exception, perform an inspection on the data to see if it can cause an exception. For example, in your case I think you should have a method like userBo.existsWithId(id) which will return a boolean that says whether the user exists. Or have the findById return null if the user cannot be found, and check to see if user == null. I think this is your best bet.
      2. Recover from the exception in some sane way (depends on your business logic).
like image 139
Vivin Paliath Avatar answered Sep 22 '22 13:09

Vivin Paliath


This is bad practice.

There's no instability, per se, but with empty catches, nothing is being done about the exception, which could leave an object in an inconsistent state if some aspects didn't get processed due to the exception.

With that said, empty catch blocks make for very difficult debugging. Even in production code, you should include a basic "notification" for most exceptions.

In testing code, use e.printStackTrace( in every catch block that does nothing otherwise.

like image 30
nanofarad Avatar answered Sep 23 '22 13:09

nanofarad