Try/Catch inside or outside functions

Question

I have a very basic question about best practice of using try/catch.

I have a simple function (DAO) like this:

public void addVehicle(Vehicle vehicle) {

    em.getTransaction().begin();
    em.persist(vehicle);
    em.getTransaction().commit();
}

and using DAO function inside web service:

@WebMethod(operationName = "addVehicle")
public void addVehicle(Vehicle vehicle) {

    try {
        vehicleDAO.addVehicle(vehicle);
        System.out.print("Vehicle added");
    } catch (Exception e) {
        e.printStackTrace();
    }
}

OR is better using try/catch inside DAO function like this:

public void addVehicle(Vehicle vehicle) {

    try {
        em.getTransaction().begin();
        em.persist(vehicle);
        em.getTransaction().commit();
    } catch (Exception e) {
        e.printStackTrace();
    }
}

Answer 1

There is no perfect rule for that.

Often code is clearer and less complex if exceptions are catched as early as needed, but as late as possible.
You should think who has to take an action when that Exception happens, this decides if you catch it inside the method (addVehicle) or if you throw it such that the caller has to catch it.

E.g:

 public void addVehicle(Vehicle vehicle) throws SQLException{
        em.getTransaction().begin();
        em.persist(vehicle);
        em.getTransaction().commit();
 }

In this example the caller has to catch.
Further only in few situations you should catch Exception or RunTimeException, better catch that specific Exception, like IOException instead of Exception.

Somewhere in your code you will need a "last line of defense" where it make sense to catch (Exception ex). This is needed to handle errors that should not happen.

Answer 2

When deciding where to handle a specific kind of exception, the best rule of thumb is to stop looking at the micro details of your code, take a step back to reason about your program's logic and consider these things:

• Is the exception something that your program's current operation cannot recover from? If yes, it only makes sense to put the exception at the topmost level of that operation, to ensure that it doesn't continue.
• If your program can work around that particular exception (perhaps by trying something else before giving up), take each layer of nested functions (starting from the highest) and each time ask yourself: If the exception occurs during the execution of some line of code in this function, would it make sense for this function to continue? As long as the answer is "yes", move to the deeper level. As soon the answer is "no", chances are this is the best place to put the handler for that exception.
• Alternatively to the previous one, you could decide what would your program's alternate "plan of attack" be in case the exception is raised. Then, go to the line of code that would raise that exception and ask yourself: Does this function have enough context information to perform the workaround I have in mind? As long as the answer is "no", move to the caller function. As soon as the answer becomes "yes", consider putting your exception handler there.

That being said, you should only catch reasonably specialized exceptions and keep the catch(Exception ex) construct only as a last resort only at the top level and only after all the other possible catch blocks, reserving it only for kinds of exceptions you really couldn't predict at the time of writing. (I know you said this is not the point of the example, but since we're at it, I thought it should be mentioned to make this answer more complete.)

Answer 3

Use both, the only reason is to use catch RuntimeException or even Throwable. Because this kind of exception is typically thrown by the underlying frameworks. An you should catch exactly this kind of exception if you want to make some actions, for example logging, print stack trace, etc., before you re-throw it again. If you don't do in such way you may loose the cause of exception.

@Transactional
public void addVehicle(Vehicle vehicle) {
  try {
    //do whatever with session
  } catch (RuntimeException e) {
    e.printStackTrace();
    throw new Exception(e);
  }
}

Answer 4

You should only catch those exceptions which you want to handle. You may include a topmost exception handler to turn any unhandled exceptions into something somewhat useful for the end user.

Instead of e.printStackTrace();, try return proper exception message.

find out more about exception handling here

Here is more discussion about exception handling.

Answer 5

AFAIK the best practice will be smth like that:

public void addVehicle(Vehicle vehicle) {
        em.getTransaction().begin();
        try {
            em.persist(vehicle);
            em.getTransaction().commit();
        } catch (Exception e) {
            if (em.getTransaction().isActive()) {
                try {
                   em.getTransaction().rollback();
                } catch (Exception e) {
                   // Log rollback failure or something
                }
            }
            throw new RuntimeException(e);
        } 
}