views:

149

answers:

5

I am trying to develop generic DAO in java.I have tried the following.Is this a good way to implement generic dao?I dont want to use hibernate.I am trying to make it as generic as possible so that i dont have to repeate the same code again and again.

public abstract class  AbstractDAO<T> {



    protected ResultSet findbyId(String tablename, Integer id){
        ResultSet rs= null;
        try {
           // the following lins in not working;
            pStmt = cn.prepareStatement("SELECT * FROM "+ tablename+ "WHERE id = ?");
            pStmt.setInt(1, id);
            rs = pStmt.executeQuery();


        } catch (SQLException ex) {
            System.out.println("ERROR in findbyid " +ex.getMessage() +ex.getCause());
            ex.printStackTrace();
        }finally{
            return rs;
        }

    }

}

Now i have

public class UserDAO extends AbstractDAO<User>{

  public List<User> findbyid(int id){
   Resultset rs =findbyid("USERS",id) //USERS is tablename in db
   List<Users> users = convertToList(rs);
   return users; 
}


 private List<User> convertToList(ResultSet rs)  {
        List<User> userList= new ArrayList();
        User user= new User();;
        try {
            while (rs.next()) {
                user.setId(rs.getInt("id"));
                user.setUsername(rs.getString("username"));
                user.setFname(rs.getString("fname"));
                user.setLname(rs.getString("lname"));
                user.setUsertype(rs.getInt("usertype"));
                user.setPasswd(rs.getString("passwd"));
                userList.add(user);
            }
        } catch (SQLException ex) {
            Logger.getLogger(UserDAO.class.getName()).log(Level.SEVERE, null, ex);
        }

        return userList;

    }
}
A: 
Matthieu BROUILLARD
i want to know why is my line mentioned in code not working
akshay
+1  A: 

It is okay but change the method

private List<User> convertToList(ResultSet rs)  { 
        List<User> userList= new ArrayList(); 
        User user= new User();; 
        try { 
            while (rs.next()) { 
                user.setId(rs.getInt("id")); 
                user.setUsername(rs.getString("username")); 
                user.setFname(rs.getString("fname")); 
                user.setLname(rs.getString("lname")); 
                user.setUsertype(rs.getInt("usertype")); 
                user.setPasswd(rs.getString("passwd")); 
                userList.add(user); 
            } 
        } catch (SQLException ex) { 
            Logger.getLogger(UserDAO.class.getName()).log(Level.SEVERE, null, ex); 
        } 

        return userList; 

    } 

to

private List<User> convertToList(ResultSet rs)  { 
        List<User> userList= new ArrayList<User>(); 
        try { 
            while (rs.next()) { 
                User user= new User();
                user.setId(rs.getInt("id")); 
                user.setUsername(rs.getString("username")); 
                user.setFname(rs.getString("fname")); 
                user.setLname(rs.getString("lname")); 
                user.setUsertype(rs.getInt("usertype")); 
                user.setPasswd(rs.getString("passwd")); 
                userList.add(user); 
            } 
        } catch (SQLException ex) { 
            Logger.getLogger(UserDAO.class.getName()).log(Level.SEVERE, null, ex); 
        } 

        return userList; 

    } 

User object should be created inside while loop.

RaviG
plz see the line // the following lins in not working;
akshay
This code will leak resources if an SQLException is thrown. You need to ensure the ResultSet, Statement and Connection are closed. Easiest way is to use a framework like Spring JDBC rather than raw JDBC.
Adamski
What is the error r u getting?
RaviG
Is that because of this? (missing space before WHERE) Change the below one "SELECT * FROM "+ tablename+ "WHERE id = ?"to"SELECT * FROM "+ tablename+ " WHERE id = ?"
RaviG
+3  A: 

My advice:

  • Don't write a generic DAO; Generic classes come back to bite you when you realise they don't quite do what you need in a specific situation and often end up growing in complexity to cover the ever-increasing array of use-cases. Better to code application specific DAOs and then attempt to generify any common behaviour later on.
  • Consider using Spring JDBC to write app-specific DAOs but in a much more compact and less error-prone fashion than JDBC. Also, unlike Hibernate, Spring JDBC only acts a thin wrapper around raw JDBC giving you finer grained control and more visibility.

