Displaying user scores

I have started teaching myself MySQL and created a simple application which asks for user input stores it and displays it. Can you guys please let me know if there is something I am doing wrong or any tips on what can be improved? I want to build a student management application which will use a MySQL database and want to make sure I know the basics.

I have a class that holds connection method and closing methods:

import java.sql.*;

public class DBUtil {

// Connecting to database.
public static Connection getConnection() throws SQLException {
String url = "jdbc:mysql://localhost:3306/database";
String user = "root";
String pass = "admin";

Connection connection = DriverManager.getConnection(url, user, pass);
    return connection;
}

// Close database connection.
public static void closeConnection(Connection con) {
    try {
        if(con != null) {
            con.close();
            System.out.println("Database Connection Closed!");
        }
    }catch(Exception e) {
        e.printStackTrace();
    }
}
// Close Prepared Statement.
public static void closePrepStatement(PreparedStatement pStmt) {
    try {
        if(pStmt != null) {
            pStmt.close();
            System.out.println("Prepared Statemet Closed");
        }
    }catch (Exception e) {
        e.printStackTrace();
    }
}
// Close Result Set.
public static void closeResSet(ResultSet rSet) {
    try{
        if(rSet != null) {
            rSet.close();
            System.out.println("Result Set Closed");
        }
    }catch (Exception e) {
        e.printStackTrace();
    }
}
}

This class gets the input, stores it, and displays it:

```
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.util.Scanner;

// SQL variables defined
public class Source {
DBUtil dbConnection = new DBUtil();
Scanner input = new Scanner(System.in);
Connection connection = null;
PreparedStatement pStatement = null;
ResultSet rSet = null;

// Constructor
pub

• It is usually considered bad practice to use * imports. Be specific about your dependencies.

• Don't use root DB access for your application. That should truly be reserved for a root DB user. Create a MySQL user for the application with appropriate privilege levels.

• Don't hard code your DB credentials into your code. This should ideally be derived from application or environmental configuration.

• Should you really be swallowing up your exceptions like this?

• Your validation against NPE's seems odd here. Rather than a pattern like

this:

try {
    if(con != null) {
        con.close();
        System.out.println("Database Connection Closed!");
    }
}catch(Exception e) {
    e.printStackTrace();
}

perhaps you should be doing something like:

if(con == null) {
    throw new NullPointerException('...');
}
// rest of code in method. Now, without need to be nested in conditional

But you should definitely let the caller know if they passed you a null pointer vs. swallowing it.

• I find it very odd to be opening prepared statements and result sets in your Source class but closing them using the DBUtil class. This seems very asymmetric to me and a leaking of responsibility outside the class.

• Don't get yourself into the bad habit of using SELECT * queries. They can be wasteful of bandwidth (by pulling unnecessary fields from DB into application), they can obfuscate the data structure that is being worked in within the code (i.e. you have to go look at the table in the database to understand what fields you are working with), and it can make your code fragile to table schema changes

• getInput() is a bad method name. This method is actually both reading in user input and inserting data in the database, something that is not implied from the method name, and activities that should probably be split into individual methods. The same could be said about displayData(). This method is not just displaying data, but also retrieving it from the database.

• Consider passing the database connection and the scanner to the Source class as dependencies rather than having the class have to hold the knowledge on how to set them up. Of course, this might require a larger thought shift towards dependency injection throughout other areas of code.

• I know this is a bit of a trivial example, but why would the constructor for the class trigger both the input and ultimate display of content?

• I don't see the need to have class properties for storing the prepared statement and result set objects.

• You don't use dbConnection class property at all. There is probably no need for even storing the connection if you aren't going to reference if across different methods - something not happening the way this class is currently designed.

• I see no need to actually connect twice to the database here, especially considering this class is really designed for the purpose of doing one single operation that is really more procedural in nature.

• When building classes in the future, begin to think about how to separate class functionality from end user messaging or view output. Again, I know this is a simple example, but the more you do this in the future, the more you are better able to make your classes re-usable..

StackExchange Code Review Q#159272, answer score: 4