CODEWITHSUNDEEP
javasolid-principles
user3181034
user3181034

Reputation: 189

Single Responsibility Principle - my class

Does this class violate Single Responsibility Principle? It has more responsibilities, but all of them communicate with database, or should I create one class for each method?

public class DBLoader {

Connection connection;
ArrayList<Book> list;

public DBLoader(String url, String userName, String password) {

        connection = DriverManager.getConnection(url, userName, password);
        list = new ArrayList();

}

public ArrayList getBooks() {
//select * from books
}

public boolean registerBook(String isbn, String title) {

    PreparedStatement preparedStatement = connection
    .prepareStatement("insert into book(isbn, title) values (?, ?)");
}

public boolean updateBook(String title, String isbn) {

    PreparedStatement ps;
    try {
        ps = connection
                .prepareStatement("UPDATE book SET title = ? WHERE isbn = ?");
}

Upvotes: 2

Views: 495

Answers (3)

Nathan Hughes
Nathan Hughes

Reputation: 96454

With the posted code, what would you do if you had a requirement to access or save different entities within the same transaction? And what if separate users want to do something with the same entity? One would have to wait for the other to get done with the connection, which would be bad. This is an example of responsibilities having gotten entangled, when you try to extend a design to do something unanticipated and either you have to write a lot of extra code to patch over things or you get adverse consequences.

There are at least 3-4 different responsibilities here:

1) Getting a database connection

2) Updating and inserting books into the database

3) Keeping a list of books, hard to tell why, maybe it's for caching

4) Transaction control (if the connection the code can use is limited to this instance then that limits the ability of this code to manage transactions)

These are all things that in a typical Java EE application get handled in separate places and which vary for different reasons, they shouldn't be lumped into the same class.

Typically there's a Java EE pattern called Data Access Object (DAO) where #2 functionality gets implemented. The DAO shouldn't know about how connections are acquired and it should be able to be reused in different transactions, so it shouldn't have transactionality coded into it. DAOs are typically stateless and don't know anything about user-specific state.

Connection-retrieving is handled with a connection pool and some servlet filter or interceptor that stashes the connection (or possibly a stack of connections for the case where there are nested transactions) in a threadlocal so that it's available to any code that needs it, regardless of what entity it's for. Connections and transactions should be able to be used by many entities, it doesn't make sense to dedicate a connection to each type of entity.

Transactions are usually handled with a service layer or stateless session bean that contains multiple DAOs. The service methods are annotated to declare where the transaction boundaries are, which the framework uses to create proxies that wrap the service, which find the connection in the threadlocal and start or commit a transaction on it on the way in or out. This way the different responsibilities stay separated from each other.

I'd suggest learning more about how mature Java frameworks handle this kind of thing, and look up Java EE design patterns.

Upvotes: 0

Lajos Arpad
Lajos Arpad

Reputation: 77118

Your class's purpose is to load the database, as suggested by the name of DBLoader. So it should be responsible for loading the database and keeping track of connection, maybe to run queries received. On this level, we should not talk about concepts like books and stuff. We should talk about connections, commands, queries, responses and errors. This is your connection layer. A higher layer should be the data access layer, where you would define classes, such as Book, which would serve as models for database tables. A further higher layer should be the business layer, where you would define classes which would do more complex operations with your models. So yes, you should separate the concept of book from the concept of database loader into separate classes.

Upvotes: 0

Matt
Matt

Reputation: 311

You've got two ideas in this class: "create a connection to the database," and "persist books to the database."

You might consider splitting those apart, and giving the book class a way to retrieve an active db connection.

Generally, if you need to use a conjunction like "and" to describe the responsibility of a class, it's a smell that it might be more than one class.

Upvotes: 3

Related Questions