looking for suggestions on better DAO design

[J. Kevin Robbins]

I'm not entirely happy with the way I've designed my servlets and DAOs and I'm looking for suggestions for improvement. Right now I have a context listener which does the JNDI lookup of the datasource and stores it in the application scope. My old way of doing this was to do the JNDI lookup in the init method of each servlet, so this seems to be a lot more efficient. However, in order for the DAO to get the datasource and then a connection, I have to pass a ServletConfig to the DAO constructor. My DAOs look like this:

public class PDP00003DAO {

    private final static Logger LOGGER = Logger.getLogger(PDP00003DAO.class.getName());
    private final static String SQL = "{call PDP00003(?,?,?)}";
    private DataSource ds = null;

    public PDP00003DAO(ServletConfig sc) {
        ds = (DataSource) sc.getServletContext().getAttribute("as400DS");
    }

    public List<String> getColNames() {
        Connection conn = null;
        List<String> colNames = new ArrayList<String>();
        ResultSet rset = null;
        CallableStatement cs = null;
        try {
            conn = ds.getConnection();
            cs = conn.prepareCall(SQL);
<rest of code deleted>

There are two things that I dislike about this technique. One is the use of an instance variable for the datasource. The other is all these imports of ServletConfig. I'm thinking that there must be a better way to do this but I just don't have the experience to figure it out.

What techniques are some of you using in your servlet to DAO patterns that accomplishes the same result?

[Dhaval J. Patel]

Hey Jk,
I would suggest you use a totally different class ( lets say DataSourceHolder ) in your webapp. In your context listener you initialize this object and store your database object in one of its static class variables. And then you can get the datasource from that object. But I would suggest, instead of getting the entire datasource object, only fetch the connection from that object if you do not require anything other than the connection. This is what your DataSourceHolderClass would look like.

public class DataSourceHolder {

	private DataSource _dataSource;
	private static DataSourceHolder dataSourceHolder;

	private DataSourceHolder() {

	}

	// call this method to get the DataSourceHolder object ensuring there is
	// only one copy of this object, which is static, hence wont be eligible for
	// garbage collection
	// unless your class loader gets garbage collected.

	// the first time this method would be called would be from your context
	// listener, when a new instance would be created and stored
	// in the dataSourceHolder instance variable.

	public static DataSourceHolder getInstance() {

		if (dataSourceHolder == null) {

			dataSourceHolder = new DataSourceHolder();

		}

		return dataSourceHolder;

	}

	// After calling the getInstance method from the context listener call this
	// is the method from the context listener to set the datasource for your
	// app
	public void setDataSource (DataSource dataSource){  
  
		this._dataSource=dataSource  
  
	}

        // this method returns a connection object for anyone who needs it.
 
	public Connection getConnection() {

		return _dataSource.getConnection();

	}

}

Now the code in your DAO constructor would look something like this.

public class PDP00003DAO {  
  
    private final static Logger LOGGER = Logger.getLogger(PDP00003DAO.class.getName());  
    private final static String SQL = "{call PDP00003(?,?,?)}";  
    private Connection conn=null;
   
    public PDP00003DAO() {  
        DataSourceHolder ds= DataSourceHolder.getInstance();
        conn=ds.getConnection(); 
    }  

This solves your problem of getting your datasource from the servletContext.

There is still one bad practice involved in this. But that totally depends on your application logic. I would suggest that instead of getting a connection from the DAO constructor, get the connection in the class calling the DAO and inject into the DAO using the DAO constructor.

public class PDP00003DAO {  
  
    private final static Logger LOGGER = Logger.getLogger(PDP00003DAO.class.getName());  
    private final static String SQL = "{call PDP00003(?,?,?)}";  
    private Connection conn; 
  
    public PDP00003DAO (Connection conn) {  
       this.conn=conn;
    }  

The benefit of doing this is that you would have more control when making transactions. Suppose you have to call methods from two different DAOs to complete a transaction. If you get your connection from DAO constructor, then you have no way of making the transaction atomic. But if you inject the same connection into all DAOs from your calling class you have full control.

This is what your calling class would look like in that Scenario

public class DAOCaller{

public void doTransaction(){

		//get connection from the calling object

		DataSourceHolder ds=DataSourceHolder.getInstance();
		Connection conn=ds.getConnection();

		// inject the connection into the DAO for their use

		PDP00003DAO pdp3 = new PDP0003DAO(conn);                 //  DAO one 
		PDP00004DAO pdp4 = new PDP0004DAO(conn);                 // second DAO. 

		//here comes the important part where you make the transaction atomic.

		try{
		   conn.setAutoCommit(false);

		   pdp3.insertValue(3);     //just an example. I know it would be much more than just inserting a simple int value into the database
		   pdp4.insertValue(5);    
		   
		   conn.commit();   // if it gets here means both values got inserted correctly, hence commit the changes.

		} catch(SQLException se){
		   
		   conn.rollback();  // if it gets here, an exception occured during the transaction, hence rollback all changes to maintain the database integrity.

		}

	}


}

This totally depends on your application. But I would go with injection of only a connection object into the DAO, so that I have full control over the transaction.

I know its a long long answer to read, but I hope I helped.

Dhaval Patel

[J. Kevin Robbins]

That's very helpful, thank you. It's always enlightening to see how someone else solves the same problem. I'll play around with this.

I used to pass the connection object to the DAO, but then I was puzzled over the correct place to close it; the DAO or servlet. I would occasionally create a bug where the connection was closed too soon. It seemed to make more sense for the DAO to open and close everything (connection, preparedStatement, resultSet). But I never considered the scenario of needing to use more than one DAO in an atomic transaction. That gives it a new perspective.

Thanks again for your input.

[Jimmy Clark]

What techniques are some of you using in your servlet to DAO patterns that accomplishes the same result?

In terms of the popular J2EE programming model, Java Servlet implementations are part the Presentation tier and should only be responsible for GUI and Controller functionality. Data Access Objects are part of the Business tier and are responsible for data access and storage logic. With your design of direct servlet to DAO communication, you have deviated from a standard design model and have introduced potential limitations. Also, if your servlet connects directly to a DAO, where is the business logic (if any)? Is this coded somewhere in a servlet class?

Check out the Business Delegate design pattern for more ideas on how to connect Presentation tier to Business tier.

[J. Kevin Robbins]

Jimmy Clark wrote:In terms of the popular J2EE programming model, Java Servlet implementations are part the Presentation tier and should only be responsible for GUI and Controller functionality. Data Access Objects are part of the Business tier and are responsible for data access and storage logic. With your design of direct servlet to DAO communication, you have deviated from a standard design model and have introduced potential limitations. Also, if your servlet connects directly to a DAO, where is the business logic (if any)? Is this coded somewhere in a servlet class?

Check out the Business Delegate design pattern for more ideas on how to connect Presentation tier to Business tier.

If any business logic is required I generally put it a POJO. For instance, the following example uses a TuberReasonsChartBuilder class. But many of my servlets do nothing more than retrieve data and send it to a jsp. Here is a typical servlet:

protected void processRequest(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {

        BarChartBean chartBean = new BarChartBean();
        List<UpDownHourlyBarChartBean> beanList = new ArrayList<UpDownHourlyBarChartBean>();
        String machName = request.getParameter("machname");
        ServletConfig sc = this.getServletConfig();
        UpDownHourlyBarChartDAO barChartDAO = new UpDownHourlyBarChartDAO(sc);
        beanList = barChartDAO.getResults(machName, "", "");
        if (!beanList.isEmpty()) {
            TuberReasonsChartBuilder chartBuilder = new TuberReasonsChartBuilder();
            chartBean = chartBuilder.createChart(beanList);
            request.setAttribute("chartbean", chartBean);
            request.setAttribute("machinename",  machName.toUpperCase());
            RequestDispatcher rd = request.getRequestDispatcher("/service/it/tbrreasons.jsp");
            rd.forward(request, response);
        }
        else {
            PrintWriter out = response.getWriter();
            out.println("<html>");
            out.println("<head>");
            out.println("<title>No Data Found</title>");
            out.println("</head>");
            out.println("<body>");
            out.println("<div style=\"font-size:xx-large; font-family:Arial\">");
            out.println("No Data to Display");
            out.println("</div>");
            out.println("</body>");
            out.println("</html>");
            out.close();
        }
    }

So are you saying all this code (other than the outs) should be in a business class and the servlet should do nothing more than call that class and get the list back? I understand that doing so decreases coupling, but it seems that it does so at the cost of another layer of complexity.

And I don't see how Business Delegate helps me. Everything I've read about that pattern describes it as being used for working with remote EJBs. How is that applicable to this?

[Jimmy Clark]

All of that logic in the servlet class is now dependent upon the Java Servlet API and also a Web Container. It does not have to be and the design introduces a significant dependency as well as a few limitations. For example, can you simply test DAO logic from a command-line? Do you always need to have a web container running to execute the business logic? Are you often restarting the container to test every change? These aspects might not seem to matter for a very small application, but in the enterprise world they are significant. When you can test DAO logic from a command-line you can typically identify logic problems faster. You can also develop GUI elements independently from a relational database using mock objects.

By separating the Presentation tier code from the Business tier, you are able to develop in a more clear and concise manner. This comes in handy when there are teams of developers handling different parts. Again, if the application is small and being developed by one individual, these rationals may not seem to beneficial. Aside, the J2EE three-tier programming model was designed for "Enterprise" systems.

The purpose of the Business Delegate is to reduce the coupling between the Presentation code ond the Business code. The example in common pattern documentation about "remote" EJB is only one example of one implementation. This is not the basis of the design pattern, as patterns do not specify or dictate any specific implementation.

JSP (GUI) ---- > Servlet (Controller) ----> Business Delegate ----> Business Object ----> DAO ----> Database


Adding another object or two is not neccessarily adding additional complexity. And if it does and makes your development and maintenance efforts more efficient, then it is usually worth it.

[J. Kevin Robbins]

Thanks, good feedback. I'm going to refactor some of my previous work with this in mind. I can now see some things that I could have done better.

I'm the only developer here so the team paradigm isn't applicable, but that doesn't mean I shouldn't still practice good design fundamentals.

As for unit testing, that's a real weakness for me. I haven't been able to test DAOs with JUnit because I can't figure out how to mock up the connection. But I do try to write my business classes so they could be used from either a web app or a gui app. Not that they ever will be, but I think that's a good design criteria.

[Jimmy Clark]

Presentation Tier contains View and Controller

JSP (GUI) ---- > Servlet (Controller) ----> Business Delegate ---->

Business Tier contains Model

Business Object (Business logic) ----> DAO (Data access logic) ---->

Integration Tier contains Model

Database (Storage)

I haven't been able to test DAOs with JUnit because I can't figure out how to mock up the connection.

This is a by-product of your current design. If you redesign it a bit I'm sure you'll see how easy it becomes. A hint would be to enable your business methods to be run from a command-line. Good luck!