Problem
I was following this tutorial for JavaFX and Gradle:
JavaFX with Gradle, Eclipse and, Scene Builder on OpenJDK11
After completion of the tutorial you will have a simple GUI which you input a lower and upper bound and generate random numbers between them. The generated numbers and their bounds are then saved and listed in another view.
After completion of this tutorial I wanted to integrate MySQL as the back end.
The tutorial creator acknowledged at some point that a database like SQLite would be the preferred back end but that it was outside the scope of the tutorial. Luckily it seems he designed the app in such a way that he uses DAO class and I just replaced his code in there with my own JDBC implementation.
package RandomNumber.repository;
import java.util.*;
import java.sql.*;
import RandomNumber.models.RandomNumber;
public class LocalRandomNumberDAO implements RandomNumberDAO {
public LocalRandomNumberDAO() {
}
private List<RandomNumber> numbers = new ArrayList<>();
private Connection openConn() throws SQLException {
return DriverManager.getConnection("jdbc:mysql://localhost:3306/numbers?" +
"user=demo_java&password=1234");
}
@Override
public boolean save(RandomNumber number) {
boolean res = false;
try (
var conn = openConn();
var stmt = conn.prepareStatement("insert into numbers_tabl values(?,?,?,?,?)");
) {
stmt.setInt(1, 0);
stmt.setInt(2, number.getNumber());
stmt.setInt(3, number.getLowerBounds());
stmt.setInt(4, number.getUpperBounds());
stmt.setDate(5, java.sql.Date.valueOf(number.getCreatedAt()));
res = stmt.execute();
} catch (SQLException e) {
e.printStackTrace();
}
return res;
}
@Override
public void loadNumbers() {
try (
var conn = openConn();
var stmt = conn.createStatement();
) {
String strQuery = "select * from numbers_tabl;";
var rset = stmt.executeQuery(strQuery);
numbers.clear();
while(rset.next()) {
numbers.add(new RandomNumber(
rset.getDate("created").toLocalDate(),
rset.getInt("number"),
rset.getInt("min"),
rset.getInt("max")
)
);
}
} catch (SQLException e) {
e.printStackTrace();
}
}
@Override
public List<RandomNumber> getNumbers() {
return Collections.unmodifiableList(numbers);
}
}
I am also calling loadNumbers each time the generate button is pressed, so I essentially insert 1 row, and then query all rows in the database.
Questions:
1. Must I always provide a value for the Primary Key to the PreparedStatement in the save method? my table description follows:
mysql> describe numbers_tabl;
+---------+------+------+-----+---------+----------------+
| Field | Type | Null | Key | Default | Extra |
+---------+------+------+-----+---------+----------------+
| id | int | NO | PRI | NULL | auto_increment |
| number | int | NO | | NULL | |
| min | int | NO | | NULL | |
| max | int | NO | | NULL | |
| created | date | YES | | NULL | |
+---------+------+------+-----+---------+----------------+
5 rows in set (0.00 sec)
2. What best practices am I blatantly ignoring?
3. Is there a better way? Or rather, is there a library or framework that is more worth my time?
4. This is my first time using Java-11. I have always used pretty much only Java 7 or 8 features. Is my usage of the var keyword appropriate?
Solution
You’re not following Java naming conventions. Package names should be all lower case.
The JDBC URL is hard coded. This should be provided as a parameter. Or better yet, provide the connection with dependency injection so you can unit test it with mocks.
Worst “offense” is hiding the exceptions from the caller. Instead of printing the stack trace (which is only visible to the end user) you should wrap it in a domain specific exception. This way the application using your library can take proper actions.
When saving the number you list all columns as parameters in the SQL. Including the automatically generated index, which you insert as zero. That was confusing. You should list only the columns that you provide as “INSERT INTO numbers_tabl (number, min, max) …” and let the database handle the columns that can be generated automatically (index and creation date).
Instead of returning a boolean from saveNumber you could return the RandomNumber object updated with it’s new database identifier and creation date.
The names in the database table don’t describe their purposes. Table us named “numbers_tabl” but it contains random numbers. The name should tell the reader what the table contains. Also the _tabl suffix is probably a bit pointless. The “created” column name suggests that the value is a boolean. It should be “creation_time”. I don’t remember MySQL data types from memory but it probably should be a timestamp, not just date.
The method choice comes from the tutorial, right? It’s a bit weird to load the numbers into memory and access them from there. After saving a number you have to call loadNumbers before you get the latest number back from getNumbers. The getNumbers should load the numbers from the database. If you want to cache the numbers in memory then the saveNumbers should invalidate the cache, but that’s probably a subject for a later tutorial. 🙂 Also there are libraries for that (Ehcache for example).
The standard library for this purpose is JPA (Java Persistence API or Jakarta Persistence). It makes boilerplate SQL so much easier.
I use var almost everywhere. To me, it makes the code more concise and easier to refactor. Essentially, unless it makes the code harder to read / understand I use it. An important element of this is variable naming. For example:
boolean res = false;
res probably means result, why not call it that… or better still, what it’s represents the result of:
var saved = false;
Similarly, rset could be resultSet or rows. It’s also fairly out of fashion to include variable types in variable names, so rather than strQuery, you probably want to just go with query.
It’s been a while since I used this type of connection, however result sets often need to be cleaned up after you’re done with them. It looks like there’s a close method on it, so you might want to wrap it in a try with resources block as well…
try(var rows = statement.executeQuery(query)) {