Optimization of Hibernate DAO in desktop application

Question

I'm working on medium sized desktop application (with around 100 tables in database). For persistence layer I decided to use Hibernate for the first time to avoid massive redundancy of code for persistence layer.

This is final implementation of DAO interfaces:

public class BaseDAOImpl<E extends Serializable> implements BaseDAO<E> {

    @Override
    public void insert(E t) {
        Session session = HibernateUtil.openSession();
        try {
            session.beginTransaction();
            session.save(t);
            session.getTransaction().commit();
        } catch (Exception e) {
            e.printStackTrace();

            session.getTransaction().rollback();
        } finally {

            session.close();
        }
    }

    @Override
    public void delete(Class<E> e, int id) {
        Session session = HibernateUtil.openSession();
        try {
            session.beginTransaction();
            Object object = get(id, e);
            if (object != null) {
                session.delete(object);
            }
            session.getTransaction().commit();
        } catch (Exception ex) {
            ex.printStackTrace();

            session.getTransaction().rollback();
        } finally {
            session.close();
        }

    }

    @Override
    @SuppressWarnings("unchecked")
    public List<E> getAll(Class<E> c) {
        Session session = HibernateUtil.openSession();
        List<E> t = null;
        try {
            session.beginTransaction();
            t = session.createCriteria(c).list();
            session.getTransaction().commit();
        } catch (Exception e) {
            e.printStackTrace();
            session.getTransaction().rollback();
        } finally {
            session.close();
        }
        return t;
    }

    @Override
    public void update(E t) {
        Session session = HibernateUtil.openSession();
        try {
            session.beginTransaction();
            session.merge(t);
            session.getTransaction().commit();
        } catch (Exception e) {
            e.printStackTrace();
            session.getTransaction().rollback();
        } finally {

            session.close();
        }

    }

    @SuppressWarnings("unchecked")
    @Override
    public E get(int id, Class<E> c) {

        Session session = HibernateUtil.openSession();
        E t = null;
        try {
            session.beginTransaction();
            t = (E) session.get(c, id);
            session.getTransaction().commit();
        } catch (Exception e) {
            e.printStackTrace();
            session.getTransaction().rollback();
        } finally {
            session.close();
        }
        return t;
    }

}

Now, I'm a bit suspicious about session management. Is this a proper way to manage them (open session, commit changes, close session) or should I use some other approach, and what are the other suggestions to improve my code?

Answer 1

Wrong Transaction Management

Transactions are not a Data Access Layer concern, they are Business Logic Layer concern.

Consider how you would delete an entity, (any operation updating the database is similar) :

public class WidgetManager {
    public void deleteWidget(int widgetId) throws BusinessException{
        Widget widget = dao.get(Widget.class, widgetId);

        if (widget == null) throw new BusinessException("WidgetNotFound");

        if (!widget.isDeletable()) throw new BusinessException("WidgetCannotBeDeleted");

        dao.delete(widget);
    }
}

this method should be run in a transaction.

I would suggest you use Spring for transaction management as @ohiocowboy. (But using Hibernate API or Hibernate through JPA API (for example EntityManager) is not relevant to your current question.)

Repetitive Code

The following snippet is repeated in each method:

    Session session = HibernateUtil.openSession();
    try {
        session.beginTransaction();

        // do something with `session`

        session.getTransaction().commit();
    } catch (Exception e) {
        e.printStackTrace();
        session.getTransaction().rollback();
    } finally {
        session.close();
    }

This situation arises quite frequently, mostly when you deal with resources such as Hibernate sessions, DB transactions, connections, resultsets, files, sockets, ... You can use Execute Around Idiom to alleviate this problem.

Common code can be factored thus:

interface Function<T, R> {
    R apply(T t);
}

<R> R execute(final Function<Session, R> func) {
    R result = null;

    Session session = HibernateUtil.openSession();
    try {
        session.beginTransaction();

        result = func.apply(session);

        session.getTransaction().commit();
    } catch (Exception e) {
        e.printStackTrace();
        session.getTransaction().rollback();
    } finally {
        session.close();
    }

    return result;
}

Now get reads like this [Annotations omitted for clarity]:

public E get(final Class<E> c, final int id) {
    return execute(new Function<Session, E>() {
        public E apply(Session session) {
            return (E) session.get(c, id);
        }
    });
}

If you have Java 8, you already have Function interface. And above can be written even more concisely as follows:

public E get(final Class<E> c, final int id) {
    return execute(session -> (E) session.get(c, id));
}

For methods that return void, you can avoid unnecessary return null; statements with:

interface Consumer<T> {
    void accept(T t);
}

void execute(final Consumer<Session> cons) {
    execute(new Function<Session, Object>() {
        @Override
        public Object apply(Session t) {
            cons.accept(t);
            return null;
        }
    });
}

Now insert reads like this:

public void insert(final E t) {
    execute(new Consumer<Session>() {
        public void accept(Session session) {
            session.save(t);
        }
    });
}

If you have Java 8, you already have Consumer interface. And above can be written even more concisely as follows:

public void insert(final E t) {
    execute(session -> {session.save(t);});
}

Evil Singleton

HibernateUtil is either a singleton or, less likely and much worse, a real utility class that creates a SessionFactory that loads Hibernate mappings on each call to openSession. A utility class should not have any state, such as a SessionFactory.

EDIT: What to do about HibernateUtil singleton?

Let me tell you first how I would fix it: I would delete the static modifier from the openSession method and fix all the compile errors. But this style depends on how familiar you are with the compiler, your IDE and the code base you are currently working on, not to go out of hand.

However you do it in the end HibernateUtil and BaseDAOImpl should look similar to these:

public class HibernateUtil {
    private SessionFactory sessionFactory;

    public HibernateUtil (SessionFactory sessionFactory, ...) {
         this.sessionFactory = sessionFactory;
    }

    public openSession() { // <----- not static
        // The contents should not change much, if at all
    }
}

public class BaseDAOImpl .... {
    private HibernateUtil hibernateUtil;

    public BaseDAOImpl(HibernateUtil hibernateUtil) {
        this.hibernateUtil = hibernateUtil;
    }
// Later use `hibernateUtil` instance to call `openSession`
    Session session = hibernateUtil.openSession();

If you are using a DI framework, you make HibernateUtil another component and inject it to DAO instances.

If, instead, you are bootstrapping your application from main or some init method:

void initMyApp(.....) {

     // create SessionFactory once
     SessionFactory sessionFactory = .......

     // create HibernateUtil once
     HibernateUtil hibernateUtil = new HibernateUtil(sessionFactory) .......

     // Use the same `hibernateUtil` many times

     FooDAO fooDAO = new FooDAOImpl(hibernateUtil);
     BarDAO barDAO = new BarDAOImpl(hibernateUtil);
     BazDAO bazDAO = new BazDAOImpl(hibernateUtil);
}