patternjavaMinor
Java and MySQL connection classes and method implementations
Viewed 0 times
implementationsmethodjavamysqlclassesandconnection
Problem
I got three classes for MySQL Database access/manipulation:
These are my classes in the mentioned order:
First class
```
public class Conector {
private Connection connection = null;
private Statement statement = null;
private ResultSet set = null;
String host;
String port;
String login;
String password;
String url;
public Conector (String login, String password, String db, String host, String port) {
this.login = login;
this.password = password;
this.host = host;
this.port = port;
url = "jdbc:mysql://"+host+":"+port+"/"+db;
Conectar ();
}
public void Desconectar () {
connection = null;
}
private void Conectar() {
try {
DriverManager.registerDriver(new com.mysql.jdbc.Driver ());
} catch (SQLException ex) {
Logger.getLogger(Conector.class.getName()).log(Level.SEVERE, null, ex);
}
try {
connection = DriverManager.getConnection(url, login, password);
statement = connection.createStatement();
}
catch (SQLException e) {
Logger logger = Logger.getLogger(Conector.class.getName());
logger.log(Level.SEVERE, e.getSQLState(), e);
}
}
public ResultSet Query(String query){
try {
statement = connection.createStatement();
set = statement.executeQuery(query);
}
catch (Exception e) {
System.out.println("Exception in query method:\n" + e.getMessage());
}
return set;
}
public boolean Update
Conector. It has got methods for connecting, disconnecting, querying and updating db.
ConectorCliente. It simply extends first and got its constructor receivinguser,password,host,portanddb.
- I have written has several different methods per each DB, depending on data which will be needed to be selected, updated or deleted.
These are my classes in the mentioned order:
First class
```
public class Conector {
private Connection connection = null;
private Statement statement = null;
private ResultSet set = null;
String host;
String port;
String login;
String password;
String url;
public Conector (String login, String password, String db, String host, String port) {
this.login = login;
this.password = password;
this.host = host;
this.port = port;
url = "jdbc:mysql://"+host+":"+port+"/"+db;
Conectar ();
}
public void Desconectar () {
connection = null;
}
private void Conectar() {
try {
DriverManager.registerDriver(new com.mysql.jdbc.Driver ());
} catch (SQLException ex) {
Logger.getLogger(Conector.class.getName()).log(Level.SEVERE, null, ex);
}
try {
connection = DriverManager.getConnection(url, login, password);
statement = connection.createStatement();
}
catch (SQLException e) {
Logger logger = Logger.getLogger(Conector.class.getName());
logger.log(Level.SEVERE, e.getSQLState(), e);
}
}
public ResultSet Query(String query){
try {
statement = connection.createStatement();
set = statement.executeQuery(query);
}
catch (Exception e) {
System.out.println("Exception in query method:\n" + e.getMessage());
}
return set;
}
public boolean Update
Solution
I seen a lot of things what can better so I'm glad you came here to learn.
First :
Your
The
Here is a problem : Or you set your first class abstract if you know you are going to have other implementations (preferable with more code) or your first class should be an interface.
Second :
Why do you do it right for Connection, Statement an Resultset and don't set a modifier before all the strings?
The best practice is make them all private and if you need them outside the class make an getter/setter for it.
Third :
should be :
Method names do not start with a capital.
starting with a capital is for classes.
Fourth :
You rely on users of your class that they close your connection.
You will have memory leaks cause they will forget to close your connection.
There are 2 possibilities what you can do.
First :
So override the finalize of the class and close the connection there.
This is called when the class is destroyd.
I prefer the second one cause the finalize has issues that it could be that it is never called.
As it seems in the comment a nice discussion on this issue :).
Second :
Open and close your connection before you do an action to the DB.
Do a try-with-resource or close the connection in the finally block of the try-catch.
Edit : added sample code.
Fifth :
Your
So no instanciation has to be made.
Why not make the class static and maybe even final when you want to be sure nobody may extend that class.
First :
Your
Conectoris a normal class.The
ConectorCliente extends that class and do just calling the super constructor without anything else.Here is a problem : Or you set your first class abstract if you know you are going to have other implementations (preferable with more code) or your first class should be an interface.
Second :
private Connection connection = null;
private Statement statement = null;
private ResultSet set = null;
String host;
String port;
String login;
String password;
String url;Why do you do it right for Connection, Statement an Resultset and don't set a modifier before all the strings?
The best practice is make them all private and if you need them outside the class make an getter/setter for it.
Third :
private void Conectar() {should be :
private void conectar() {Method names do not start with a capital.
starting with a capital is for classes.
Fourth :
You rely on users of your class that they close your connection.
You will have memory leaks cause they will forget to close your connection.
There are 2 possibilities what you can do.
First :
@Override
protected void finalize() throws Throwable {
super.finalize();
cierraConexion();
}So override the finalize of the class and close the connection there.
This is called when the class is destroyd.
I prefer the second one cause the finalize has issues that it could be that it is never called.
As it seems in the comment a nice discussion on this issue :).
Second :
Open and close your connection before you do an action to the DB.
Do a try-with-resource or close the connection in the finally block of the try-catch.
Edit : added sample code.
// Register the driver once in the constructor.
private Connection getConectar() throws SQLException {
return DriverManager.getConnection(url, login, password);
}
public boolean Update (String update) {
int result;
try (Connection connection = getConectar(),
Statement statement = connection.createStatement()) {
result = statement.executeUpdate(update);
} catch (SQLException e) {
System.out.println("Exception in update method:\n" + e.getMessage());
return false;
}
return (result!=0); // if 0 is returned there is NO row changed => means nothing is updated.
}Fifth :
Your
InteraccionDB has only all static methods and variable. So no instanciation has to be made.
Why not make the class static and maybe even final when you want to be sure nobody may extend that class.
Code Snippets
private Connection connection = null;
private Statement statement = null;
private ResultSet set = null;
String host;
String port;
String login;
String password;
String url;private void Conectar() {private void conectar() {@Override
protected void finalize() throws Throwable {
super.finalize();
cierraConexion();
}// Register the driver once in the constructor.
private Connection getConectar() throws SQLException {
return DriverManager.getConnection(url, login, password);
}
public boolean Update (String update) {
int result;
try (Connection connection = getConectar(),
Statement statement = connection.createStatement()) {
result = statement.executeUpdate(update);
} catch (SQLException e) {
System.out.println("Exception in update method:\n" + e.getMessage());
return false;
}
return (result!=0); // if 0 is returned there is NO row changed => means nothing is updated.
}Context
StackExchange Code Review Q#49036, answer score: 7
Revisions (0)
No revisions yet.