Method for executing any type of HQL

Question

I have written two methods that executes any type of HQL and returns data. Methods are like

public static Query getHqlQueryObject(String hql) throws Exception{

        Query query = null;
        Session session = null;
        try {
            session = HibernateUtil.getSessionFactory().getCurrentSession();
            Transaction transaction = session.beginTransaction();
            query = session.createQuery(hql);
        } catch (Exception e) {
            System.out.println("Exception in Helper::getHqlQueryObject");
            e.printStackTrace();
        }
        return query;
    }

    public static List<Object> getHqlQueryData(Query query) throws Exception{
        List dataList = new ArrayList();
        Session session = null;
        try {
            session = HibernateUtil.getSessionFactory().getCurrentSession();
            Transaction transaction = session.getTransaction();
            dataList = query.list();
            transaction.commit();
        } catch (Exception e) {
            System.out.println("Exception in Helper::getHqlQueryData");
            e.printStackTrace();
        }
        return dataList;
    }

I am calling this methods like this

Query query = Helper.getHqlQueryObject("from BookDetails where pkBookId = :pkBookId");
query.setParameter("pkBookId", new Long(1));
List dataList = Helper.getHqlQueryData(query);

My question is, is it right approach? or should I write session initialization code every time when I want to execute a query?

Answer 1

or should i write session initialization code every time when i want to execute query?

Sessions are meant to be re-used. You should not initialize a new method every time. I believe HibernateUtil.getSessionFactory().getCurrentSession(); is OK to use as that re-uses an existing session.

As for the rest of your code, I have a couple of things to say.

public static Query getHqlQueryObject(String hql) throws Exception
public static List<Object> getHqlQueryData(Query query) throws Exception

Neither of these methods actually throw an Exception. Because you have this code:

} catch (Exception e) {
    System.out.println("Exception in Helper::...");
    e.printStackTrace();
}

THIS IS BAD! A caller of your method has absolutely no guarantee that nothing went wrong. And is therefore unable to tell any potential user of your application that something did went wrong.

Simply printing the stacktrace accomplishes nothing for robustness!

Additionally, the only exception that should be able to occur is HibernateException, there is no need to catch all the other ones.

---

Additionally, there is no need for Transaction transaction = session.beginTransaction(); in your getHqlQueryObject. The transaction is only needed when the Query is performed, not when it is created.

Likewise, your getHqlQueryData are not guaranteed that a transaction is active.

---

I also have to question the usefulness of your code. Hibernate is already an abstraction and does a great job in simplifying things. What is it exactly you want to make easier with these methods?

Your current code:

Query query = Helper.getHqlQueryObject("from BookDetails where pkBookId = :pkBookId");
query.setParameter("pkBookId", new Long(1));
List dataList = Helper.getHqlQueryData(query);

How I would do it:

try {
    Query query = session.createQuery("from BookDetails where pkBookId = :pkBookId");
    query.setParameter("pkBookId", 1L);
    Transaction transaction = session.getTransaction();
    List<?> dataList = query.list();
    transaction.commit();
}
catch (HibernateException ex) {
    if (transaction.isActive()) {
         transaction.rollback();
    }
    // either re-throw the exception, wrap in another exception, or return a value indicating error.
}

You do have a session variable available here already, I hope? If not, you should remember the principle Tell, don't ask. That is, if your code needs some information, you should pass it there. I generally don't recommend calling a public static method or similar to get access to it.

By the way, I am not so sure that query.list(); needs an active transaction.

Also, when a transaction fails (an exception is thrown), you should call transaction.rollback();

And the last thing I'd like to say, with your HQL being "from BookDetails where pkBookId = :pkBookId" there is no need to return a list, you can instead call the method uniqueResult

Answer 2

Few points to mention here

1. You should not be catching exceptions unless you are doing some business logic in catch block.

2. Getting session instance in getHqlQueryObject() and getHqlQueryData() is redundant, should avoid this. With a connection pool, the overhead to open and close session isn't big. What takes time is to construct the session factory. Anyways, try to merge the above two methods into single.

3. Anyways you need to open a session, while setting query parameters, as some might be lazy and need to initialize it. This will be at your implementation level, whoever is calling your service.