Java: Static Database Connection

Ziycon

I'm looking to create a static database connection from when the application opens and then the connection is killed when the application closes which is working grand.

My question is how can I keep the connection static within the database class?

When I call checkPassword() from a login class it always returns false due to there being no valid connection to the database(conn) even though the database connection was opened in the static main.

Any help greatly appreciated.

public class database {	
	Server hsqlServer = null;
	static Connection conn = null;
	ResultSet rs = null;
	Statement stmt = null;
	String query = null;
	
	public void startDatabase() {
		// Start database process
		hsqlServer = new Server();
		
		hsqlServer.setLogWriter(null);
		hsqlServer.setSilent(true);
		
		hsqlServer.setDatabaseName(0, "testdb");
		hsqlServer.setDatabasePath(0, "file:testdb");
		
		hsqlServer.start();
	}
	
	public void stopDatabase() {
		// Stop database process
		try {
			hsqlServer.stop();
		}
		catch(Exception e) {
			JOptionPane.showMessageDialog(null, config.DATABASE_ERROR);
			System.exit(0);
		}
	}
	
	public void connOpen() {
		try {
			Class.forName("org.hsqldb.jdbcDriver");
			
			conn = DriverManager.getConnection("jdbc:hsqldb:hsql://localhost/testdb","SA","");
		}
		catch(Exception e) {
			// error opening database connection
		}
	}

	public void connClose() {
		try {
			conn.close();
		}
		catch(Exception e) {
			// error closing database connection
		}
	}

	public boolean checkPassword(String username,char[] password) {
		boolean validPassword = false;
		try {
			query = "SELECT id,username,password FROM users";
			stmt = conn.createStatement();
			rs = stmt.executeQuery(query);
			
			rs.next();
			System.out.println("ID: " + rs.getInt(1) + "\n");
		}
		catch(SQLException e) {
			// error checking users password
		}
		
		return validPassword;
	}
}

CreepingDeath

checkPassword always returns false because you set the variable "validPassword" to false at the start of the method and you never change it under any condition.

Also I'd remove "rs", "stmt" and "query" as class variables and only declare them when needed.
Note that you really need to explicitly close resultsets and statements also, preferrably in a "finally" block eg.

public boolean checkPassword(String username, String password)
    {
        boolean validPassword = false;
        
        Statement stmt=null;
        ResultSet rs=null;
        
        try
        {
            String query = "SELECT id, username, password FROM users where username='" + username + "' and password = '" + password + "'";
            stmt = conn.createStatement();
            rs = stmt.executeQuery(query);

            if (rs.next())
            {
                // One (or more) results found
                validPassword=true;
                System.out.println("ID: " + rs.getInt(1) + "\n");
            }
            else
            {
                System.out.println("User and password combination not found");
            }            

        }
        catch (SQLException e)
        {
            // error checking users password
        }
        finally
        {
            // Must close the resultset and statement to avoid database resource leaks
            try
            {
                if (rs!=null)
                {
                    rs.close();
                    rs=null;
                }
                
                if (stmt!=null)
                {
                    stmt.close();
                    stmt=null;
                }
            }
            catch (SQLException cleanUpProblem)
            {
                System.out.println("Problem closing resultset & statement");
            }            
        }

        return validPassword;
    }

regards,
CD

Ziycon

Thanks for the feedback CreepingDeath. I'm aware that the value for checkPassword() is always false but my issue is how to keep a static connection open in this class as when I call checkPassword() the error that is caught and output to the console is:

connection exception: connection does no exist

I'm using the below code for the function:

public boolean checkPassword(String username,char[] password) {
	boolean validPassword = false;
	try {
		query = "SELECT id,username,password FROM users";
		stmt = conn.createStatement();
		rs = stmt.executeQuery(query);
		
		rs.next();
		System.out.println("ID: " + rs.getInt(1) + "\n");
	}
	catch(SQLException e) {
		System.out.println(e.getMessage());
	}
	
	return validPassword;
}

GreeBo

why do you want to keep it open?

Its going to time out on one or both sides if its not being used or unless you are keeping it alive.
Would a connection pool not be a better, more standard way of doing this?

gaelicyoda

Just a few comments here:

(1) Your class is not correctly named. It should be Database, not database.

(2) The Connection object is static, and you're assigning it from a non-static method.
Not good practice - what if you had 10 instances of this class running?

