2

I am working on a school project in java and mssql where i have some trouble with the database connection.

Is necessary to close the database connection in a situation like this?

I want to build an Animal object which consist of a Person object.

this is the code from DBLayer/DBAnimal:

First we have the method where i connect to the database and get the resultset.

public Animal findAnimal(int id) 
    {
        con = DBConnection.getInstance().getDBcon();
        PreparedStatement preStmnt = null;
        String query = "select * from animal where id = ?";
        Animal animalObj = null;
        ResultSet results;
        try 
        {
            preStmnt = con.prepareStatement(query);
            preStmnt.setInt(1, id);
            results = preStmnt.executeQuery();
            if (results.next()) 
            {
                animalObj = buildAnimal(results);
            }
        } catch (SQLException SQLe) {
            System.out.println("SQL Exception i findAnimal: " + SQLe);
        } catch (Exception e) {
            System.out.println("Exception i findAnimal() " + e.getMessage());

        }

        DBConnection.closeConnection(); 
        return animalObj;
    }

then where i build the Animal object and call the FindPersonOnId method:

public Animal buildAnimal(ResultSet results) throws SQLException 
    {

        Animal animalObj = new Animal();

        Person personObj = new Person();

        try 
        {

            animalObj.setId(results.getInt("id"));
            animalObj.setName(results.getString("name"));           
            animalObj.setRace(results.getInt("raceid"));
            animalObj.setSpecie(findSpecieId(results.getInt("raceid")));
            animalObj.setSex(results.getString("sex").charAt(0));
            animalObj.setBorn(results.getDate("born"));
            animalObj.setAlive(results.getBoolean("alive"));
            animalObj.setPerson(dbPerson.findPersonOnId(results.getInt("personId")));
        }

        catch (SQLException e) 
        {
            System.out.println("Exception i buildAnimal" + e.getMessage());
        }

        return animalObj;
    }

This is from the DBLayer/DBPerson:

public Person findPersonOnId(int id) 
    {
        con = DBConnection.getInstance().getDBcon();
        PreparedStatement preStmnt = null;
        String query = "SELECT * FROM " + TABLE_NAME + " WHERE id = ?";
        Person personObj = null;
        ResultSet results;
        try 
        {
            preStmnt = con.prepareStatement(query);
            preStmnt.setInt(1, id);
            results = preStmnt.executeQuery();
            if (results.next()) 
            {
                personObj = buildPerson(results);
            }
        } 
        catch(SQLException SQLe) 
        {
            System.out.println("SQLException i findPersonOnId(id): " + SQLe);
        } 
        catch (Exception e)
        {
            System.out.println("Fejl i findPersonOnId(id): " + e.getMessage());
        }

        DBConnection.getInstance().closeConnection();
        return personObj;
    }

as you can see i am using the closeConnection both times and this is causing some problems since it is very slow. And i was wandering wheater it is necessary to close the connection at all in this situation.

If it isn't, in what situation would it be?

Michael Kirkegaard
  • 379
  • 1
  • 3
  • 14
  • 1
    The design of the `DBConnection` class seems problematic; `getDBcon()` method returns a `Connection` instance, but on the other hand, its `closeConnection()` method does not take a `Connection` instance as an input parameter. How does the class function internally -- will `closeConnection()` always close the last connection that was opened? What happens if you have multiple connections open simultaneously? Or does `DBConnection` always only contain a single `Connection` instance? – Mick Mnemonic Jan 10 '16 at 00:59
  • 1
    Usually, closing/opening `Connection`s is managed externally, e.g. by your application server (which internally distributes connections from a _connection pool_). However, you _should_ close `Statement` and `ResultSet` objects (which is what your code is not doing ATM). Best way to do this is using the [try-with-resources statement](http://stackoverflow.com/questions/8066501/how-should-i-use-try-with-resources-with-jdbc). – Mick Mnemonic Jan 10 '16 at 01:03
  • @MickMnemonic If your application server uses a connection pool, you should still close the connection to indicate that you're done with it; in that case this doesn't really close the connection but return it to the pool. However you don't need to know that: you should close the connection when you're done with it. – Erwin Bolwidt Jan 10 '16 at 01:50
  • @ErwinBolwidt, it depends -- you should only close connections you've opened/acquired yourself. And this is best done using try-with-resources. However, you shouldn't close connections that have been injected into your DAO and might be used elsewhere in the same _transaction_. – Mick Mnemonic Jan 10 '16 at 02:16
  • @MickMnemonic, But the problem i have now is when i built an Animal object with an Person object i have to connect with the database two times and as it is right now i close the connection both times, which is a problem when it comes to performance/speed. so if i use the try-with-resources statement, wouldn't i have the same problem? – Michael Kirkegaard Jan 10 '16 at 03:02
  • Yes, opening/closing a connection is slow, which is why app servers use connection pools. My comment was more about closing the `PreparedStatement`, which is something you should add to your code. Regarding your performance problem, I think you have two options: either use a single connection which you pass to the other method or (even better), only execute a single query that combines data from the two tables using an SQL `JOIN`. – Mick Mnemonic Jan 10 '16 at 03:16
  • @MickMnemonic You're right, but that was implied by "when you're done with it". You can't be sure that you're done with the connection if it was passed to you. But it's good to make it more explicit. – Erwin Bolwidt Jan 10 '16 at 10:09

1 Answers1

2

Yes it is.

Every time you establish a connection you have to make sure it gets closed eventually, otherwise you will be leaking resources and will be left with too many open connection and your application will break.

Having said that, there are tools and libraries that can make this an easier thing to do, Spring JdbcTemplate is one of them.

Without looking at your DBConnection class, it seems like it is trying to do what a Datasource does, consider using a proper datasource instead.

Sleiman Jneidi
  • 22,907
  • 14
  • 56
  • 77