I'm learning about SRP and had to reduce the responsibility of a customer object, which contained too much info. So I turned that into just a Pojo and pulled out the database logic. I'm trying to find the best design for this setup and am split between making the DatabaseManager class be a parent class, or a singleton that can return the single connection object. This system will be used to insert and delete customers from the database. I don't want my domain objects/DAOs to worry about the connection details. Is the design I currently have a strong follower of OOP design principles?
public class DatabaseManager {
private Connection conn;
private static DatabaseManager managerInstance = new DatabaseManager();
private DatabaseManager() {
}
public static DatabaseManager getInstance() {
return managerInstance;
}
/**
* contains connection details
*
* @throws SQLException
*/
public void connect() throws SQLException {
System.out.println("Established Database Connection...");
conn = DriverManager.getConnection("Some/Database/URL");
}
public Connection getConnectionObject() {
return conn;
}
public void disconnect() throws SQLException {
conn.close();
System.out.println("Disconnected from Database...");
}
}
Here is the Customer Object:
public class Customer {
private int id;
private String name;
private boolean active;
public Customer(int id, String name, String department, boolean working) {
super();
this.id = id;
this.name = name;
this.department = department;
this.working = working;
}
@Override
public String toString() {
return "Customer [id=" + id + ", name=" + name + ", department="
+ department + ", working=" + working + "]";
}
}
The customer DAO:
public class CustomerDAO {
public CustomerDAO() {
}
public void addCustomer(Customer Customer) throws SQLException {
DatabaseManager.getInstance().getConnectionObject().prepareStatement("some sql... ");
}
public void removeCustomer(Customer Customer) throws SQLException {
DatabaseManager.getInstance().getConnectionObject().prepareStatement("some sql... ");
// implementation details avoided
}
}
1 Answer 1
Single responsibility principle
In terms of SRP, the current code is good, each class is has one clear purpose:
- Customer: only the customer details, nothing else
- CustomerDAO: only data accessor methods related to customers
- DatabaseManager: manage database connection,
DatabaseConnectionManager
might be a better name
But they can be improved.
Customer
If you can make its fields final, make them final.
No need to call super()
in a class whose parent is Object
. You can just delete that line.
CustomerDAO
The throws SQLException
declarations make this class database specific. In theory, a CustomerDAO
could work over a web service or a CSV file. Actually CustomerDAO
should be an interface, to model a backend-agnostic concept, with throws
declarations that are backend-agnostic. Then an implementation using a database backend should wrap the SQLExceptions and throw the backend-agnostic exceptions instead. In such design you will have the flexibility to replace the DAO with a different backend.
Also, the public empty constructor is pointless.
DatabaseManager
The modern pattern of implementing singletons is this:
public enum DatabaseManager {
INSTANCE;
// ...
}
This is truly thread-safe. Use this instead of a private static
field, which is not truly thread-safe.
Use a framework
Instead of managing database connections yourself, I strongly recommend using a framework that can greatly simply it for you. For example Spring Framework.
-
\$\begingroup\$ Thank you for the greatt explanation! Im learning oop concepts as well as common design patterns hence i wanted to build this thing myself rather than depend on a framework. But do u think it would be considered over engineering if i created an interface for each one of my domain object daos? Can i define a single dao that is database specific and have my domain daos implement that? What would you recommend? \$\endgroup\$GrindMan– GrindMan2014年08月22日 11:13:21 +00:00Commented Aug 22, 2014 at 11:13
-
\$\begingroup\$ I don't think it will be overengineering, it will be a good thing to do to create an interface for each domain dao. To avoid repetition in your database operations, you can either have a common parent class, or a utility class. \$\endgroup\$janos– janos2014年08月22日 12:46:43 +00:00Commented Aug 22, 2014 at 12:46
-
\$\begingroup\$ Hmmm, how would I make this enum work for my code? \$\endgroup\$GrindMan– GrindMan2014年08月22日 12:47:40 +00:00Commented Aug 22, 2014 at 12:47
-
\$\begingroup\$ Replace all occurrences of
DatabaseManager.getInstance()
withDatabaseManager.INSTANCE
\$\endgroup\$janos– janos2014年08月22日 13:25:13 +00:00Commented Aug 22, 2014 at 13:25
Explore related questions
See similar questions with these tags.
// implementation details avoided
is a sign that your question doesn't really fit here, just like your previous question... \$\endgroup\$