(3) Make all the methods in the Database class static... I don't see why they need to be non-static.

(4) The checkPassword() method does not do anything meaningful right now - the arguments are not used (the way the SQL is written, "username" and "password" are column names in the "users" table). I should probably assume that you planned on updating this?

(5) If you really wanted to go this route (without using pools, which would probably be a better option), then what I would recommend is storing the connection as a variable in the calling class instead, and passing the Connection object in as an argument to the checkPassword() method, e.g.

public class Database {
    ...
    public static Connection newConnection() {
        try {
            Class.forName("org.hsqldb.jdbcDriver");            
            return DriverManager.getConnection("jdbc:hsqldb:hsql://localhost/testdb","SA","");
        } catch(Exception e) {
            // TODO: Handle properly ...
            e.printStackTrace();
        }
    }

    ...

    public static boolean checkPassword( Connection conn, String username, char[] password) {
        ...
    }    
}

public class CallerClass {
    ...
    private Connection conn = null;
    ...

    public CallerClass() {
        conn = Database.newConnection();
    }

    public void someMethod() {
        ...
        Database.checkPassword( conn, [I]<username>[/I], [I]<password>[/I] );
    }
}

... you could probably close the connection in a finalize() method.

I still don't like this though - in this example, you should really create and close the connection within the someMethod() class.

GreeBo

gaelicyoda wrote: »

... you could probably close the connection in a finalize() method.

I still don't like this though - in this example, you should really create and close the connection within the someMethod() class.

You have no guarantee of when it will be called or even of it being called, also if your finalize method happens to throw an exception it wont go anywhere, you wont even get a stacktrace... so, close it when you are done using it.

Again OP, why do you specifically want a static connection? (What I would consider the connection that is not closed between uses)
Or do you mean a connection object that uses the static keyword?
i.e. there is only one instance of it?

gaelicyoda

GreeBo wrote: »

You have no guarantee of when it will be called or even of it being called, also if your finalize method happens to throw an exception it wont go anywhere, you wont even get a stacktrace... so, close it when you are done using it.

Yup, that's why I don't like it

OP - unless there is a compelling reason not to do it, then the method that requires the DB access should create the Connection object, use it, and then close it when it's finished with it.

ChRoMe

gaelicyoda wrote: »

OP - unless there is a compelling reason not to do it, then the method that requires the DB access should create the Connection object, use it, and then close it when it's finished with it.

There is rarely, if ever, a good reason not to close it.

gaelicyoda

ChRoMe wrote: »

There is rarely, if ever, a good reason not to close it.

Agree totally, but I was more talking about the whole life cycle of the Connection object within the method (rather than talking about only the closing aspect of it). Really just making the point that, in general, static or indefinitely open connections only lead to unnecessary complications down the road in my experience.

CreepingDeath

gaelicyoda wrote: »

OP - unless there is a compelling reason not to do it, then the method that requires the DB access should create the Connection object, use it, and then close it when it's finished with it.

Opening a database connection is an expensive operation timewise.
You should open a connection/finite connection pool, and minimise the number of connection open/closes.

Ziycon wrote: »

... even though the database connection was opened in the static main.

I don't quite understand the problem here, or maybe that's because you haven't shown us your main() method.

You could make the connOpen() and connClose() methods private and only call them in startDatabase() and stopDatabase().
So the connection is automatically opened and closed when the server starts & stops.

Also, you've no error handling in connOpen(), the catch block is empty.
So are you sure that it opened the connection at all?
At least put an e.printStackTrace(); in your catch blocks.

I'd bet that you haven't registered HSQLDB with the driver manager.
eg. adding -Djdbc.drivers=org.hsqldb.jdbcDriver on the command line.

public void connOpen() {
		try {
			Class.forName("org.hsqldb.jdbcDriver");
			
			conn = DriverManager.getConnection("jdbc:hsqldb:hsql://localhost/testdb","SA","");
		}
		catch(Exception e) 
               {
			// error opening database connection
                       e.printStackTrace();
		}
	}

GreeBo

CreepingDeath wrote: »

You could make the connOpen() and connClose() methods private and only call them in startDatabase() and stopDatabase().
So the connection is automatically opened and closed when the server starts & stops.

Even this is likely to cause problems...how long is this connection going to stay alive?
If performance not an issue, and connections are "rare" (for you definition of rare!) then just open, use, close each time.
Otherwise use a pool.