Example

// Create or inject underlying DataSource.
DataSource ds = ...
// Initialise Spring template, which we'll use for querying.
SimpleJdbcTemplate tmpl = new SimpleJdbcTemplate(ds);     

// Create collection of "Role"s: The business object we're interested in.
Set<Role> roles = new HashSet<Role>();

// Query database for roles, use row mapper to extract and create
// business objects and add to collection.  If an error occurs Spring
// will translate the checked SQLException into an unchecked Spring
// DataAccessException and also close any open resources (ResultSet, Connection).
roles.addAll(tmpl.query("select * from Role", new ParameterizedRowMapper<Role>() {
  public Role mapRow(ResultSet resultSet, int i) throws SQLException {
    return new Role(resultSet.getString("RoleName"));
  }
}));
Adamski
Another vote for Spring JDBC. It handles a lot of the boilerplate connection stuff (to avoid resource leaks). In addition, it provides a much nicer way of mapping columns to your objects.One other comment, I wouldn't hard-code the primary key column name. As soon as you do that, someone comes along a creates a table with a column named something else, or uses a multi-column primary key.
David
+3  A: 

If you can live with Spring, I will suggest the following improvements:

  • Let Spring do the exception handling.
  • Use JdbcTemplate instead of creating prepared statements yourself.

Independent of using Spring, I will recommend the following:

  • Don't send the table name as parameter. That should be done in the initialization phase.
  • Use a String on the id parameter, since that's much more generic.
  • Consider returning a generic object instead of a collection, since the collection should always contain only one object.

An improved AbstractDao with Spring:

import java.util.Collection;

import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;

public abstract class AbstractDao<T> {

    protected final RowMapper<T> rowMapper;

    protected final String findByIdSql;

    protected final JdbcTemplate jdbcTemplate;

    protected AbstractDao(RowMapper<T> rowMapper, String tableName,
            JdbcTemplate jdbcTemplate) {
        this.rowMapper = rowMapper;
        this.findByIdSql = "SELECT * FROM " + tableName + "WHERE id = ?";
        this.jdbcTemplate = jdbcTemplate;
    }

    public  Collection<T> findById(final String id) {
        Object[] params = {id};
        return jdbcTemplate.query(findByIdSql, params, rowMapper);
    }
}

As you see, no exception handling or hacking with the primitive SQL classes. This templates closes the ResultSet for you, which I can't see in your code.

And the UserDao:

import java.sql.ResultSet;
import java.sql.SQLException;

import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;

public class UserDao extends AbstractDao<User> {

    private final static String TABLE_NAME = "USERS";

    public UserDao(JdbcTemplate jdbcTemplate) {
        super(new UserRowMapper(), TABLE_NAME, jdbcTemplate);
    }

    private static class UserRowMapper implements RowMapper<User> {
        public User mapRow(ResultSet rs, int rowNum) throws SQLException {
            User user = new User();
            user.setUserName(rs.getString("username"));
            user.setFirstName(rs.getString("fname"));
            user.setLastName(rs.getString("lname"));

            return user;
        }
    }
}

Updated:

When you know the id and the id corresponds to a single row in the database, you should consider returning a generic object instead of a collection.

public T findUniqueObjectById(final String id) {
    Object[] params = {id};
    return jdbcTemplate.queryForObject(findByIdSql, params, rowMapper);
}

This makes your service code more readable, since you don't need to retrieve the user from a list, but only:

User user = userDao.findUniqueObjectById("22");
Espen
A: 

You need to add a space before your "WHERE" clause see below:

pStmt = cn.prepareStatement("SELECT * FROM "+ tablename+ "WHERE id = ?");

to

 pStmt = cn.prepareStatement("SELECT * FROM "+ tablename+ " WHERE id = ?");
Robert