CODEWITHSUNDEEP
java
Phate
Phate

Reputation: 6612

Is executing code inside of an exception bad in this case?

I have this specific scenario:

  1. my exceptions have a code and a localized message
  2. The thrower just knows the code
  3. The catcher expects a localized message
  4. The localization is inside of a DB table

Would it be wrong something like this:

 public class MyException{
    public MyException(int code){
       try{
          this.message = db.selectMessage(code);
       }catch(Exception ex){
          this.message = "";
       }
    }
 }

This would completely hide from the thrower the fact that the message is localized.

Alternatively I should go with something like this:

   public class ExceptionUtils{
       public static throwMyException(int code) throws MyException{
           String message = db.selectMessage(code);
           throw new MyException(code, message);
       }

}

But this requires the thrower to know about this util.

Upvotes: 0

Views: 81

Answers (5)

Omid
Omid

Reputation: 6103

This is a better approach:

public class MyException extends Exception {
    private int code;

    public MyException(String message, int code) {
        super(message);
        this.code = code;
    }

    public int getCode() {
        return code;
    }
}

Usage:

Integer messageCode = null;
try {
    // do stuff
} catch (MyException e) {
    logger.log(e.getMessage(), e); // log actual message
    messageCode = e.getCode();
}
if(messageCode != null /* && you really want to show it to end user */) {
    String localizedMessage = db.selectMessage(code);
    // show localized message to end user
}

Adavantages:

  • You don't need a Util class to throw exceptions.
  • You don't need to access db every time you throw an exception but only when you catch it and "decide" to fetch the message if you want to show it to user.
  • You don't need to catch an exception inside another exception.
  • You don't lose the actual non-localized message.
  • You don't lose actual stack trace in case db.getMessage() fails and throws exception.

Edit:

There is a dispute about whether the if part is a good idea or not so I have to elaborate.

message and localized version of message, these are very different.

Message:

  • is a description of what has gone wrong.
  • is what you see in console and in log records when exception occurs.
  • is in English.
  • must be shown regardless of any conditions and if not shown it's considered a very bad practice.

Localized Message:

  • is a translation of Message for the benefit of End User and not programmer.
  • is not in English.
  • is not expected to be shown either in console or log records.
  • is only needed when interacting with End User in UI.
  • is not expected to be used in non-UI related parts of code.

In the actual code provided by asker, message is replaced by localized message which violates expected behavior of a well-designed exception so I separated these two. In my code the message is Always shown and can be logged whenever exception occurs regardless of any conditions; Fetching localized message occurs only IF you actually need to interact with End Users. So access to DB can be skipped when you are not interacting with them.

Upvotes: 1

davidbak
davidbak

Reputation: 5999

The problem is one of too many moving parts. If your database access within the catch block fails by throwing an exception - and it can fail for any one of a number of reasons - then you won't ever see even your original exception. The same goes for anything you do in the catch block that could itself throw an exception.

This has recently happened to me, in fact, in legacy code (different language though same idea). Man is that a pain when you know your production system is failing for some specific reason but you have no idea what that specific reason is ...

It may be the case that your routine db.selectMessage() is itself protected against and won't throw an exception. Ok then. But it's going to be your responsibility to check that in every catch block you write. It's better to go with a design approach that doesn't involve catch blocks that do much of anything except things known to be 'safe'.

In some languages, by the way, catching all exceptions catches some really nasty ones (in Java, those aren't usually subclasses of java.lang.Exception though). And sometimes you really don't want to do anything unnecessary, 'cause you don't know what state you're in and what'll happen.

In this case, you're probably going to be logging that exception somewhere (or otherwise notifying the user). That logging code is (or should be) centralized in your application. That's the place to carefully translate/localize/interpret the exception code ... someplace where in one place you can make sure it works and is protected ... same as the logging code itself.

Upvotes: 0

Joseph
Joseph

Reputation: 837

It is not bad as long it is code that helps you best handle the exception

Upvotes: 0

Michael Gantman
Michael Gantman

Reputation: 7792

I would suggest using ResourceBundle class as it is widely accepted way for localization. This way you store your messages in files as oppose to DB. Also reading your messages from those files is handled by JVM and you don't have to write your own code for it. However, if you insist on using DB, I would sudgest to read all your messages from DB into file system or onto your memory during your app initialization and then you don't have to read from DB and risk DB connectivity failure for each exception.

Upvotes: 1

Manish Sakpal
Manish Sakpal

Reputation: 299

the catch block is designed to do some actions after an exception occurs in your program, hence i would recommend you to provide some of the exception handling code in the catch block as it will allow others to understand your program efficiently

Upvotes: 0

Related Questions