DAO methods and synchronized

Question

The following are methods I currently have in an Abstract DAO class. If there are concurrent calls, are they safe as they are or should synchronization be used? I know synchronization should be used if there is a reference to an attribute outside the scope of a method but it's unclear to me how should things be handled with an outside ressource.

public Connection getConnection() {
    // Call to singleton handling JDBC stuff
    return Database.getInstance().getCon();
}

public boolean isConnectionAvailable(){     
    if( getConnection() != null ){
        return true;
    }

    return false;
}

public PreparedStatement getPreparedStatement( String sqlStatement ){
    Connection connection = getConnection();
    PreparedStatement pS = null;

    if( connection != null ){
        try {
            pS = connection.prepareStatement( sqlStatement );
        } catch (SQLException e) {
            return null;
        }
    }

    return pS;
}

Edit: I might reformulate this question to include information on writing DAOs as it's what's important here.

Answer 1

I don't agree with this implementation at all.

First, DAOs should be given their connection information by the services that own units of work and transactions.

Second, I don't see an interface.

Third, I don't see model or domain objects.

Fourth, prepared statements should only be part of the internal implementation. If they're leaking out of your DAO, you're doing it wrong.

Fifth, passing a prepared statement out of the object makes responsibility for closing it and cleaning up far less clear. Your DAO will die a resource leak death in no time.

Here's an interface for a generic DAO. You'll notice that it's all CRUD operations, with no mention of connnections or any interfaces from java.sql package:

package persistence;

import java.io.Serializable;
import java.util.List;

public interface GenericDao<T, K extends Serializable>
{
    T find(K id);
    List<T> find();
    List<T> find(T example);
    List<T> find(String queryName, String [] paramNames, Object [] bindValues);

    K save(T instance);
    void update(T instance);
    void delete(T instance);
}

You can go a long way with this. It's a better abstraction. The T is your business object type, and K is the primary key.

Answer 2

If getCon() returns a new Connection each time it is called, or returns a ThreadLocal connection, then you are safe and there is no need to use synchronized

If you return the same connection to everyone you might still save in terms of synchronization, because there is no state in the connection that gets changed (in your current code). But you should avoid this practice. Consider a connection pool instead.

And a few notes on general design principles. DAOs form a separate layer. Each layer exists for a reason, not juts for the sake of having cool names. The DAO layer exists in order to abstract, or in other words - hide the database access from the services that use the DAO objects. In order to imagine it more clearly - the DAO has to be written in a way that if tomorrow you decide to switch from RDBMD storage (via JDBC) to XML storage, you should be able to do that by changing only the DAO objects and nothing else.

Answer 3

The JDBC Connection class is not guaranteed to be thread safe. If your Database.getInstance().getCon() method is always returning the same connection, then you will run into problems. If, however, it is using a pool such that each call to getInstance().getCon() returns a different connection you will be fine.

That said, if you are returning a different connection with each call to getCon() then the getPreparedStatement() won't work if you want two Prepared Statement calls to use the same connection (and same transaction).

I like Spring's JDBCTemplate class as a basis for my DAO classes.