Java Data-Entity model: Constructing general types

Question

I have had some trouble with using a general type in a static method.

All comments on the source code are welcome, especially ones that significantly improve the code. I am also currently not planning on using any external framework, apart from JDBC, to keep it still simple, please do not put too much emphasis on that.

My view on not using external frameworks is also supported by the fact that the operations I will be using on the database are very minimal:

• Inserting data
• Updating data
• Retrieving all fields. (And simply by putting in a different SQL Query you could already select what fields to retrieve

I do not plan on making a full framework, so I know that it will not be supporting everything. The speed of retrieving all fields is neither a real issue, as this will be pretty much only done on server bootup, and if used at any other time it will be done in a background task for which I do not really care when it is finished.

Entity.java:

abstract public class Entity<KeyType, DataType> {
    protected KeyType key;
    protected List<Object> data;

    public Entity() {
        data = new ArrayList<>();
    }

    //abstract public static Map<KeyType, DataType> getAll();

    protected List<Object> createData(final DataAction dataAction) {
        List<Object> list = new ArrayList<>();
        if (dataAction == DataAction.INSERT) {
            list.add(key);
        }

        list.addAll(data);

        if (dataAction == DataAction.UPDATE) {
            list.add(key);
        }
        return list;
    }

    abstract public void insert();

    abstract public void update();

    protected static <KeyType, DataType> Map<KeyType, DataType> getData(final Class<DataType> dataTypeClass, final String query) {
        Map<KeyType, DataType> map = new HashMap<>();
        try {
            PreparedStatement preparedStatement = DatabaseConnection.getConnection().prepareStatement(query);
            ResultSet resultSet = preparedStatement.executeQuery();
            while (resultSet.next()) {
                KeyType key = (KeyType)resultSet.getObject(1);
                int index = 2;
                List<Object> dataList = new ArrayList<>();
                while (resultSet.getObject(index) != null) {
                    dataList.add(resultSet.getObject(index));
                    index++;
                }
                DataType dataObject = null;
                try {
                    dataObject = dataTypeClass.getConstructor(List.class).newInstance(dataList);
                } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException | NoSuchMethodException | SecurityException ex) {
                    Logger.getLogger(Entity.class.getName()).log(Level.SEVERE, null, ex);
                }
                map.put(key, dataObject);
            }
        } catch (SQLException ex) {
            Logger.getLogger(Entity.class.getName()).log(Level.SEVERE, null, ex);
        }        
        return map;
    }

    protected void executeQuery(final String query, final List<Object> data) {
        try {
            PreparedStatement preparedStatement = DatabaseConnection.getConnection().prepareStatement(query);
            int dataIndex = 0;
            for (Object dataObject : data) {
                preparedStatement.setObject(dataIndex, dataObject);
                dataIndex++;
            }
            preparedStatement.execute();
            preparedStatement.close();
        } catch (SQLException ex) {
            Logger.getLogger(Entity.class.getName()).log(Level.SEVERE, null, ex);
        }
    }
}

A concrete implementation, Account.java:

public class Account extends Entity<String, Account> {
    private final static String SELECT_ALL_QUERY = "SELECT * FROM accounts";
    private final static String INSERT_QUERY = "INSERT INTO accounts (username, password) VALUES(?, ?)";
    private final static String UPDATE_QUERY = "UPDATE accounts SET password=? WHERE username=?";

    private String username;
    private String password;

    public Account(final String username, final String password) {
        this.username = username;
        this.password = password;

        key = username;
        data.add(password);
    }

    public Account(final List<Object> data) {
        this((String)data.get(0), (String)data.get(1));
    }

    public String getUsername() {
        return username;
    }

    public void setUsername(final String username) {
        this.username = username;
    }

    public String getPassword() {
        return password;
    }

    public void setPassword(final String password) {
        this.password = password;
    }

    public static Map<String, Account> selectAll() {
        return getData(Account.class, SELECT_ALL_QUERY);
    }

    @Override
    public void insert() {
        executeQuery(INSERT_QUERY, createData(DataAction.INSERT));
    }

    @Override
    public void update() {
        executeQuery(UPDATE_QUERY, createData(DataAction.UPDATE));
    }
}

I am generally happy about the concrete implementation, it seems like I have managed to bring it down to a bare minimum, except public Account(final List<Object> data) does not seem that nice, but I can live with it.

However, as guessed, the getData() from Entity is definately not nice, and I would like to improve it if possible.

What I would like to use is something like DataType dataObject = new DataType(dataList), but it seems like Generic Type Arguments cannot be instantiated.

So are there any ways of optimizing my current code in my current view? And is it possible to decouple the concrete classes and abstract classes even more?

EDIT:

Added a relevant question (I don't think I should make a fully new question for this thing, right?):

Is there a way to move the static Strings (SQL Queries) and the insert() and update() out of the Account class, into the Entity class?

Answer 1

To avoid the use of reflection in your getData method you should accept a factory that given a ResultSet creates instances of the specific type. Your selectAll method would then be something like:

public static Map<String, Account> selectAll() 
{
  return getData(
    new EntityFactory<Account>()
    {
      public Account newInstance(ResultSet resultSet) throws SQLException
      {
        return new Account(resultSet.getString(0), resultSet.getString(1));
      }
    },
    SELECT_ALL_QUERY
  );
}

The getData method then ends up something like:

protected static <K, T extends Entity<K>> Map<K, T> getData(EntityFactory<T> entityFactory, String query) 
{
  Connection connection = null;
  PreparedStatement preparedStatement = null;      
  ResultSet resultSet = null;

  try 
  {
    connection = dataSource.getConnection();

    preparedStatement = connection.prepareStatement(query);

    resultSet = preparedStatement.executeQuery();

    Map<K, T> entities = new HashMap<>();

    while (resultSet.next()) 
    {
      Entity<K> entity = entityFactory.newInstance(resultSet);
      entities.put(entity.getKey(), entity);
    }

    return entities;
  }
  finally 
  {
    closeQuietly(resultSet);
    closeQuietly(prepareStatement);
    closeQuietly(connection);
  }
}

And assumes the Entity looks like:

public interface Entity<K>
{
  public K getKey();
}

This allows you to remove the reflection and keeps the code that understands the database structure in one place. You should also use a similar template pattern to map from the domain object to the prepared statement when doing inserts and updates.

Now you've asked for comments on the code in general.

First off, code like this violates the Single Responsibility Principal and Seperation Of Concerns. A domain class should be a domain class and not contain persistance logic. Look at patterns like the Data Access Object for how this should be done.

Second, while I'm all for keeping it simple, Hibernate solved this problem a long time ago and JPA standardized it - you need a very good reason not to use one or both of these APIs.

Finally, your use of database resources - if you are going to use JDBC directly you have to clean up properly. Database connections are expensive resources and should be handled as such, the basic template for any JDBC call should be:

Connection connection = null;
PreparedStatement preparedStatement = null;
ResultSet resultSet = null;

try 
{
  connection = //get connection from pool or single instance.

  preparedStatement = connection.prepareStatement("SELECT * FROM table WHERE column = ?");
  preparedStatement.setString(1, "some string");

  resultSet = preparedStatement.executeQuery();

  while (resultSet.next())
  {
    //logic goes here.
  }
}
catch (SQLException e)
{
  //Handle exceptions.
}
finally 
{
  closeQuietly(resultSet);
  closeQuietly(prepareStatement);
  closeQuietly(connection);
}

The closeQuietly method has to be overloaded but should take the general form:

try 
{
  if (resultSet != null)
  {
    resultSet.close();
  }
}
catch (SQLException e)
{  
  //Log exceptions but don't re-throw.
}