How can I improve my DAO? - Java EE

I want to access my applicant database that's why I created a DAO class for it.

I think I have lots of code smells because I keep on repeating some code. So what can I do to make my code simpler in order to achieve less code smells? What rules am I violating? How can i improve my code? Thank you.

My code is as follows:

public class ApplicantDAO {

private static ApplicantDAO me = null;

private ApplicantDAO(){};

public static synchronized ApplicantDAO getInstance() {
    if(me == null) {
        me = new ApplicantDAO();
    }
    return me;
}

public Applicant getApplicant(int applicantNumber) throws SQLException {
    Applicant applicant = null;

    Connection conn = null; 
    Statement statement= null;
    String query = null;
    ResultSet rs = null;

    try {
        conn = ConnectionManager.getConnection();
        statement = conn.createStatement();
        query = "SELECT * FROM applicant WHERE applicant_no = '" + applicantNumber +"'";    //check applicant_number
        rs = statement.executeQuery(query);

        while(rs.next()){
            applicant = new Applicant();

            applicant.setApplicantNumber(rs.getInt("applicant_no"));
            applicant.setApplicationDate(rs.getString("applicant_date")); 
            applicant.setfName(rs.getString("first_name"));
            applicant.setlName(rs.getString("last_name"));
            applicant.setmName(rs.getString("middle_name"));
            applicant.setAge(rs.getInt("age"));
            applicant.setGender(rs.getString("gender"));
            applicant.setEmail(rs.getString("email_address"));
            applicant.setContactNumber(rs.getString("contact_no"));
            applicant.setCity(rs.getString("city"));
            applicant.setSchool(rs.getString("school"));
            applicant.setCourse(rs.getString("course"));
            applicant.setYearGraduated(rs.getInt("year_graduated"));
            applicant.setYearWorkExp(rs.getInt("year_work_exp"));
            applicant.setSourceChannel(rs.getString("source_channel"));
            ap开发者_JS百科plicant.setStatus_id(rs.getInt("status_id"));

        }

    } catch (SQLException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }finally {
        if (rs != null) try { rs.close(); } catch (SQLException logOrIgnore) {}
        if (statement != null) try { statement.close(); } catch (SQLException logOrIgnore) {}
        if (conn!= null) try { conn.close(); } catch (SQLException logOrIgnore) {}
    }


    return applicant;
}

public ArrayList<Applicant> getApplicants() throws SQLException{
    ArrayList<Applicant> applicantList = null;
    Applicant applicant = null;

    Connection conn = null; 
    Statement statement= null;
    String query = null;
    ResultSet rs = null;

    try {
        conn = ConnectionManager.getConnection();
        statement = conn.createStatement();
        query = "select * from applicant";
        rs = statement.executeQuery(query);

        while(rs.next()){
            if(applicantList == null){
                applicantList = new ArrayList<Applicant>();
            }
            applicant = new Applicant();

            applicant.setApplicantNumber(rs.getInt("applicant_no"));
            applicant.setApplicationDate(rs.getString("applicant_date")); 
            applicant.setfName(rs.getString("first_name"));
            applicant.setlName(rs.getString("last_name"));
            applicant.setmName(rs.getString("middle_name"));
            applicant.setAge(rs.getInt("age"));
            applicant.setGender(rs.getString("gender"));
            applicant.setEmail(rs.getString("email_address"));
            applicant.setContactNumber(rs.getString("contact_no"));
            applicant.setCity(rs.getString("city"));
            applicant.setSchool(rs.getString("school"));
            applicant.setCourse(rs.getString("course"));
            applicant.setYearGraduated(rs.getInt("year_graduated"));
            applicant.setYearWorkExp(rs.getInt("year_work_exp"));
            applicant.setSourceChannel(rs.getString("source_channel"));
            applicant.setStatus_id(rs.getInt("status_id"));

            applicantList.add(applicant);
        }

    } catch (SQLException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }finally{
         if (rs != null) try { rs.close(); } catch (SQLException logOrIgnore) {}
         if (statement != null) try { statement.close(); } catch (SQLException logOrIgnore) {}
         if (conn!= null) try { conn.close(); } catch (SQLException logOrIgnore) {}
    }
    return applicantList;
}

Giant, glaring issues I see:

1. The DAO is a singleton. Why?
2. You're not using EntityManager. Also, why?

If you're actually using Java EE (as the question is tagged) rather than J2EE (which is a miserable spec built around Java 1.4) then the DAO pattern is completely unnecessary. EntityManager is the new DAO.

Have a look at the first few sections of The Java EE 6 Tutorial - Persistence.

we are required to use J2ee.. :(

Ok, so you still need to fix the singleton implementation. There is simply no reason to only create one instance of this object, given that it does not store any internal state whatsoever. There's an easy fix:

• Remove private static ApplicantDAO me = null; entirely

• Change the getInstance() implementation to

  public static ApplicantDAO getInstance() {
      return new ApplicantDAO();
  }
  

Other smells:

• You'll almost always want to declare Lists rather than ArrayLists, so change declarations like

  public ArrayList<Applicant> getApplicants() throws SQLException
  // to
  public List<Applicant> getApplicants() throws SQLException
  
  // and
  
  ArrayList<Applicant> applicantList = null;
  // to
  List<Applicant> applicantList = null;
  

To give you some idea of what a more modern DAO would look like:

@Stateless
public class JPAApplicantDAO implements ApplicantDAO {

    @PersistenceContext(unitName = "myPU")
    private EntityManager entityManager;

    @Override
    public Applicant getByID(Long applicantID) {
        return entityManager.find(Applicant.class, applicantID);
    }

    @Override
    public void update(Applicant applicant) {
        applicant.setLastModifiedDate(new Date());
        entityManager.merge(applicant);
    }

    @Override
    public void delete(Applicant applicant) {
        Applicant deletedApplicant = applicant;
        if (!entityManager.contains(applicant)) {
            deletedApplicant = entityManager.merge(applicant);
        }

        entityManager.remove(deletedApplicant);
    }

    @Override
    public List<Applicant> getBySomethingID(Long somethingID) {
        return entityManager.createNamedQuery("Applicant.getBySomethingID", Applicant.class)
                            .setParameter("somethingID", somethingID)
                            .getResultList();
    }
}

Now it has been suggested to forgo the entire DAO concept and directly use the entity manager everywhere. I don't entirely agree with that.

This DAO example shows 4 different methods. The first method is a simple get via the main ID of an entity. It's this kind of method that makes people wonder whether the DAO abstraction is still needed. But read on.

The second method shows an update method. In this case the application might want to do something extra to the entity, such as setting a last modified date. The DAO is quite a natural place to do this. Yes, it could be done in the DB as well, but then the DAO would still be handy since you might have to read the entity back in order to learn about the date being set.

The third method is a delete method. Due to a peculiar issue in the JPA spec, you can only delete an entity that's in the attached state. This means some extra logic to check whether it's attached (contained in the persistence context) and if not attach it (merge it).

The fourth method shows data retrieval via a (JPQL) query. Both the name of the query and the name of the parameter are not type-safe. The DAO conveniently hides this behind a type-safe Java method. Yes, you can extract those names to a constant, but the association between this particular parameter and this particular query would still not be enforced.

In general, the DAO allows a certain amount of refactoring to take place. At some point I might want to replace the JPQL query with a criteria query. Changing this at all call-sites might be problematic. Then there's the case that an entity manager as a general DAO is simply to powerful. I don't want to send these to all client sites (in case of remote clients this is even impossible or very bad practice).

Finally, using an entity manager from client code that is itself not transactional, means this client has to worry about transactions. This adds a lot of verbosity to the code. With a DAO, the client code becomes much simpler:

@Named
@RequestScoped
public class SomeBean {

    @EJB
    private ApplicantDAO applicantDAO;

    public void someMethod() {
        applicantDAO.delete(applicant);
    }
}

Your example looks like a perfect "before" picture to show a great looking "after" picture after using Hibernate for mapping objects to the relational database and Spring dependency injection features to support connection and transaction management.

You could use Java Persistence API and most of this boring code would be unnecessary.

Worst code smell ever: you're reinventing the wheel - again. Why not using some open library, such as Spring JDBC ? (nota: you may read the code to learn from it too!)

If you're practising here, and/or are not allowed to use external code, then here are a few hints to enhance your code:

• all the code which is not about defining a statement (with its arguments) or processing a resultSet, may be factorized in reusable methods to reduce code duplication. This may lead you to callback-style arguments or varargs for statement creation, and adapter-style components for Applicants reconstitution (try looking into Spring's code for its JdbcTemplate and RowMapper classes)
• your signatures are too precise: why return an ArrayList when you may return a List?
• why is your getInstance() method synchronized? You may avoid this by initializing me at class initialization time:

  private static ApplicantDAO me = new ApplicantDAO();
  public static ApplicantDAO getInstance() { return me; }
  

• your log policy is not defined it seems: I assume it's on purpose here, but keep in mind that this is quite important in real-life apps.