Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

By reading some tutorial i have written some peace of code to do crud operation . I just want to know how is this code is efficient or how can i make better ?

Here i am giving code of 3 class 1. EntityManagerProvider : get connection from DB and do crud operation 2. BaseDao : A generalised Dao which uses EntityManagerProvider
3. TargetGroupsDao : A specific Dao

import java.util.Collection;

import javax.persistence.EntityExistsException;
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.EntityTransaction;
import javax.persistence.Persistence;
import javax.persistence.PersistenceException;
import javax.persistence.Query;
import javax.persistence.RollbackException;
import javax.persistence.TransactionRequiredException;

public class EntityManagerProvider {

    private static EntityManagerFactory emf;
    private static boolean lock;
    private static String persistentUnitName = "TMS_PU";
    static {
        EntityManagerProvider.emf = Persistence.createEntityManagerFactory(persistentUnitName);
    }
    private static final ThreadLocal<EntityManager> localEm = new ThreadLocal<EntityManager>() {
        @Override
        protected EntityManager initialValue() {
            if (emf == null || !emf.isOpen()) {
                emf = Persistence
                        .createEntityManagerFactory(persistentUnitName);
            }
            return emf.createEntityManager();
        }
    };

    @Override
    protected void finalize() throws Throwable {
        EntityManager em = getEntityManager();
        if (em != null && em.isOpen())
            em.close();
        super.finalize();
    }

    public EntityManagerProvider() {
    }

    public static EntityManager getEntityManager() {
        EntityManager em = localEm.get();
        if (em == null || !em.isOpen()) {
            em = emf.createEntityManager();
            localEm.set(em);
        }
        return em;
    }

    public static void beginTransaction() {
        if (lock)
            return;
        EntityTransaction tx = getEntityManager().getTransaction();
        if (!tx.isActive()) {
            tx.begin();
        }
    }

    public static void rollbackTransaction() {
        EntityTransaction tx = getEntityManager().getTransaction();
        if (tx != null && tx.isActive()) {
            tx.setRollbackOnly();
        }
    }

    public static void commitTransaction() {
        if (lock)
            return;
        EntityTransaction tx = getEntityManager().getTransaction();
        if (tx != null && tx.isActive()) {
            try {
                if (tx.getRollbackOnly())
                    tx.rollback();
                else
                    tx.commit();
            } catch (RollbackException e) {
            } catch (Exception e) {
            }
        }
    }

    public void persist(Object object) throws Exception {
        try {
            if (object instanceof Collection) {
                Collection<?> objects = (Collection<?>) object;
                for (Object o : objects) {
                    getEntityManager().persist(o);
                }
            } else {
                getEntityManager().persist(object);
            }
        } catch (EntityExistsException e) {
            throw e;
        } catch (IllegalStateException e) {
            throw e;
        } catch (IllegalArgumentException e) {
            throw e;
        } catch (TransactionRequiredException e) {
            throw e;
        } catch (Exception e) {
            rollbackTransaction();
            throw e;
        } finally {
        }
    }

    public Object merge(Object object) throws Exception {
        Object result = null;
        try {
            if (object instanceof Collection) {
                Collection<?> objects = (Collection<?>) object;
                for (Object o : objects) {
                    getEntityManager().merge(o);
                }
            } else {
                result = getEntityManager().merge(object);
            }
        } catch (IllegalStateException e) {
            throw e;
        } catch (IllegalArgumentException e) {
            throw e;
        } catch (TransactionRequiredException e) {
            throw e;
        } catch (Exception e) {
            rollbackTransaction();
            throw e;
        } finally {
        }
        return result;
    }

    public void clear() throws Exception {
        try {
            getEntityManager().clear();
        } catch (IllegalStateException e) {
            throw e;
        }
    }

    public void flush() throws Exception {
        try {
            getEntityManager().flush();
        } catch (IllegalStateException e) {
            throw e;
        } catch (TransactionRequiredException e) {
            throw e;
        } catch (PersistenceException e) {
            throw e;
        } catch (Exception e) {
            throw e;
        }

    }

    public Object getById(final Class<?> persistentClass, Object id) {
        return getEntityManager().find(persistentClass, id);
    }

    public Object getReferenceById(final Class<?> persistentClass, Object id) {
        return getEntityManager().getReference(persistentClass, id);
    }

