Java – Thoughts on Refactoring a Generic DAO

Architecturedaojavarefactoringreinventing-the-wheel

I have designed a generic DAO in Java and am having confusion about whether to refactor it to different design or not.

PS Don't say to use already existing 3rd person framework. I know there are some present but I am learning Java and design patterns so I need all the practice I can get.

If there's any confusion then please ask and I'll clarify.

The current Architecture

DAOFactory.java

Contains the abstract methods for creation of DAODelete, DAOUpdate and 2 other interfaces to be implemented by concrete implementation classes
public Class method for creating the concrete Factories for producing the concrete implementaton of the interfaces mentioned before.

DAORead.java

public abstract <T> T getPojoForPrimarKey(Connection con,
        TableName tableName, String primaryKey) throws SQLException;

Similarly in my other interfaces I have generic methods.

Here Connection is passed by the service layer and closed there for transaction management.
TableName is a enum that I am using to have a switch case in lower layer of DAO for calling Table Specific functions. Also using an enum is helping me in type checking.

OracleRead.java

@Override
    public <T> T getPojoForPrimarKey(Connection con, TableName tableName,
            String primaryKey) throws SQLException {
        T currentPojo = null;
        PreparedStatement ps = null;
        ResultSet rs = null;
        try {
            String queryString = OracleSpecifics.queryString(tableName,
                    primaryKey, QueryType.READ);
            ps = con.prepareStatement(queryString);
            rs = ps.executeQuery();
            if (rs.next()) {
                currentPojo = OracleSpecifics.getPojoFromResultSet(tableName, rs);
            }
        } finally {
            DAOFactory.closeAll(ps, rs);
        }
        return currentPojo;
    }

This is how I have implemented the deleteFrom function from the interface in concrete class.

Here OracleSpecifics is the lower layer of DAO that I was talking about. It basically contains switch cases to check for specific Table.

OracleSpecifics.java

// These functions call table-specific functions
    @SuppressWarnings("unchecked")
    static <T> T getPojoFromResultSet(TableName tableName, ResultSet rs)
            throws SQLException {

        switch (tableName) {
        case ASEEM_MASTER:
        case ASEEM_MASTER_TEMP:
            return (T) SpecAseemMaster.getPojo(rs);
        case AUTHENTICATE_TABLE:
            return (T) SpecAseemRole.getPojo(rs);
        default:
            return null;
        }
    }

Here SpecAseemMaster and SpecAseemRole are Table Specific (specifically schema specific) layer producing the required fields

SpecAseemRole.java

public static AuthenticationDetails getPojo(ResultSet rs)
            throws SQLException {
        AuthenticationDetails cur = new AuthenticationDetails();
        cur.setUserName(rs.getString(1));
        cur.setPassWord(rs.getString(2));
        cur.setRole(rs.getString(3));
        return cur;
    }

In service layer

The type by which the DAO's method is called is the Pojo representing the schema's data fields. e.g.

myFactory.getDAORead().<AuthenticationDetails> getPojoForPrimarKey(con, CUR_TABLE, primaryKey);

Here AuthenticationDetails is my Pojo aka DTO for communicating between my Controller and service layer (in Model).

Please Note that I am giving just a overview in the simplest way. There are 4 interfaces in the abstract DAOs containing total of 8 abstract methods implemented by 4 concrete classes. All these generic classes depend on a lower layer in DAO for getting specific queries, PreparedStatements etc. which further calls functions in auxiliary utility class's functions depending on the tableName passed down this hierarchy. The bottom layer is schema-specific.

My Confusion about refactoring

I am satisfied with the upper layers of DAO but I think that at the the lower level specifically at the OracleSpecific the things which should have strong coupling has weak coupling. e.g. there are different methods for getting PreparedStatement for insert and update. Each of these in turn have switch case for calling the functions in lower utility class.
Although the schema-specific methods are tied together in different utility classes the method call themselves in OracleSpecifics.java do not have any coupling.
I am thinking whether I should change the enum TableName to contain a specific state. The state I am thinking for the schema-specific lowest level utility classes.
These specific states contained in enums can be used to change the state of DAO and the DAO can then call the specific functions based on an interface implemented by all such states. Thus depending on the state the behavior can change automatically.
Is this design decision correct or would it be meaningless?
Any other thing that I might have overlooked?
Any thoughts about the design itself?
Would I lose type safety introduced by generics due to this change?

Best Answer

The first question you need to answer is whether you intend to create a truly generic data access framework, or if you want it tailored to the particular project you're working on.

A generic framework has the benefit of being portable and fairly loosely coupled to the underlying DB + schema, but at the cost of pushing schema level information further up in the framework. The more generic you make the framework, the more of this knowledge you push up.

A tailored framework alleviates the higher levels of the application from worrying about the schema or DB details, but at the cost of cost of being less re-usable in other environments. As a corollary, the more specific you make the framework, the less easy it is to extend elsewhere.

