0
\$\begingroup\$

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
    }

}
\$\endgroup\$
  • 1
    \$\begingroup\$ For more info: programmers.stackexchange.com/search?q=dao+design (that's the Programmers.SE I'm talking about) \$\endgroup\$ – h.j.k. Aug 22 '14 at 1:59
  • \$\begingroup\$ Sorry if I'm coming off as too curt, but comments like // implementation details avoided is a sign that your question doesn't really fit here, just like your previous question... \$\endgroup\$ – h.j.k. Aug 22 '14 at 2:00
  • 1
    \$\begingroup\$ It would be best to include implementation details. \$\endgroup\$ – RubberDuck Aug 22 '14 at 2:21
  • \$\begingroup\$ That just removes the customer from the database. No need to get into those details. Im talking about oop design principles. Thats it. Does this design overall follow, OCP, DRP, ISP, and SRP? \$\endgroup\$ – GrindMan Aug 22 '14 at 3:06
2
\$\begingroup\$

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.

\$\endgroup\$
  • \$\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 Aug 22 '14 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 Aug 22 '14 at 12:46
  • \$\begingroup\$ Hmmm, how would I make this enum work for my code? \$\endgroup\$ – GrindMan Aug 22 '14 at 12:47
  • \$\begingroup\$ Replace all occurrences of DatabaseManager.getInstance() with DatabaseManager.INSTANCE \$\endgroup\$ – janos Aug 22 '14 at 13:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.