4
\$\begingroup\$

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.

  1. I am using JavaDB (Netbeans Service) should I be using something better? As I've read it has some security flaws.

  2. I split one class to two (as you'll see below), is what I've done correct?

  3. 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) {
                System.err.println(ex.toString());
            }
        }
    }
}

Employees.java (same package, I split Database.java)

/**
 * @author Script47
 */
package ers.database;

import java.sql.ResultSet;
import java.sql.SQLException;

public class Employees extends Database {

    public void createNewEmployee(String firstName, String secondName, String emailAddress, String jobRole, Integer currentStatus, Integer hireType, Double wages) {
        Integer employeeID;
        try {
            String getRowCountSQL = "SELECT COUNT(*) AS rowcount FROM employees";
            ResultSet getRowCountResultSet = statement.executeQuery(getRowCountSQL);

            getRowCountResultSet.next();
            employeeID = getRowCountResultSet.getInt("rowcount") + 1;
            getRowCountResultSet.close();

            String createNewEmployeeSQL = "INSERT INTO employees (ID, first_name, second_name, email_address, job_role, current_status, hire_type, wage) VALUES (?, ?, ?, ?, ?, ?, ?, ?)";
            preparedStatement = db.prepareStatement(createNewEmployeeSQL);
            preparedStatement.setInt(1, employeeID);
            preparedStatement.setString(2, firstName);
            preparedStatement.setString(3, secondName);
            preparedStatement.setString(4, emailAddress);
            preparedStatement.setString(5, jobRole);
            preparedStatement.setInt(6, currentStatus);
            preparedStatement.setInt(7, hireType);
            preparedStatement.setDouble(8, wages);

            if (preparedStatement.executeUpdate() == 1) {
                System.out.println("Employee record added to system.");
            } else {
                System.err.println("Error adding new employee record to system.");
            }
        } catch (Exception ex) {
            System.err.println(ex.toString());
        } finally {
            tryClosePreparedStatement();
            tryCloseConnectionToDatabase();
        }
    }

    public void deleteEmployee(Integer employeeID) {
        try {
            String deleteEmployeeSQL = "DELETE FROM employees where ID = ?";
            preparedStatement = db.prepareStatement(deleteEmployeeSQL);
            preparedStatement.setInt(1, employeeID);

            if (preparedStatement.executeUpdate() == 1) {
                System.out.println("Employee record deleted.");
            } else {
                System.err.println("Error deleted employee record.");
            }
        } catch (SQLException ex) {
            System.err.println(ex.toString());
        } finally {
            tryClosePreparedStatement();
            tryCloseConnectionToDatabase();
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

Working with 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.

\$\endgroup\$
  • \$\begingroup\$ To answer you question about validation, all that is done in a separate class, this is only for DB stuff. \$\endgroup\$ – Script47 Feb 13 '15 at 21:37
1
\$\begingroup\$

To make it more "industry" standard I would recommend you to take a look at the Data Access Object design pattern. It helps you separate concerns and gives you flexibility when it comes to implementation.

I personally don't like to have my SQL in the code so I might have just calls in Java and perhaps functions or stored procedures in the DB, this way I can control what the SQL does separating the code from it, I mean, suppose you get a request to bring not only the name of an employee but also his last name, being the case you would need to change the query (code level), then perhaps re-compile and re-deploy, I don't know, I wouldn't recommend it.

Regards!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.