Base generic DAO optimizations

I have base abstract GenericDAO class that executes CRUD operations with different kind of objects. Customer and Employee POJO classes are used in specific DAO classes that extends GenericDAO class. From my previous related question I have some unresolved problems:

Questions:

• How to realize immutable objects in GenericDAOImpl?

• Is it a good idea or not 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 should I deal with search() method? I should put it inside specific DAO which is extended from GenericDAO as it's now or I can generify search() method in GenericDAO somehow? What will be the best way in my case?

• How can I implement getSpec() method inside Customer or Employee classes which will take the opportunity to receive all specs(PersonalSpecs, EmployeeSpecs if we talk about Employee class) applicable for a certain pojo class?

NB! If you see that some piece of code could be optimized, please take me know.

GenericDAO interface:

public interface GenericDAO {
    boolean add(T t);
    boolean update(int index, T t);
    boolean remove(T t);
    T getByIndex(int index);
    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 {

private static final Logger Logger = LoggerFactory.getLogger(GenericDAOImpl.class);

private Set objects = new HashSet();

@Override
public boolean add(T t) {
if (t != null && !objects.contains(t)) {
if (objects.add(t)) {
Logger.info("Added new " + t.toString());
return true;
}
}

return false;
}

@Override
public boolean update(int

• How to realize immutable objects in GenericDAOImpl?

By creating copies of the objects you get in, rather than keeping references to the objects. With generics this is may be tricky but if T extends Cloneable you could at least call clone() on it to create a field for field copy. This will not get you the control you would have if you knew which types you use but it is something at least.
As output (when asked for a property) you should again make a copy, so that noone can alter your internal state. basic rule of thumb: don't trust input you get to be unmutated, don't trust output you give to stay unmutated.

Your employee DAO maintains an internal set of objects (which is fine for an in-memory version of a dao I suppose. though it's not "accessing" anything but it's internal map/list/set. ) However you seem to be mixing this up in your search. On a dao, I would expect a search to return a list of objects without changing it's internal state.

However in your case, if I do: (excuse the end of line comments)

EmployeeDao dao = (get from somewhere);
dao.add(someEmployee);
dao.add(otherEmployee);
dao.getAll(); // returns someEmployee, otherEmployee;

// assume spec matches only otherEmployee
// search returns someEmployee, otherEmployee
dao.search(specX, specY);  // returns someEmployee, otherEmployee

I might be misunderstanding the above code ofcourse, but I think your search should do:

public List search(PersonalSpecs personalSpecs, EmployeeSpecs employeeSpecs, boolean nameComparable) {
    List result = new ArrayList();

    if (personalSpecs == null && employeeSpecs == null) {
        return result;
    }

    // there is actually something to search on.
    for (Employee employee : getAll()) {
        if (employee.getPersonalSpecs().matches(personalSpecs, nameComparable) || (!nameComparable && employee.getEmployeeSpecs().matches(employeeSpecs))) {
            result.add(employee);
        }
    }

    return result;
}

ofcourse in combination with clone : result.add(employee.clone())

• Is it a good idea or not 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.

I think you are better off keeping your properties divided by how your domain is organized rather than how a search happens to need them grouped. This might be the same at the moment but it may change. What if you have 3 more types of searches that work on overlapping objects. However you could have the object return a representation of itself if you want to do it that way. This will allow an object to have all sorts of properties organized in a number of ways, but when you need a spec version of a set of attributes it can create this.

// in Employee: 
public final EmployeeSpec getEmployeeSpec() {
    EmployeeSpec spec = new EmployeeSpec(this.xx, this.yy);
}

This still limits you to some degree. (it ties you to the spec, you have to add it to every thing you would like to search for). I'm getting a little out of my depth here, but you could have a "Spec" class that does something like:

// could call it specmatcher but I didn't want to make it look like it was still using spec.
public interface Matcher {
     public boolean matches(T match);  
}

// ofcourse implement this as you wish, in your case with strict/nonstrict.
public static class EmployeeMatcher extends Matcher {
     private EmployeeType type;
     public boolean matches(Employee match) {
          if (type.equals(match.getType()) {
               return true;
          }
     }
}

You would need to alter your search method to do:

public List search(Matcher matcher) {
    List result = new ArrayList();
    for (Employee employee : getAll()) {
        if (matcher.matches(employee) {
            result.add(employee);
        }
    }
    return result;
}

• How should I deal with search() method? I should put it inside specific DAO which is extended from GenericDAO as it's now or I can generify search() method in GenericDAO somehow? What will be the best way in my case?

The below code should do that (replace Employee with T)

public List search(Matcher matcher) {
    List result = new ArrayList();
    for (T t : getAll()) {
        if (matcher.matches(t) {
            result.add(t);
        }
    }
    return result;
}

• How can I implement getSpec() method inside Customer or Employee classes which will take the opportunity to receive all specs(PersonalSpecs, EmployeeSpecs if we talk about Employee class) applicable for a certain pojo class?

I would drop specs altogether and use matcher. or keep specs at least only as ways to feed matchers.

Again (mentioned it earlier) part of this is an educated guess. I'm hoping I've understood your question.

Pointers (and please if anyone disagrees, leave me a comment):

-
search and similar lookup me

StackExchange Code Review Q#57755, answer score: 2