Generic DAO pattern implementation Design

Question

I am working on a GWT+Hibernate project. It consists of various module out of which I will name two - Company and Login. For each module, I have created an RPC service so that the project doesn't end up with one God-like service that does everything.

In order to interact with database, I use Hibernate APIs - to be specific, EntityManager and Annotations in POJOs.

Also, I have created a Generic DAO class that will handle the basic CRUD operations. GenericDAO class will also handle EntityManager. Each module service class will extend this GenericDAO so that it can add its own query methods.

The following is the stub of GenericDAO class -

public class GenericDataAccessService<EntityType, PrimaryKeyType extends Serializable> {

    // Constructor

    @Override
    public void save(EntityType newEntity) {
        // TODO Auto-generated method stub
    }

    @Override
    public void update(EntityType entity) {
        // TODO Auto-generated method stub

    }

    @Override
    public EntityType find(PrimaryKeyType primaryKey) {
        // TODO Auto-generated method stub
        return null;
    }

    @Override
    public List<EntityType> findByProperty(String property) {
        // TODO Auto-generated method stub
        return null;
    }

    @Override
    public List<EntityType> findAll() {
        // TODO Auto-generated method stub
        return null;
    }

    @Override
    public void delete(PrimaryKeyType primaryKey) {
        // TODO Auto-generated method stub

    }

    @Override
    public void delete(EntityType entity) {
        // TODO Auto-generated method stub

    }
}

Now, say I want one more find method for Company module. So, I write my CompanyService class thus -

public class CompanyService extends GenericDataAccessService<Company, int> {

    public void addCompany(Company company) {
        super.save(company);
    }

    public Company updateCompany(Company company) {
        return super.update(company);
    }

    public List<Company> findMatchingCompanies(String companyName) {
        return super.findByProperty(companyName);
    }

    public void deleteCompany (int companyId) {
        super.delete(companyId);
    }

    public List<Company> findThroughSomeCustomSearch() {
        // custom query code.
    }
}

Other modules follow similar footsteps. That way, I can also add non-data access related methods to each module's services, too.

Now, I need to expose these modules on client side. I choose not to expose GenericDAO class in any way; so no interface for it. Instead, I create an interface per module.

So, for CompanyService, it is -

public interface CompanyService {

    public void addCompany(Company company);

    public Company updateCompany(Company company);

    public List<Company> findMatchingCompanies(String companyName);

    public void deleteCompany (int companyId);

    public List<Company> findThroughSomeCustomSearch();
}

Other modules' interfaces goes the same way.

Is this a good design? GenericDAO does save some boilerplate code of session management and basic CRUD operations. However, that has already reduced to 3-4 lines of code per method, due to Hibernate APIs. I don't find any other use of GenericDAO in this case. Or am I implementing it a wrong way?

Please suggest better ways than this, in case this design is not good enough.

EDIT: I would like to know what names you would give to Service Modules in this case. I am right now using "Impl" suffix, but it feels like it's stuck in my throat. For example, for Company module, Interface - CompanyService Class - CompanyServiceImpl

A better suggestion?

Answer 1

I think there are a couple of things you could do to improve this:

Coupling:

Most of the times composition is preferable to inheritance. Your services are strongly coupled because of that kind of relation. I purpose change it, use composition instead as follow:

public class CompanyService  {
    private GenericDataAccessService<Company, int> dao; // interface 

    public void addCompany(Company company) {
        dao.save(company);
    }
    ....
}

In order to break the dependency, the dao field should be an abstract class or interface (i don´t know neither your code nor your needs nor java) to allow you inject it.

Testeability:

Injecting these classes' dependencies will also helps you to test your code easily injecting them mock instances.

Unwanted operations:

Using inheritance force you to have operations that probably you don´t want to allow. For example, just imagine you have an AuditingDataService as follow:

public class AuditingService extends GenericDataAccessService<Audit, int> 

If you do that, yout AuditingService will inherit a Delete method!!! Can you feel that smell as strong as I do? It will drives you to the following point.

Anti-pattern

In the previous example (delete method for auditing log entries) you will be forced to override it with a do-nothing or throwing-exception method to prevent someone use it... ummmm.... ummm... Can that be a good design?

Conclusion:

I think it is not, simply because inheritance is not well suited here.

Forget for a moment about LoCs and focused on changing the design.

Update:

Java has its own naming conventions and if well Impl is an awful suffix, it is one of those conventions that every java programmer understands and shares. So, I think it is okay.