I might be missing the point here of the factory and implementing incorrectly?
Factories are an abstraction for how to obtain instances, as distinct from explicitly pooling, singletonning, or creating instances (through constructors) from client code. Exactly how far into this abstraction you want to go is a matter of debate and, in some cases, ridicule, but this base abstraction is what you need to keep in mind when designing them.
Because factories are a means to an end, it's hard to tell whether your factory will be helpful without seeing your use case. What will your use of the pattern help you accomplish? (As a hunch, I'd recommend you to look into dependency injection.)
Is a singleton necessary with my implementation within the factory and more specifically with each DAO?
No. A singleton is never really necessary anywhere(1) and it's usually a convenience hack to prevent having to pass a reference into a method or store it in context; a bypass of sorts. That's not to say that singletons serve no useful purpose, but that the decision to use a singleton should be an implementation choice, and never a design choice.
(1) It can even be downright hazardous in server environments with managed containers.
That's for the abstract part; let's get to concrete suggestions.
#Pull singletons up to the factory level#
The concern of the DAO implementations is data storage and retrieval. The concern of the factory is how to supply instances. Lifting the singletons from the individual implementations leaves the factory in control of instantiation and life cycle.
Here's a straightforward example of such an approach:
// I like that boilerplate. That's some nice boilerplate.
public class SimpleStoringFactory extends DAOFactory {
private final GenericDAO<Trade, String, Boolean> tradeDAO;
private final GenericDAO<User, String, Boolean> userDAO;
private final GenericDAO<User, String, Boolean> logDAO;
private final GenericDAO<ServiceRequest, String, Boolean> requestDAO;
public SimpleStoringFactory(
GenericDAO<Trade, String, Boolean> tradeDAO,
GenericDAO<User, String, Boolean> userDAO,
GenericDAO<User, String, Boolean> logDAO,
GenericDAO<ServiceRequest, String, Boolean> requestDAO) {
this.tradeDAO = tradeDAO;
this.userDAO = userDAO;
this.logDAO = logDAO;
this.requestDAO = requestDAO;
}
public GenericDAO<Trade, String, Boolean> getTradeDAO() { return tradeDAO; }
public GenericDAO<User, String, Boolean> getUserDAO() { return userDAO; }
public GenericDAO<User, String, Boolean> getLogDAO() { return logDAO; }
public GenericDAO<ServiceRequest, String, Boolean> getRequestDAO() { return requestDAO; }
}
#GenericDAO
#
public boolean update(U user, B active) throws NullUserException,
UserExistException, NullListException, UserInstanceException,
ReceivedNullParameterException;
That's... quite something. Only list exceptions that you expect clients to handle/catch, and see whether built-in, available exception types like IllegalArgumentException
suffice.
If this is not feasible, document when exceptions are expected to be thrown, and when the return value is true or false. I could not make this out from the method signature. Compare with:
/**
* Finds the record identified by <var>user</var> and updates it to the given values.
* Returns true if changes were made to the record, and false if the record was found,
* but all current values matched given values (i.e. no update occurred).
*
* @return {@code true} iff changes were made to the record.
* @throws NotFoundException if no record for this user was found.
*/
public boolean update(U user, B active) throws NotFoundException;
#General Remarks#
The currentUsedStorage
field in DAOFactory
looks out of place. What is its purpose? Consider deleting it. In that same vein, setCurrentUsedStorage
is not declared static.
Auto-generated getter and setter comments are not useful as documentation: they simply repeat the name of the function. Remove these to reclaim some vertical space.