0
\$\begingroup\$

I have my class DBFuntcions containing this 2 methods:

The method for the connection to the database:

Connection connection_db(String username, String password, String dbname, Connection c) {
    try {
        Class.forName("org.postgresql.Driver");
        c = DriverManager.getConnection("jdbc:postgresql://localhost:5432/"
                + dbname, username, password);
        System.out.println("Opened database successfully");
    } catch (ClassNotFoundException | SQLException e) {
        System.err.println(e.getClass().getName() + ": " + e.getMessage() + "\n\n\n");
    } 
    return c;
}

And the method for the query.

void viewAll(String tableName, long startDate, long endDate, Connection c, String path) {
    String query = "SELECT * FROM " + tableName + " WHERE CAST(receivedtime AS integer) BETWEEN ? AND ? ORDER BY source, receivedtime;";

    try (PreparedStatement stmt = c.prepareStatement(query)) {              //try with resources
        stmt.setLong(1, startDate);                                         //prepare query
        stmt.setLong(2, endDate);
        ResultSet rs = stmt.executeQuery();

        try {
            while (rs.next()) { ... }

My question it is the best to open and close the connection?

Just before calling the query and closing right after? Opening and closing the connection inside the query method? Or it is better to open it in the main and close it at the end?

I think it should depends of the number of time I try to access to it. Let's say for the moment I only have this query and run it only once.

\$\endgroup\$
  • \$\begingroup\$ When to open and close a database connection does depend on the number of times you create, read, update, or delete rows. If you have one query executed once, it doesn't matter when you open and close the database connection. If it's all your code, like a desktop Java Swing application, then you should open the database connection once before you start the application and close the database connection once before you end the application. If your code is running in a shared environment like a web site, you should open and close the database connection for each database access. \$\endgroup\$ – Gilbert Le Blanc Feb 12 '16 at 20:22
  • \$\begingroup\$ I fear that when "it is the best to open and close the connection" is really too broad because it highly depends on your requirements. You could have a connection pool for example also. \$\endgroup\$ – Tunaki Feb 12 '16 at 20:56
  • \$\begingroup\$ Assuming I only use it once for the moment. I call the createDb in my main method. Should I change this call to the method viewAll? I once been told that for security reason we should open and close the connection in each method who need it. \$\endgroup\$ – Charly Roch Feb 15 '16 at 10:46
1
\$\begingroup\$

Inconsistent method naming

Connection connection_db(String username, String password, String dbname, Connection c) {
void viewAll(String tableName, long startDate, long endDate, Connection c, String path) {

These methods have both different naming schemes, the top one uses all_lowercase_with_underscores, while the button user pascalCase. The latter is recommend in Java.

URL Injection using dbname

 c = DriverManager.getConnection("jdbc:postgresql://localhost:5432/"
            + dbname, username, password);

You never check for special characters in the dbname. This can cause databases containing special characters to mess up your function, especially when the database can be edited though config. Escape it using URLEncoder.encode

\$\endgroup\$
  • \$\begingroup\$ Thanks I forgot to change the name of this method. I will check for URL Injection. But the user is the database administrator so I don't think he will try to do this. Anyway it is always good to enforce security. \$\endgroup\$ – Charly Roch Feb 15 '16 at 10:45
0
\$\begingroup\$

Usually the best way to do this is with a DataSource.

void viewAll(DataSource source, String tableName, long startDate, long endDate, String path) {
    String query = "...";
    try (Connection connection = source.getConnection()) {
        try (PreparedStatement statement = connection.prepareStatement(query)) {
            ...

This has the advantage of using Java's native resource management and being able to pool the prepared statement. Also, the DataSource should keep track of the username, password and other database information.

For PostgreSQL I believe the DataSource implementation is a PGConnectionPoolDataSource.

\$\endgroup\$

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.