Arne VajhHj wrote:
Are Nybakk wrote:
I started replying to this post, but I gave it up. I don't quite get
what you want answered here. I'll give you some tips on how to
organize things.
Make a class that takes care of the database connection:
public class DatabaseConnector {
private Connection con;
public DatabaseConnector(user, pass, ...) {
con = ...;
}
public Connection getConnection() {
return con;
}
public void close() {
con.close();
}
//...
}
What does that class provide that Connection does not ?
I see your point. The idea is to isolate what has got to do with the
actual initialization and maintainance of the server connection. Only
this class needs to know about JDBC driver and such. That way it can
also be reused.
And a database handler class:
public class DatabaseHandler {
private DatabaseConnector dbCon;
private Connection con;
private PreparedStatement stmt1 = new PreparedStatement("...");
public DatabaseHandler(user, pass, ...) {
dbCon = new DatabaseConnector(user, pass, ...);
con = dbCon.getConnection();
}
public void insertSomeObject(SomeObject obj) {
stmt.setXxx(...);
//...
stmt.executeXxx();
}
/...
public void closeConnection() {
dbCon.close();
}
}
That DatabaseConnector is not needed is emphasized by the fact
that you have both the DatabaseConnector wrapper and the underlying
Connection as fields here.
Alright let me revise. I guess I wrote this in a hurry.
(Error catching left out)
public class DatabaseConnector {
private String serverURL = "...";
private String driverName = "...";
private Connection con;
private PreparedStatement stmt1 = new PreparedStatement("...");
//...
public DatabaseConnector(user, pass) {
Class.forName(driverName);
con = DriverManager.getConnection(serverURL, user, pass);
}
public void insertSomeObject(SomeObject obj) {
stmt.setXxx(obj.getZzz);
//...
stmt.executeWww();
}
This makes it hard to reuse the class. It would be better to have a
public boolean isActive() {
return !con.isClosed();
}
public void close() {
stmt.close();
con.close();
}
//...
}
public class DatabaseHandler {
private DatabaseConnector dbCon;
public DatabaseHandler(user, pass, ...) {
dbCon = new DatabaseConnector(user, pass, ...);
}
public insert(SomeObject obj) {
//dbCon.insertSomeObj(...)
}
public insert(Collection<SomeObject> obj) {
//loop: dbCon.insertSomeObj(...)
}
//...
public void end() {
if(dbCon.isActive()) {
dbCon.close();
}
//Any other close operataions.
}
}
Furthermore you should really open and close connection in the
insert method. If you keep the DatabaseHandler around then you
collect way to many database connections. If you construct and
close DatabaseHandler for every insert call, then you could
just as well do everything in the insert method.
Remember that the point of PreparedStatements is a way to declare
statements that are frequently used. To declare such a statement for
every method call removes the point entirely.
PreparedStatement provides other useful functionality including:
- handling of special characters like ' (both relevant for
scottish names and malicious SQL injection)
- date format handling
About closing connections, simply close them when you won't be using
it for a while. I don't think open connections would make any problem
unless the database is accessed by a lot of clients simultaneously.
If it does not cost anything to make the code scalable, then why'
not do it ?
You never know how long time some code will live and in how many
different contexts it will be used !
Point taken.
Arne