HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Returning a database connection

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
databasereturningconnection

Problem

I've written this MySQL database class for my Java application and I'd just like some feedback on it before moving onto making my next class.

The idea here is that I have a simple properties file which stores the hostname, username, password, and database name. I then connect to the database (if possible) and return the Connection.

Although I am not really sure what should happen if the properties file doesn't exist, or the MySQL connection fails, should I return Connection which would be null, throw a new exception, or re-throw the existing exception?

```
package app;

import java.io.IOException;
import java.io.InputStream;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.util.Properties;

public class Database {

private String hostname;
private String database;
private String username;
private String password;
private Connection conn;

public Connection getConnection() {

if(conn == null) {

try {

System.out.println("Getting properties file...");
Properties prop = new Properties();
InputStream input = Database.class.getResourceAsStream("database.properties");
prop.load(input);
this.hostname = prop.getProperty("hostname");
this.database = prop.getProperty("database");
this.username = prop.getProperty("username");
this.password = prop.getProperty("password");
input.close();

} catch (IOException ex) {

// Error Reading Properties File
ex.printStackTrace();

} finally {

if (this.database != null) {

try {
System.out.println("Connecting to database...");
Class.forName("com.mysql.jdbc.Driver");
conn = DriverManager.getConnection("jdbc:mysql://" + this.hostname +

Solution

The original solution from the question, as well as the one suggested by @oopexpert seem too complex and over-engineered.

I guess that the author of the question is trying to implement just a simple wrapper around DriverManager.getConnection(), which reads connection parameters from an expected location.

Some remarks about the original design choices:

-
getConnection() should return a new Connectioneach time. The huge drawback of the original solution is that the returned instance represents a sort of singleton. What happens if this instance is closed somewhere by the caller?

-
I don't suggest to use connection pooling here, e.g. Hikari - let this topic just remain out of scope of this review.

-
There is not enough reason to use stateful objects for the functionality: we need just to provide some additional expected values to the factory method and can remain in static context.

-
The exceptions should be re-thrown, except the I/O that might occur when the expected properties cannot be loaded: this is critical and blocking for the functionality and means a completely unwanted state of the application.

-
Since JDBC4, there is no need to call Class.forName to load the driver. THe driver only needs to be on the classpath.

How it could look like after refactoring:

public class Database {

  private static final String PROPS_FILE_NAME = "database.properties";
  // optionally extract prop keys in constants 

  public static Connection getConnection() throws SQLException {
    Properties properties = readProperties();
    String url = String.format("jdbc:mysql://%1$s/%2$s",
                               properties.getProperty("hostname"),
                               properties.get("database"));
    return DriverManager.getConnection(url, 
                                       properties.getProperty("username"), 
                                       properties.getProperty("password"));
  }

  private static Properties readProperties() {
    try (InputStream input = Database.class.getResourceAsStream(PROPS_FILE_NAME)) {
      Properties props = new Properties();
      props.load(input);
      return props;
    } catch (IOException ex) {
      throw new IllegalStateException(ex);
    }
  }

}

Code Snippets

public class Database {

  private static final String PROPS_FILE_NAME = "database.properties";
  // optionally extract prop keys in constants 

  public static Connection getConnection() throws SQLException {
    Properties properties = readProperties();
    String url = String.format("jdbc:mysql://%1$s/%2$s",
                               properties.getProperty("hostname"),
                               properties.get("database"));
    return DriverManager.getConnection(url, 
                                       properties.getProperty("username"), 
                                       properties.getProperty("password"));
  }

  private static Properties readProperties() {
    try (InputStream input = Database.class.getResourceAsStream(PROPS_FILE_NAME)) {
      Properties props = new Properties();
      props.load(input);
      return props;
    } catch (IOException ex) {
      throw new IllegalStateException(ex);
    }
  }

}

Context

StackExchange Code Review Q#155269, answer score: 2

Revisions (0)

No revisions yet.