If you're not sure on which of those to pick, the generic framework makes sense if you suspect there will be changes at the DB layer or the schema. Likewise, if those are pretty well locked down then the tailored framework is the better approach. Ultimately, you're optimizing the system to more easily handle where you think the change will occur.

As to whether or not you should refactor - the answer is No unless you're encountering a specific problem.

Worrying about the degree of coupling is nice, I suppose, but doesn't necessarily address a specific problem that you may be seeing. Refactoring based upon presumed issues within coupling is actually more likely to create a problem for you than just leaving it alone.

Likewise with expressing the state. If the current mechanism of an enum is sufficient and doesn't present an apparent problem then don't change what you've got.

Overall, it sounds like you've designed a reasonably robust data access method for the domain you're working worth. Your question doesn't list any specific issues that are holding back your development, so I'd call it good and move to the next stage of development.

Type System Abuse?

Reading the post office example, there is some evidence to suggest that you might be trying to abuse the type system to encode property values in interfaces. And failing...

The way you are currently implementing it, Speed vs. Normal are mutually exclusive, while there is no reason why a class cannot simultaneously be an ExecutivePostageLot and a PublicPostageLot, as far as I can tell.

From your description, however, it appears that you want to categorize postage lots (and postages for that matter) along two dimensions: velocity (speed vs. normal) and importance (executive vs. public).

If so, having an interface for each of the values is not how you should approach it.

Alternative Interpretation

But your question and the diagrams seem to allow another way to interpret what you're trying to do. Maybe I'm imagining things, but I think you want to do the following (I'll use a different example, but it should map 1:1 to your diagram):

You have an Entity class with common methods - e.g. int Id() and DateTime Created().

You have multiple modules, each module defining its own Entity subclasses. So for example, you have a purchasing module (with entities like Order and Product), and a newsletter module (with entities like Post and Subscription).

Now, it seems you want to work with various kinds of sets of a particular subclass of Entity. Some of those sets need to provide only generic functionality that is not specific to a kind of entity. For example, there might be a SortableSet like the following:

class SortableSet<T extends Entity> 
    implements EntitySet<T>
{
    // resorts the set (i know, this makes no sense for a set).
    void Sort(IComparer<T> cmp);

    // ... whatever methods are required by EntitySet ...
    // ... these might also be abstract, of course ...
}

Each of the modules, however, may also need more specific functionality added to our sets. The newsletter module might need to combine all posts in a set to form a new super-post:

// assuming Post has subclasses, otherwise this does not
// need to be generic, of course.
interface CombinablePostSet<T extends Post>
    implements EntitySet<T>
{
    T combineAll();
}

Now we can define a class that combines the required functionality of a module with the generic functionality provided by a general purpose EntitySet, like SortableSet.

class CombinableSortablePostSet<T extends Post>
    extends SortableSet<T>
    implements CombinablePostSet<T>
{
    T combineAll() { ... }
}

If, for some reason, we want to create a class that produces a SortableSet, it should probably not create a set of basic entities, but of a specific entity type. This means we have to let the method know the type of entity that will be in the set. This can be solved with a generic method. (Or with a generic class, but let's go with the method for now).

Of course, in the method body, you can still only treat the elements as Entities, not as Products or Posts, because this method does not know anything about their module-specific functionality.

// this might be a Factory?
class CommonSortableSetLogic
{
    <T extends Entity> SortableSet<T> Processor()
    {
        SortableSet<T> set = null;
        return set;
    }
}

If instead the method returned a SortableSet<Entity> or even an EntitySet<Entity>, we cannot simply cast the returned result to just any subclass of EntitySet with any generic parameter, just because that is the type we need. This is the reason you're getting errors - your CommonMethod1EntitySetLogic methods need to provide the right kind of EntitySet.

But, factories and the like aside, the common logic for a SortableSet<T> should be implemented in the SortableSet<T> class itself. That's why its a class (from which you derive) and not an interface.

Some Notes

Assuming I have understood what you wanted to do, there's still the question of whether you should actually do it like this. We would need to know more about your actual application (what's the purpose of the Entity class - what's special about entity sets that is different from regular sets, etc).

With proper use of generics, operation classes that work on regular sets (like a SetSorter<T>) might lead to less coupling and a clearer structure, if composed well. Anyways, this is a topic for another day maybe.

If you do stick to the design, just remember that A<X> cannot be cast to B<X> or A<Y>, even if B extends A and Y extends X. This kind of thing tends to bite you when not being careful with generics.

Best Answer

Related Solutions

Unit Testing – Should Unit Test Subclasses or Abstract Parent Class in Java?

Java – Designing generic type inheritance hierarchy

Type System Abuse?

Alternative Interpretation

Some Notes

Related Topic