patternjavaMinor
JPA connection : is this code is efficient enough
Viewed 0 times
thisefficientcodeenoughconnectionjpa
Problem
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 localEm = new ThreadLocal() {
@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() {
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 localEm = new ThreadLocal() {
@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() {
Solution
1.This sample is really ugly :) You can extract method
2.Cycle over keyset is a bad practice.
You should use
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 entry : var.entrySet()) {Code Snippets
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());for (String str : var.keySet()) {for (Entry<String, Object> entry : var.entrySet()) {Context
StackExchange Code Review Q#19177, answer score: 3
Revisions (0)
No revisions yet.