    public void remove(Object valueObject) throws Exception {
        try {
            EntityManager em = getEntityManager();
            em.remove(valueObject);
            em.flush();
        } catch (IllegalStateException e) {
            throw e;
        } catch (IllegalArgumentException e) {
            throw e;
        } catch (TransactionRequiredException e) {
            throw e;
        } catch (Exception e) {
            rollbackTransaction();
            throw e;
        } finally {
        }
    }

    public Query createQuery(String jpql) {
        return getEntityManager().createQuery(jpql);
    }

    public static void lock() {
        lock = true;
    }

    public static void unlock() {
        lock = false;
    }

    public static boolean getlock() {
        return lock;
    }
}

import java.util.List;
import java.util.Map;
import javax.persistence.Query;
public class BaseDao {
    private EntityManagerProvider emp;
    private Class<?> className;

    public BaseDao(final Class<?> className) {
        this.className = className;
        emp = new EntityManagerProvider();

    }

    public BaseDao(final Class<?> className, String localMsg) {
        this.className = className;
        emp = new EntityManagerProvider();
    }

    public void create(Object object, EntityManagerProvider emp)
            throws Exception {

        try {
            if (emp == null) {
                EntityManagerProvider.beginTransaction();
                this.emp.persist(object);
                EntityManagerProvider.commitTransaction();
            } else {
                emp.persist(object);
            }
        } catch (Exception e) {
            throw e;
        }

    }

    public void update(Object object, EntityManagerProvider emp)
            throws Exception {

        try {
            if (emp == null) {
                EntityManagerProvider.beginTransaction();
                this.emp.merge(object);
                EntityManagerProvider.commitTransaction();
            } else {
                object = emp.merge(object);
            }
        } catch (Exception e) {
            throw e;
        }

    }

    public Object getById(Object primaryKey) throws Exception {

        Object object = null;
        try {
            emp = new EntityManagerProvider();

            // FLUSH
            EntityManagerProvider.beginTransaction();
            // emp.flush();
            // emp.clear();

            object = emp.getById(className, primaryKey);
            EntityManagerProvider.commitTransaction();
        } catch (Exception e) {
            throw e;
        }
        return object;
    }



    public Object getReferenceById(Object primaryKey) throws Exception {
        Object object = null;
        try {
            emp = new EntityManagerProvider();
            object = emp.getReferenceById(className, primaryKey);
        } catch (Exception e) {
            throw e;
        }
        return object;
    }

    public List<?> getAll() throws Exception {
        return getAll(null, null);
    }

    public List<?> getAll(Integer fromRowNo, Integer noOfRows) throws Exception {

        List<?> list = null;
        String sql = null;
        Query query = null;
        try {
            emp = new EntityManagerProvider();
            // CLEAR & FLUSH
            EntityManagerProvider.beginTransaction();
            emp.flush();
            emp.clear();
            EntityManagerProvider.commitTransaction();

            sql = "SELECT p " + "FROM " + className.getCanonicalName() + " p";
            query = emp.createQuery(sql);

            if (fromRowNo != null)
                query.setFirstResult(fromRowNo);
            if (noOfRows != null)
                query.setMaxResults(noOfRows);

            list = query.getResultList();
        } catch (Exception e) {
            throw e;
        }
        return list;
    }

    public void delete(Object primaryKey, EntityManagerProvider emp)
            throws Exception {
        Object object = getById(primaryKey);
        if (object == null) {
            return;
        }

        try {
            if (emp == null) {
                EntityManagerProvider.beginTransaction();
                this.emp.remove(object);
                EntityManagerProvider.commitTransaction();
            } else {
                emp.remove(object);
            }
        } catch (Exception e) {
            throw e;
        }
    }

    public List<?> search(Map<String, Object> var, String opr) throws Exception {
        return search(var, null, null, true, null, null, opr);
    }

    public List<?> search(Map<String, Object> var, Integer fromRowNo,
            Integer noOfRows, String opr) throws Exception {
        return search(var, null, null, true, fromRowNo, noOfRows, opr);
    }

    public List<?> search(Map<String, Object> var, Map<String, Boolean> flags,
            String opr) throws Exception {
        return search(var, flags, null, true, null, null, opr);
    }

    public List<?> search(Map<String, Object> var, Map<String, Boolean> flags,
            Integer fromRowNo, Integer noOfRows, String opr) throws Exception {
        return search(var, flags, null, true, fromRowNo, noOfRows, opr);
    }

