tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <>
Subject Re: web application - student need help
Date Fri, 05 Jan 2007 22:10:13 GMT
Hash: SHA1


Michael Ni wrote:
> i don't get any error when there isn't that much traffic
> but i dont close my jdbc connections, could that be a problem?

Oooooh, yeah. Failing to close connections is very likely to give you
errors, as you will end up running out of memory, or the database server
will cut you off when you reach the maximum number of connections.

Someone else mentioned the use of a connection pool, which I highly
recommend. You can use Tomcat to configure a connection pool and then
obtain a JDBC DataSource through JNDI. Then, you can get a connection
from that for use.

It's not really as complicated as it sounds ;)

>         try {
>             DBConstants db = new DBConstants();
>             Class.forName(db.getDrivername());
>             Connection conn;
>             conn =
> DriverManager.getConnection("jdbc:microsoft:sqlserver://" +
> db.getHostname() + "","" + db.getUsername() + "","" + db.getPassword() +
> "");
>             Statement stmt = conn.createStatement();
>             ResultSet rs = stmt.executeQuery(queryStr);
>             return rs;
>         }
>         catch(Exception e) {
>             e.printStackTrace();
>             System.out.println("getData error");
>             throw new Exception();
>         }
>     }

Let me suggest some changes to your code:

Connection conn = null;
Statement stmt = null;
ResultSet rs = null;

    conn = (get connection from jndi datasource)
    stmt = conn.createStatement();
    rs = stmt.executeQuery(queryStr);

    // Package the results into Java objects
    // instead of using the ResultSet directly

    return ??;
catch (SQLException sqle)  // Don't catch "Exception" unless you have to
    System.err.println("getData error");  // System.err is better

    throw sqle; // No need for a new exception; use existing one
    if(null != rs)
        try { rs.close(); } catch (SQLException sqle)
        { System.err.println("Could not close ResultSet"); }
    if(null != stmt)
        try { stmt.close(); } catch (SQLException sqle)
        { System.err.println("Could not close Statement"); }
    if(null != conn)
        try { conn.close(); } catch (SQLException sqle)
        { System.err.println("Could not close Connection"); }

This is about as clean as you can get. If you must return the ResultSet
to to the calling method, then you will have to do something different.
I recommend you copy from the ResultSet into something like an
ArraryList of objects specific for your needs.

One last thing: when possible, use PreparedStatement instead of
Statement for your queries. The use of this class goes a long way
towards protecting you against SQL injection attacks (where people can
do nasty things like drop tables and stuff). It requires a bit more
setup (especially if you are trying to write a reusable method to
execute all your queries) but I think it's worth it.

- -chris
Version: GnuPG v1.4.6 (MingW32)
Comment: Using GnuPG with Mozilla -


To start a new topic, e-mail:
To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message