patternjavaMinor
Creating and deleting employees in a database
Viewed 0 times
deletingemployeescreatingdatabaseand
Problem
I've been learning Java Database Programming for like 2 days, I read theory at first. Now decided to get stuck in. I just wanted a few pointers.
-
I am using JavaDB (Netbeans Service) should I be using something better? As I've read it has some security flaws.
-
I split one class to two (as you'll see below), is what I've done correct?
-
How can I improve my code to make it "more professional" or more like "industry" standard code?
I bit about my previous background, I am back-end PHP developer, I use PDO and understand and use prepared statements so that wasn't too hard to pick up.
Database.java
```
/**
* @author Script47
*/
package ers.database;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.sql.Statement;
import java.sql.PreparedStatement;
public class Database {
Connection db;
public Statement statement;
public PreparedStatement preparedStatement;
String[] dbConfig = {"jdbc:derby://localhost:1527/ERS", "Script47", "dbpassword"};
public Database() {
try {
db = DriverManager.getConnection(dbConfig[0], dbConfig[1], dbConfig[2]);
statement = db.createStatement();
} catch (SQLException ex) {
System.err.println(ex.toString());
}
}
public void tryCloseConnectionToDatabase() {
if (db != null) {
try {
db.close();
} catch (SQLException ex) {
System.err.println(ex.toString());
}
}
}
public void tryCloseStatement() {
if (this.statement != null) {
try {
statement.close();
} catch (SQLException ex) {
System.err.println(ex.toString());
}
}
}
public void tryClosePreparedStatement() {
if (this.preparedStatement != null) {
try {
preparedStatement.close();
} catch (SQLException ex) {
-
I am using JavaDB (Netbeans Service) should I be using something better? As I've read it has some security flaws.
-
I split one class to two (as you'll see below), is what I've done correct?
-
How can I improve my code to make it "more professional" or more like "industry" standard code?
I bit about my previous background, I am back-end PHP developer, I use PDO and understand and use prepared statements so that wasn't too hard to pick up.
Database.java
```
/**
* @author Script47
*/
package ers.database;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.sql.Statement;
import java.sql.PreparedStatement;
public class Database {
Connection db;
public Statement statement;
public PreparedStatement preparedStatement;
String[] dbConfig = {"jdbc:derby://localhost:1527/ERS", "Script47", "dbpassword"};
public Database() {
try {
db = DriverManager.getConnection(dbConfig[0], dbConfig[1], dbConfig[2]);
statement = db.createStatement();
} catch (SQLException ex) {
System.err.println(ex.toString());
}
}
public void tryCloseConnectionToDatabase() {
if (db != null) {
try {
db.close();
} catch (SQLException ex) {
System.err.println(ex.toString());
}
}
}
public void tryCloseStatement() {
if (this.statement != null) {
try {
statement.close();
} catch (SQLException ex) {
System.err.println(ex.toString());
}
}
}
public void tryClosePreparedStatement() {
if (this.preparedStatement != null) {
try {
preparedStatement.close();
} catch (SQLException ex) {
Solution
Working with
Your class design stinks. The
Methods that take more than a few parameters are awful, and quickly become a usability and maintenance nightmare. What about validation of all those parameters? It would be better to make the method take an
Don't catch exceptions and print the stack trace. Log instead. If possible, don't catch
Connection, PreparedStatement objects directly is extremely tedious. It involves a lot of boilerplate try-catch blocks that are hard to read and hard to get right. I recommend to look into Spring's JdbcTemplate. It simplifies the resource management and exception handling a lot, so that you can focus on your business logic.Your class design stinks. The
Database class is certainly not a"database ". It's a bunch of boilerplate utility methods and a connection string put together. This is not good abstraction and certainly not something to inherit from. As @BanFinch suggested, look into the data access object pattern.Methods that take more than a few parameters are awful, and quickly become a usability and maintenance nightmare. What about validation of all those parameters? It would be better to make the method take an
EmployeeData object of some sort, probably with a constructor to validate its fields. Since too many constructor parameters are just as bad as too many method parameters, the builder pattern might be useful.Don't catch exceptions and print the stack trace. Log instead. If possible, don't catch
Exception, but user the most specific kind of exception possible.Context
StackExchange Code Review Q#80371, answer score: 2
Revisions (0)
No revisions yet.