    public List<?> search(Map<String, Object> var, Map<String, Boolean> flags,
            Map<String, Boolean> orderBy, Boolean orderFlag, Integer fromRowNo,
            Integer noOfRows, String opr) throws Exception {
        List<?> list = null;

        if (opr == null)
            opr = "AND ";

        try {
            emp = new EntityManagerProvider();
            String sql = "SELECT p FROM " + className.getCanonicalName()
                    + " p ";
            String sql2 = null;
            String sql3 = null;
            String str2 = null;

            if (var != null) {
                for (String str : var.keySet()) {
                    if (str.indexOf(".") > 0) // This solves only for one
                        // level
                        str2 = str
                                .substring(str.indexOf(".") + 1, str.length());
                    else
                        str2 = str;
                    if (sql2 != null) {
                        if (flags != null) {
                            if (!flags.get(str))
                                sql2 += opr + "p." + str + " != :" + str2 + " ";
                            else
                                sql2 += opr + "p." + str + " = :" + str2 + " ";
                        } else
                            sql2 += opr + "p." + str + " = :" + str2 + " ";
                    } else {
                        if (flags != null) {
                            if (!flags.get(str))
                                sql2 = "WHERE p." + str + " != :" + str2 + " ";
                            else
                                sql2 = "WHERE p." + str + " = :" + str2 + " ";
                        } else
                            sql2 = "WHERE p." + str + " = :" + str2 + " ";
                    }
                }
            }

            if (orderBy != null) {
                for (String str : orderBy.keySet()) {
                    if (sql3 != null) {
                        if (orderBy.get(str) != null && orderBy.get(str))
                            sql3 += ", p." + str + " ";
                    } else {
                        if (orderBy.get(str) != null && orderBy.get(str))
                            sql3 = " ORDER BY p." + str;
                    }
                }
            }

            if (sql2 != null)
                sql += sql2;

            if (sql3 != null) {
                if (orderFlag)
                    sql += sql3;
                else
                    sql += sql3 + " DESC ";
            }

            Query query = emp.createQuery(sql);

            for (String str : var.keySet()) {
                if (str.indexOf(".") > 0) // This solves only for one level
                    str2 = str.substring(str.indexOf(".") + 1, str.length());
                else
                    str2 = str;
                query.setParameter(str2, var.get(str));
            }

            if (fromRowNo != null) {
                query.setFirstResult(fromRowNo);
            }
            if (noOfRows != null) {
                query.setMaxResults(noOfRows);
            }

            EntityManagerProvider.beginTransaction();
            // emp.flush();
            // emp.clear();
            list = query.getResultList();
            EntityManagerProvider.commitTransaction();
        } catch (Exception e) {
            throw e;
        }

        return list;
    }

    public Integer countRecords(Map<String, Object> var,
            Map<String, Boolean> flags, String opr) throws Exception {
        Long count = null;

        if (opr == null)
            opr = "AND ";

        try {
            emp = new EntityManagerProvider();
            String sql = "SELECT COUNT(p) FROM " + className.getCanonicalName()
                    + " p ";
            String sql2 = null;
            String str2 = null;

            if (var != null) {
                for (String str : var.keySet()) {
                    if (str.indexOf(".") > 0) // This solves only for one
                        // level
                        str2 = str
                                .substring(str.indexOf(".") + 1, str.length());
                    else
                        str2 = str;
                    if (sql2 != null) {
                        if (flags != null) {
                            if (!flags.get(str))
                                sql2 += opr + "p." + str + " != :" + str2 + " ";
                            else
                                sql2 += opr + "p." + str + " = :" + str2 + " ";
                        } else
                            sql2 += opr + "p." + str + " = :" + str2 + " ";
                    } else {
                        if (flags != null) {
                            if (!flags.get(str))
                                sql2 = "WHERE p." + str + " != :" + str2 + " ";
                            else
                                sql2 = "WHERE p." + str + " = :" + str2 + " ";
                        } else
                            sql2 = "WHERE p." + str + " = :" + str2 + " ";
                    }
                }
            }

            if (sql2 != null)
                sql += sql2;

            Query query = emp.createQuery(sql);

            for (String str : var.keySet()) {
                if (str.indexOf(".") > 0) // This solves only for one level
                    str2 = str.substring(str.indexOf(".") + 1, str.length());
                else
                    str2 = str;
                query.setParameter(str2, var.get(str));
            }

            EntityManagerProvider.beginTransaction();
            // emp.flush();
            // emp.clear();
            count = (Long) query.getSingleResult();
//          System.out.println(count); //TODO: REMOVE ME
            EntityManagerProvider.commitTransaction();
        } catch (Exception e) {
            throw e;
        }

        return count.intValue();
    }
}  

