1
\$\begingroup\$

This class functions, in that the program can be left open for hours and it will still reconnect to the database when the user prompts for a CRUD operation. I've read about DAO, and some other approaches, and, at a minimum, recognize that reconnecting each transaction is overkill. Some sources say to use a DAO, others say that with JPA it's no longer necessary. Almost all documentation I've found doesn't apply either because it's dated or for JavaEE rather than JavaSE.

package legacy.database;

import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.logging.Logger;
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.Persistence;
import javax.persistence.Query;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.Predicate;
import javax.persistence.criteria.Root;

public enum Queries {

    INSTANCE;
    private final Logger log = Logger.getLogger(Queries.class.getName());
    EntityManagerFactory emf = Persistence.createEntityManagerFactory("LegacyDatabasePU");
    EntityManager em = emf.createEntityManager();

    private EntityManager reconnect() {
        emf = Persistence.createEntityManagerFactory("LegacyDatabasePU");
        em = emf.createEntityManager();
        log.info("em\t" + em.isOpen());
        return em;
    }

    private List<Clients> findAll() {
        reconnect();
        Query q = em.createQuery("select c from Clients c");
        List<Clients> clients = q.getResultList();
        em.close();
        emf.close();
        return clients;
    }

    public List<Clients> selectByCriteria(Criteria criteria) {
        log.info(criteria.toString());
        reconnect();
        CriteriaBuilder criteriaBuilder = em.getCriteriaBuilder();
        CriteriaQuery<Clients> clientCriteriaQuery = criteriaBuilder.createQuery(Clients.class);
        Root<Clients> clientRoot = clientCriteriaQuery.from(Clients.class);
        clientCriteriaQuery.select(clientRoot);
        List<Predicate> predicates = new ArrayList<>();
        predicates.add(criteriaBuilder.like(clientRoot.get(Clients_.name), "%" + criteria.getName() + "%"));
        predicates.add(criteriaBuilder.like(clientRoot.get(Clients_.phone1), "%" + criteria.getPhone1() + "%"));
        predicates.add(criteriaBuilder.like(clientRoot.get(Clients_.email), "%" + criteria.getEmail() + "%"));
        if (!criteria.getOpener().equalsIgnoreCase("all")) {
            predicates.add(criteriaBuilder.like(clientRoot.get(Clients_.opener), "%" + criteria.getOpener() + "%"));
        }
        if (!criteria.getCloser1().equalsIgnoreCase("all")) {
            predicates.add(criteriaBuilder.like(clientRoot.get(Clients_.closer1), "%" + criteria.getCloser1() + "%"));
        }
        if (!criteria.getStatus().equalsIgnoreCase("all")) {
            predicates.add(criteriaBuilder.like(clientRoot.get(Clients_.status), "%" + criteria.getStatus() + "%"));
        }
        clientCriteriaQuery.where(predicates.toArray(new Predicate[0]));
        List<Clients> clients = em.createQuery(clientCriteriaQuery).getResultList();
        em.close();
        emf.close();
        return clients;
    }

    public Clients findById(int id) {
        reconnect();
        Clients client = em.find(Clients.class, id);
        em.close();
        emf.close();
        return client;
    }

    public void send(int id) {
        reconnect();
        Clients c = em.find(Clients.class, id);
        java.util.Date date = new java.util.Date();
        Timestamp t = new Timestamp(date.getTime());
        em.getTransaction().begin();
        c.setDateUpdated(t.toString());
        em.getTransaction().commit();
        em.close();
        emf.close();
    }

    private Timestamp stamp() {
        Date d = new Date();
        return new Timestamp(d.getTime());
    }

    private int oldPersist(Clients client) {
        log.info(client.toString());
        reconnect();
        client.setDateCreated(stamp().toString());
        log.info("starting transaction...");
        em.getTransaction().begin();
        log.info("..transaction started..");
        em.persist(client);
        log.info("..client persisted..");
        em.getTransaction().commit();
        log.info("..comitted..");
        em.close();
        emf.close();
        log.info("transaction complete...");
        return client.getId();
    }

    public int persistClient(Clients client) {
        reconnect();
        em.getTransaction().begin();
        Clients c = new Clients();
        em.persist(c);
        em.flush();
        int id = c.getId();
        em.getTransaction().commit();
        em.close();
        emf.close();
        client.setId(id);
        update(client);
        return id;
    }

