Base generic DAO to work with different POJO classes

Right now, I have base abstract GenericDAO class to execute CRUD operations with different kind of objects. I have Customer and Employee POJO classes which are used in specific DAO classes that extends GenericDAO class.

Questions:

• How correct is the current base GenericDao implementation and SpecificDAO classes which extend GenericDAO?

• Is it a good idea to separate object(i.e. EmployeeSpecs, PersonalSpecs) properties in separate class where some of them are refereced to specific POJO classes? I did it to use search() method easier passing spec object(not by a certain object property) to it.

• How can I implement a search method in the GenericDAOImpl abstract class correctly when the Employee search method requires 2 arguments (EmployeeSpecs and PersonalSpecs) and the Customer search method requires only one argument (PersonalSpecs)?

• How can I implement specs that I can invoke the following getSpec() method inside Customer or Employee classes receiving all specs claases(PersonalSpecs, EmployeeSpecs if we talk about Employee class) applicable for a certain pojo class?

I've read about varargs. Is it possible to use them with generics that I can put search method inside GenericDAO class and can search by multiple object specs?

If you will find that some other places could be optimized or totally improved, please let me know.

NB! If someone knows the anwers on 2 & 3 question, take me know, please.

GenericDAO interface:

public interface GenericDAO {
    public boolean add(T t);
    public boolean update(int index, T t);
    public boolean remove(int index);
    public T getByIndex(int index);
    public List getAll();

    // I want to add search method here if it's possible to search by any multiple object specs
}

GenericDAO implementation:

```
public abstract class GenericDAOImpl implements GenericDAO {

public Set objects = new HashSet();

@Override
public boolean add(T t) {
if

One generic (pun unintended) suggestion: Use braces { } for your if statements to ensure you do not accidentally introduce bugs, because it makes the presentation clearer.

Minor points:

• You can compare enums by ==, e.g. employeeSpecs.getType() == type.

• Casting of obj in your matches() method is potentially unsafe and may/will throw ClassCastExceptions if you accidentally pass in the wrong object type.

Major points:

-
hashCode() computation usually does not simply add the hash codes from multiple fields, especially in your case where a Person with the name and surname John, Doe will have the same hashcode as Doe, John. If you are on Java 7, you can consider Objects.hash(Object... values) instead. In fact, if you look at its implementation, the code goes something like this:

result = 1;
for (Object element : values) {
    result = 31 * result + (element == null ? 0 : element.hashCode());
}
return result;

-
You may want to re-visit the meaning of the boolean return type from your add() and remove() methods in GenericDaoImpl. Using the add() method for example, what happens when the data store objects already contain the incoming object t? Do you still want the method to return true, i.e. t is already present, or return false, because strictly speaking the add() operation isn't successful as objects is not modified at all? In your current implementation, you can consult the Javadoc for Set.add():


Returns:
true if this set did not already contain the specified element

Or more generically speaking, look at Collection.add().


Returns:
true if this collection changed as a result of the call

edit:

One clarification regarding my final point above: I'm quoting the Javadocs from the Collections classes primarily because you are using a Set currently for your "persistence". Even if you are going to replace it with a database back-end and JDBC APIs that do not necessarily return boolean datatypes, it's worthwhile to consider what is the most suitable return value you need from your own APIs. Perhaps you discover that most of your use cases are just to check again whether the input object for add and remove are persisted or not, and therefore the original implementation will still make sense (you'll want to document it properly too).

Looking at your new changes...

public boolean remove(int index);
public boolean remove(T t);

These two implementations are pretty much the same, you can consider passing getByIndex(index) within the remove(int index) method to the remove(T t) method to reduce code duplication. :)

I also don't think your getByIndex() implementation is doing what you think... ultimately, there is still one instance of your persisted objects in objects. If you are thinking of returning immutable copies of your Customer or Employee objects, then you actually have a bit more work to do.

I'm not sure what your two new updateSpecs methods are doing in your EmployeeDAO class, care to explain? Actually, it'll be better if you can submit a new question for further significant changes, so that the history and answers for this question can be largely consistent...

Finally, onto your original question about searching for different generic types right within your GenericDAOImpl class: I don't think that's possible, and this is where I hope other Java experts can offer their advice or solution on this...

StackExchange Code Review Q#57407, answer score: 4