debugjavaMinor
Coding style and best practice regarding exception-handling
Viewed 0 times
handlingexceptionpracticeregardingstyleandcodingbest
Problem
I need comments on my JPA code. Am I doing it correctly? I need some advice on best practices in Java.
I have this method below:
When I called the method, I just have:
I got no error on the above code, what I want are some suggestions on my coding style. Is the above good code if used in production?
I have this method below:
public Comics add(Comics comics) {
EntityManager em = EMF.get().createEntityManager();
EntityTransaction tx = null;
try{
tx = em.getTransaction();
tx.begin();
em.persist(comics);
tx.commit();
}catch(Exception ex){
ex.printStackTrace();
if(tx != null && tx.isActive())
tx.rollback();
} finally{
em.close();
}
return comics;
}When I called the method, I just have:
ComicsAccess access = new ComicsAccess();
access.add( comics );I got no error on the above code, what I want are some suggestions on my coding style. Is the above good code if used in production?
Solution
The way you are handling the exception will result in the method returning normally whether or not the method has succeeded. This is bad API design. Better design would be to either return some special value to indicate failure or (better) to allow the exception to propagate, or throw a different one.
Catching
Unconditionally writing stacktraces to standard output is bad practice. Use a logging system; e.g. java.util.logging, log4j, etcetera.
Here is an alternative version that avoids these problems:
Catching
Exception is usually a bad idea. The problem is that you may end up catching all sorts of unexpected exceptions.Unconditionally writing stacktraces to standard output is bad practice. Use a logging system; e.g. java.util.logging, log4j, etcetera.
Here is an alternative version that avoids these problems:
public Comics add(Comics comics) throws ... {
EntityManager em = EMF.get().createEntityManager();
EntityTransaction tx = null;
try {
tx = em.getTransaction();
tx.begin();
em.persist(comics);
tx.commit();
} finally {
try {
if (tx != null && tx.isActive()) {
tx.rollback();
}
} finally {
em.close();
}
}
return comics;
}Code Snippets
public Comics add(Comics comics) throws ... {
EntityManager em = EMF.get().createEntityManager();
EntityTransaction tx = null;
try {
tx = em.getTransaction();
tx.begin();
em.persist(comics);
tx.commit();
} finally {
try {
if (tx != null && tx.isActive()) {
tx.rollback();
}
} finally {
em.close();
}
}
return comics;
}Context
StackExchange Code Review Q#15467, answer score: 5
Revisions (0)
No revisions yet.