    public int update(Clients newClient) {
        reconnect();
        Clients oldClient = em.find(Clients.class, newClient.getId());
        em.getTransaction().begin();
        oldClient.setAddress(newClient.getAddress());
        oldClient.setCity(newClient.getCity());
        oldClient.setCloser1(newClient.getCloser1());
        oldClient.setCloser2(newClient.getCloser2());
        oldClient.setCountry(newClient.getCountry());
        oldClient.setCompany(newClient.getCompany());
        oldClient.setDateCancelled(newClient.getDateCancelled());
        oldClient.setDateSold(newClient.getDateSold());

        try {
            if ((oldClient.getDateCreated().length() < 3) || (oldClient.getDateCreated() == null)) {
                oldClient.setDateCreated(stamp().toString());
            }
        } catch (NullPointerException e) {
            log.info("no date");
        }
        oldClient.setDateUpdated(stamp().toString());
        oldClient.setDescription(newClient.getDescription());
        oldClient.setEmail(newClient.getEmail());
        oldClient.setExteriorColor(newClient.getExteriorColor());
        oldClient.setFeatures(newClient.getFeatures());
        //no id set
        oldClient.setInteriorColor(newClient.getInteriorColor());
        oldClient.setInteriorType(newClient.getInteriorType());
        oldClient.setKilometres(newClient.getKilometres());
        oldClient.setMake(newClient.getMake());
        oldClient.setMileageType(newClient.getMileageType());
        oldClient.setModel(newClient.getModel());
        oldClient.setNotes(newClient.getNotes());
        oldClient.setOpener(newClient.getOpener());
        oldClient.setPaymentMethod(newClient.getPaymentMethod());
        oldClient.setPhone1(newClient.getPhone1());
        oldClient.setPhone2(newClient.getPhone2());
        oldClient.setPostal(newClient.getPostal());
        oldClient.setPrice(newClient.getPrice());
        oldClient.setProvince(newClient.getProvince());
        oldClient.setRecording(newClient.getRecording());
        oldClient.setRefunded(newClient.getRefunded());
        oldClient.setRegFee(newClient.getRegFee());
        oldClient.setStatus(newClient.getStatus());
        oldClient.setTaxRate(newClient.getTaxRate());
        oldClient.setTrim(newClient.getTrim());
        oldClient.setVerifier(newClient.getVerifier());
        oldClient.setYear(newClient.getYear());
        em.getTransaction().commit();
        em.close();
        emf.close();
        return newClient.getId();
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ You're not quite clear. Are you asking for a review or asking for a solution to your problem? Right now this question reads like the latter to me and that'd be off-topic here. \$\endgroup\$ – Vogel612 Sep 14 '14 at 17:36
  • \$\begingroup\$ Pls review again \$\endgroup\$ – Thufir Sep 15 '14 at 17:42
  • \$\begingroup\$ does your code work? \$\endgroup\$ – Malachi Sep 15 '14 at 18:10
  • \$\begingroup\$ @Malachi it runs, just bad performance.. \$\endgroup\$ – Thufir Sep 16 '14 at 4:41
  • \$\begingroup\$ @Malachi different version, this runs as well. \$\endgroup\$ – Thufir Sep 16 '14 at 11:26
1
\$\begingroup\$

Entity manager factories are usually pretty expensive to create, since they must parse their configuration and mapping files, and possibly set up a connection pool to your data source. Factories are thread-safe, so it's safe to have only one per persistence unit in your application.

Recreate the factory only when it fails to create an entity manager (and log when you had to do so, because it may be indicative of problems). Close the manager when your application shuts down, to give it the chance to shut down and cleanly disconnect any database connection pools.

That said, my experience is that this manual managing leads to bloat and maintenance overhead. If adding a library or container to help you is out of scope for your application, consider creating a template class so you can focus on the what instead of the how-with-resources.


On the review of the code:

  • Not fail-safe — Exceptions may leave the code with dangling transactions. In the absence of a managing container like Spring, transactions need to be explicitly rolled back on failure.
  • try-catch NPE — Consider explicitly checking for null instead. catch NPE is almost never a good idea.
  • dateCreated/Updated as String — Prefer using SQL Timestamp (or even Date or JodaTime's DateTime), since this will allow you to do range queries, and removes the need for parsing and formatting strings.
  • send(id)/stamp() — Lines 3-4 in send can be replaced by stamp().

The giant list of Y.setX(Z.getX()) always annoyed me in applications I've maintained. Would it be feasible to use Bean reflection to sort this out? It seems so easy to miss a field.

\$\endgroup\$
  • \$\begingroup\$ I've never use Bean reflection, but you're spot-on, I'll look into that. On the factories, how do I "Recreate the factory only when it fails to create an entity manager"? try-catch? \$\endgroup\$ – Thufir Sep 21 '14 at 16:45
  • \$\begingroup\$ @Thufir try-catch, probably check for null as well, and that the returned entity manager is not closed. Other than IllegalStateException when the factory is closed, I see no API-specified way for factories to fail, so I imagine different factory implementations will fail in different and perhaps non-deterministic ways. \$\endgroup\$ – JvR Sep 22 '14 at 13:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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