public class TargetGroupsDao {

    private BaseDao baseDao;

    public TargetGroupsDao() {
        baseDao = new BaseDao(TargetGroupsDo.class, TargetGroupsDao.class.getName());
    }

    public void create(TargetGroupsDo targetGroupsDo, EntityManagerProvider emp) throws Exception {
        baseDao.create(targetGroupsDo, emp);
    }

    public void update(TargetGroupsDo targetGroupsDo, EntityManagerProvider emp) throws Exception { 
        baseDao.update(targetGroupsDo, emp);
    }

    public TargetGroupsDo getById(String serialId) throws Exception {
        return (TargetGroupsDo)baseDao.getById(serialId);
    }

    @SuppressWarnings("unchecked")
    public List<TargetGroupsDo> getAll(Integer fromRowNo, Integer noOfRows) throws Exception {
        return  (List<TargetGroupsDo>)baseDao.getAll(fromRowNo, noOfRows);
    }

    public void delete(String serialId, EntityManagerProvider emp) throws Exception {
        baseDao.delete(serialId, emp);
     }

    @SuppressWarnings("unchecked")
    public List<TargetGroupsDo> search(TargetGroupsDto targetGroupsDto, Integer fromRowNo, Integer noOfRows, String opr) throws Exception {
        Map<String, Object> map = new HashMap<String, Object>();

        if( targetGroupsDto.getTgDesc()!= null && !targetGroupsDto.getTgDesc().trim().equals(""))
            map.put("tgDesc", targetGroupsDto.getTgDesc());
        if( targetGroupsDto.getTgId()!= null && !targetGroupsDto.getTgId().trim().equals(""))
            map.put("tgId", targetGroupsDto.getTgId());
        if( targetGroupsDto.getTgName()!= null && !targetGroupsDto.getTgName().trim().equals(""))
            map.put("tgName", targetGroupsDto.getTgName());
        if( targetGroupsDto.getOwnershipType()!= null && !targetGroupsDto.getOwnershipType().trim().equals(""))
            map.put("ownershipType", targetGroupsDto.getOwnershipType());
        if( targetGroupsDto.getNoOfMembers()!= null)
            map.put("noOfMembers", targetGroupsDto.getNoOfMembers());
        return (List<TargetGroupsDo>)baseDao.search(map, fromRowNo, noOfRows, opr);
    }

    public TargetGroupsDo getReferenceById(String tgId) throws Exception {
        return (TargetGroupsDo)baseDao.getReferenceById(tgId);
    }
}
share|improve this question
add comment

1 Answer

1.This sample is really ugly :) You can extract method isEmptyTrim() at least.

if( targetGroupsDto.getTgDesc()!= null && !targetGroupsDto.getTgDesc().trim().equals(""))
    map.put("tgDesc", targetGroupsDto.getTgDesc());
if( targetGroupsDto.getTgId()!= null && !targetGroupsDto.getTgId().trim().equals(""))
    map.put("tgId", targetGroupsDto.getTgId());
if( targetGroupsDto.getTgName()!= null && !targetGroupsDto.getTgName().trim().equals(""))
    map.put("tgName", targetGroupsDto.getTgName());
if( targetGroupsDto.getOwnershipType()!= null && !targetGroupsDto.getOwnershipType().trim().equals(""))
    map.put("ownershipType", targetGroupsDto.getOwnershipType());
if( targetGroupsDto.getNoOfMembers()!= null)
    map.put("noOfMembers", targetGroupsDto.getNoOfMembers());

2.Cycle over keyset is a bad practice.

 for (String str : var.keySet()) {

You should use

 for (Entry<String, Object> entry : var.entrySet()) {
share|improve this answer
    
Any Comment on Connection setting ? And The way BaseDao is used within TargetGroupsDao is correct? –  Arvind Dec 3 '12 at 6:08
    
@codesparkle Why? If you need both key and value, then entrySet() should be used. The link that you posted says exactly the same, too. –  Philipp Claßen Dec 31 '12 at 4:39
    
Sorry, I wasn't precise enough: in one case, only the key is used; there, keySet() is the right choice. All others should indeed be entrySet(). –  codesparkle Jan 1 '13 at 16:34
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.