I improved the code from my previous question.
At this moment, the 3 entities have not yet a connection with each other. They should only be available for CRUD actions in a SQLite database.
I would like to have a review of this basic CRUD application so I can improve and learn from your experience. The full code can be found here.
- Design pattern/coding principles improvement that could be used
- Testing
- Exception handling
- Readability
- Coding best practices (code duplication, ...)
- What should be Javadocumented?
DatumControle.java
public class DatumControle {
private Repository db;
public DatumControle(String dbType) throws ServiceException {
try {
db = RepositoryFactory.createDatabase(dbType);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void addProduct(Product product) throws ServiceException {
try {
db.addProduct(product);
} catch (DatabaseException ex) {
System.out.println(product);
throw new ServiceException(ex);
}
}
public Product getProductByEan(long ean) throws ServiceException {
try {
return db.getProductByEan(ean);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public Product getProductByHope(int hope) throws ServiceException {
try {
return db.getProductByHope(hope);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public Collection<Product> getAllProducts() throws ServiceException {
try {
return db.getAllProducts();
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void updateProduct(Product product) throws ServiceException {
try {
db.updateProduct(product);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void deleteProduct(long ean) throws ServiceException {
try {
db.deleteProduct(ean);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void addCategory(Category category) throws ServiceException {
try {
db.addCategory(category);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public Category getCategory(String name) throws ServiceException {
try {
return db.getCategory(name);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public Collection<Category> getAllCategories() throws ServiceException {
try {
return db.getAllCategories();
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void updateCategory(Category category) throws ServiceException {
try {
db.updateCategory(category);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void deleteCategory(String name) throws ServiceException {
try {
db.deleteCategory(name);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void addLocation(Location location) throws ServiceException {
try {
db.addLocation(location);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public Location getLocation(String name) throws ServiceException {
try {
return db.getLocation(name);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public Collection<Location> getAllLocations() throws ServiceException {
try {
return db.getAllLocations();
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void updateLocation(Location location) throws ServiceException {
try {
db.updateLocation(location);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
public void deleteLocation(String name) throws ServiceException {
try {
db.deleteLocation(name);
} catch (DatabaseException ex) {
throw new ServiceException(ex);
}
}
}
RepositoryFactory.java
public class RepositoryFactory {
public static Repository createDatabase(String dbType) throws DatabaseException {
if(dbType.equals("sqlite")){
return new SQLiteRepository("jdbc:sqlite:DatumControle.sqlite");
}
throw new DatabaseException(ErrorMessages.DATABASETYPE_NOT_SUPPORTED);
}
}
Repository.java
public interface Repository {
public void addProduct(Product product) throws DatabaseException;
public Product getProductByEan(long ean) throws DatabaseException;
public Product getProductByHope(int hope) throws DatabaseException;
public Collection<Product> getAllProducts() throws DatabaseException;
public void updateProduct(Product product) throws DatabaseException;
public void deleteProduct(long ean) throws DatabaseException;
public void addCategory(Category category) throws DatabaseException;
public Category getCategory(String name) throws DatabaseException;
public Collection<Category> getAllCategories() throws DatabaseException;
public void updateCategory(Category category) throws DatabaseException;
public void deleteCategory(String name) throws DatabaseException;
public void addLocation(Location location) throws DatabaseException;
public Location getLocation(String name) throws DatabaseException;
public Collection<Location> getAllLocations() throws DatabaseException;
public void updateLocation(Location location) throws DatabaseException;
public void deleteLocation(String name) throws DatabaseException;
}
SQLiteRepository.java
public class SQLiteRepository implements Repository {
private SQLiteProductRepository productDb;
private SQLiteCategoryRepository categoryDb;
private SQLiteLocationRepository locationDb;
public SQLiteRepository(String url) throws DatabaseException {
productDb = new SQLiteProductRepository(url);
categoryDb = new SQLiteCategoryRepository(url);
locationDb = new SQLiteLocationRepository(url);
}
@Override
public void addProduct(Product product) throws DatabaseException {
productDb.addProduct(product);
}
@Override
public Product getProductByEan(long ean) throws DatabaseException {
return productDb.getProductByEan(ean);
}
@Override
public Product getProductByHope(int hope) throws DatabaseException {
return productDb.getProductByHope(hope);
}
@Override
public Collection<Product> getAllProducts() throws DatabaseException {
return productDb.getAllProducts();
}
@Override
public void updateProduct(Product product) throws DatabaseException {
productDb.updateProduct(product);
}
@Override
public void deleteProduct(long ean) throws DatabaseException {
productDb.deleteProduct(ean);
}
@Override
public void addCategory(Category category) throws DatabaseException {
categoryDb.addCategory(category);
}
@Override
public Category getCategory(String name) throws DatabaseException {
return categoryDb.getCategory(name);
}
@Override
public Collection<Category> getAllCategories() throws DatabaseException {
return categoryDb.getAllCategories();
}
@Override
public void updateCategory(Category category) throws DatabaseException {
categoryDb.updateCategory(category);
}
@Override
public void deleteCategory(String name) throws DatabaseException {
categoryDb.deleteCategory(name);
}
@Override
public void addLocation(Location location) throws DatabaseException {
locationDb.addLocation(location);
}
@Override
public Location getLocation(String name) throws DatabaseException {
return locationDb.getLocation(name);
}
@Override
public Collection<Location> getAllLocations() throws DatabaseException {
return locationDb.getAllLocations();
}
@Override
public void updateLocation(Location location) throws DatabaseException {
locationDb.updateLocation(location);
}
@Override
public void deleteLocation(String name) throws DatabaseException {
locationDb.deleteLocation(name);
}
}
ProductRepository.java
public interface ProductRepository {
public int size() throws DatabaseException;
public void addProduct(Product product) throws DatabaseException;
public Product getProductByEan(long ean) throws DatabaseException;
public Product getProductByHope(int hope) throws DatabaseException;
public Collection<Product> getAllProducts() throws DatabaseException;
public void updateProduct(Product product) throws DatabaseException;
public void deleteProduct(long ean) throws DatabaseException;
}
SQLiteProductRepository.java
public class SQLiteProductRepository implements ProductRepository {
private Connection connection;
private PreparedStatement statement;
String url;
public SQLiteProductRepository(String url) throws DatabaseException {
this.url = url;
}
@Override
public int size() throws DatabaseException {
String query = "SELECT COUNT(ean) AS size FROM product";
int size = 0;
initiateStatement(query);
try {
ResultSet r = statement.executeQuery();
r.next();
size = r.getInt("size");
} catch (SQLException ex) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, ex);
} finally {
closeConnection();
}
return size;
}
@Override
public void addProduct(Product product) throws DatabaseException {
if (product == null) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_NULL);
}
String query = "INSERT INTO product (ean, hope, name) VALUES (?, ?, ?)";
initiateStatement(query);
try {
statement.setLong(1, product.getEan());
statement.setInt(2, product.getHope());
statement.setString(3, product.getName());
statement.execute();
} catch (SQLException e) {
throw new ProductAlreadyExistsException(ErrorMessages.PRODUCT_ALREADY_EXISTS, e);
} finally {
closeConnection();
}
}
@Override
public Product getProductByEan(long ean) throws DatabaseException {
String query = "SELECT * FROM product WHERE ean = ?";
Product product = null;
initiateStatement(query);
try {
statement.setLong(1, ean);
ResultSet result = statement.executeQuery();
while (result.next()) {
product = new Product();
product.setEan(result.getLong("ean"));
product.setHope(result.getInt("hope"));
product.setName(result.getString("name"));
}
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
if (product == null) {
throw new ProductNotFoundException(ErrorMessages.PRODUCT_NOT_FOUND_EAN);
}
return product;
}
@Override
public Product getProductByHope(int hope) throws DatabaseException {
String query = "SELECT * FROM product WHERE hope = ?";
Product product = null;
initiateStatement(query);
try {
statement.setInt(1, hope);
ResultSet result = statement.executeQuery();
while (result.next()) {
product = new Product();
product.setEan(result.getLong("ean"));
product.setHope(result.getInt("hope"));
product.setName(result.getString("name"));
}
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
if (product == null) {
throw new ProductNotFoundException(ErrorMessages.PRODUCT_NOT_FOUND_HOPE);
}
return product;
}
@Override
public Collection<Product> getAllProducts() throws DatabaseException {
String query = "SELECT * FROM product";
Collection<Product> products = new ArrayList<>();
initiateStatement(query);
try {
ResultSet result = statement.executeQuery();
while (result.next()) {
Product product = new Product();
product.setEan(result.getLong("ean"));
product.setHope(result.getInt("hope"));
product.setName(result.getString("name"));
products.add(product);
}
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
return products;
}
@Override
public void updateProduct(Product product) throws DatabaseException {
if (product == null) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_NULL);
}
String query = "UPDATE product SET hope = ?, name = ? WHERE ean = ?";
initiateStatement(query);
try {
statement.setInt(1, product.getHope());
statement.setString(2, product.getName());
statement.setLong(3, product.getEan());
statement.execute();
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
}
@Override
public void deleteProduct(long ean) throws DatabaseException {
String query = "DELETE FROM product WHERE ean = ?";
initiateStatement(query);
try {
statement.setLong(1, ean);
statement.execute();
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
}
private void initiateStatement(String query) throws DatabaseException {
try {
connection = DriverManager.getConnection(url);
statement = connection.prepareStatement(query);
} catch (SQLException ex) {
throw new DatabaseException(ErrorMessages.DATABASE_NOT_FOUND, ex);
}
}
private void closeConnection() throws DatabaseException {
try {
statement.close();
connection.close();
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_CLOSSING_CONNECTION, e);
}
}
}
SQLiteCategoryRepository.java
public class SQLiteCategoryRepository implements CategoryRepository {
private Connection connection;
private PreparedStatement statement;
String url;
public SQLiteCategoryRepository(String url) throws DatabaseException {
this.url = url;
}
@Override
public int size() throws DatabaseException {
String query = "SELECT COUNT(name) AS size FROM category";
int size = 0;
initiateStatement(query);
try {
ResultSet r = statement.executeQuery();
r.next();
size = r.getInt("size");
} catch (SQLException ex) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, ex);
} finally {
closeConnection();
}
return size;
}
@Override
public void addCategory(Category category) throws DatabaseException {
if (category == null) {
throw new IllegalArgumentException(ErrorMessages.CATEGORY_NULL);
}
String query = "INSERT INTO category (name, color) VALUES (?, ?)";
initiateStatement(query);
try {
statement.setString(1, category.getName());
statement.setString(2, category.getColor());
statement.execute();
} catch (SQLException e) {
throw new CategoryAlreadyExistsException(ErrorMessages.CATEGORY_ALREADY_EXISTS, e);
} finally {
closeConnection();
}
}
@Override
public Category getCategory(String name) throws DatabaseException {
if (name == null) {
throw new IllegalArgumentException(ErrorMessages.NAME_NULL);
}
String query = "SELECT * FROM category WHERE name = ?";
Category category = null;
initiateStatement(query);
try {
statement.setString(1, name);
ResultSet result = statement.executeQuery();
while (result.next()) {
category = new Category();
category.setName(result.getString("name"));
category.setColor(result.getString("color"));
}
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
if (category == null) {
throw new CategoryNotFoundException(ErrorMessages.CATEGORY_NOT_FOUND);
}
return category;
}
@Override
public Collection<Category> getAllCategories() throws DatabaseException {
String query = "SELECT * FROM category";
Collection<Category> categories = new ArrayList<>();
initiateStatement(query);
try {
ResultSet result = statement.executeQuery();
while (result.next()) {
Category category = new Category();
category.setName(result.getString("name"));
category.setColor(result.getString("color"));
categories.add(category);
}
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
return categories;
}
@Override
public void updateCategory(Category category) throws DatabaseException {
if (category == null) {
throw new IllegalArgumentException(ErrorMessages.CATEGORY_NULL);
}
String query = "UPDATE category SET color = ? WHERE name = ?";
initiateStatement(query);
try {
statement.setString(1, category.getColor());
statement.setString(2, category.getName());
statement.execute();
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
}
@Override
public void deleteCategory(String name) throws DatabaseException {
if (name == null) {
throw new IllegalArgumentException(ErrorMessages.NAME_NULL);
}
String query = "DELETE FROM category WHERE name = ?";
initiateStatement(query);
try {
statement.setString(1, name);
statement.execute();
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e);
} finally {
closeConnection();
}
}
private void initiateStatement(String query) throws DatabaseException {
try {
connection = DriverManager.getConnection(url);
statement = connection.prepareStatement(query);
} catch (SQLException ex) {
throw new DatabaseException(ErrorMessages.DATABASE_NOT_FOUND, ex);
}
}
private void closeConnection() throws DatabaseException {
try {
statement.close();
connection.close();
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_CLOSSING_CONNECTION, e);
}
}
}
Product.java
public class Product {
public static final int EAN_MIN_LENGTH = 8;
public static final int EAN_MAX_LENGTH = 13;
public static final int HOPE_MIN_LENGTH = 4;
public static final int HOPE_MAX_LENGTH = 8;
public static final int NAME_MIN_LENGTH = 2;
public static final int NAME_MAX_LENGTH = 30;
private long ean;
private int hope;
private String name;
public Product() {
}
public long getEan() {
return ean;
}
public void setEan(long ean) {
if (ean < 0) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_EAN_NEGATIVE);
}
if (InputValidator.isLengthLessThan(ean, EAN_MIN_LENGTH)) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_EAN_MIN_LENGTH);
}
if (InputValidator.isLengthGreaterThan(ean, EAN_MAX_LENGTH)) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_EAN_MAX_LENGTH);
}
this.ean = ean;
}
public int getHope() {
return hope;
}
public void setHope(int hope) {
if (hope < 0) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_HOPE_NEGATIVE);
}
if (InputValidator.isLengthLessThan(hope, HOPE_MIN_LENGTH)) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_HOPE_MIN_LENGTH);
}
if (InputValidator.isLengthGreaterThan(hope, HOPE_MAX_LENGTH)) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_HOPE_MAX_LENGTH);
}
this.hope = hope;
}
public String getName() {
return name;
}
public void setName(String name) {
if (name == null) {
throw new IllegalArgumentException(ErrorMessages.NAME_NULL);
}
name = name.replaceAll(System.getProperty("line.separator"), "");
name = name.replaceAll("\r|\n", "");
name = name.toLowerCase().trim();
if (!name.matches("^[.,a-zA-Z0-9 ]+$")) {
throw new IllegalArgumentException(ErrorMessages.NAME_NOT_ALPHANUMERIC);
}
if (InputValidator.isLengthGreaterThan(name, NAME_MAX_LENGTH)) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_NAME_MAX_LENGTH);
}
if (InputValidator.isLengthLessThan(name, NAME_MIN_LENGTH)) {
throw new IllegalArgumentException(ErrorMessages.PRODUCT_NAME_MIN_LENGTH);
}
this.name = name;
}
@Override
public int hashCode() {
int hash = 5;
hash = 83 * hash + Objects.hashCode(this.ean);
hash = 83 * hash + this.hope;
hash = 83 * hash + Objects.hashCode(this.name);
return hash;
}
@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
if (!(obj instanceof Product)) {
return false;
}
final Product other = (Product) obj;
return Objects.equals(this.ean, other.ean)
&& this.hope == other.hope
&& Objects.equals(this.name, other.name);
}
@Override
public String toString() {
return ean + "\t" + hope + "\t" + name;
}
}
ProductTest.java
public class ProductTest {
private Product chips;
public ProductTest() {
}
long pow(long a, int b) {
if (b == 0) {
return 1;
}
if (b == 1) {
return a;
}
if (isEven(b)) {
return pow(a * a, b / 2); //even a=(a^2)^b/2
} else {
return a * pow(a * a, b / 2); //odd a=a*(a^2)^b/2
}
}
boolean isEven(double num) {
return ((num % 2) == 0);
}
@Before
public void setUp() {
chips = new Product();
chips.setEan(8710398016591L);
chips.setHope(23968);
chips.setName("250GR LAY S BICKY CRISP");
}
@After
public void tearDown() {
chips = null;
}
@Test
public void setEan_Param_is_new_ean() {
long newEan = pow(10L, Product.EAN_MIN_LENGTH - 1);
chips.setEan(newEan);
assertEquals(newEan, chips.getEan());
}
@Test(expected = IllegalArgumentException.class)
public void setEan_IllegalArgumentException_If_param_has_less_than_MIN_digits() {
long newEan = pow(10L, Product.EAN_MIN_LENGTH - 2);
chips.setEan(newEan);
}
@Test(expected = IllegalArgumentException.class)
public void setEan_IllegalArgumentException_If_param_has_more_than_MAX_digits() {
long newEan = pow(10L, Product.EAN_MAX_LENGTH);
chips.setEan(newEan);
}
@Test(expected = IllegalArgumentException.class)
public void setEan_IllegalArgumentException_If_param_is_negative() {
long newEan = pow(-10L, Product.EAN_MIN_LENGTH - 1);
chips.setEan(newEan);
}
@Test
public void setHope_Param_is_new_hope() {
int newHope = (int) Math.pow(10, Product.HOPE_MIN_LENGTH - 1);
chips.setHope(newHope);
assertEquals(newHope, chips.getHope());
}
@Test(expected = IllegalArgumentException.class)
public void setHope_IllegalArgumentException_If_param_has_less_than_MIN_digits() {
int newHope = (int) Math.pow(10, Product.HOPE_MIN_LENGTH - 2);
chips.setHope(newHope);
}
@Test(expected = IllegalArgumentException.class)
public void setHope_IllegalArgumentException_If_param_has_more_than_MAX_digits() {
int newHope = (int) Math.pow(10, Product.HOPE_MAX_LENGTH);
chips.setHope(newHope);
}
@Test(expected = IllegalArgumentException.class)
public void setHope_IllegalArgumentException_If_param_is_negative() {
int newHope = (int) Math.pow(-10, Product.HOPE_MIN_LENGTH - 1);
chips.setHope(newHope);
}
@Test
public void setName_Param_is_converted_without_linebreak_and_set_to_name() {
String newName = "\nnieuwe\n productnaam1\n\r\n";
chips.setName(newName);
assertEquals("nieuwe productnaam1", chips.getName());
}
@Test
public void setName_Param_is_trimmed_and_set_to_name() {
String newName = " nieuwe productnaam1 ";
chips.setName(newName);
assertEquals("nieuwe productnaam1", chips.getName());
}
@Test
public void setName_Param_is_converted_to_lowercase_and_set_to_name() {
String newName = "NiEUwe PrODUcTnaAm1";
chips.setName(newName);
assertEquals("nieuwe productnaam1", chips.getName());
}
@Test(expected = IllegalArgumentException.class)
public void setName_IllegalArgumentException_If_param_is_null() {
String newName = null;
chips.setName(newName);
}
@Test(expected = IllegalArgumentException.class)
public void setName_IllegalArgumentException_If_param_does_not_only_consists_of_alphanumeric_char() {
String newName = "<?php echo lol; ?php>nieuwe productnaam1";
chips.setName(newName);
}
@Test(expected = IllegalArgumentException.class)
public void setName_IllegalArgumentException_If_param_If_param_has_less_than_MIN_characters() {
String newName = new String(new char[Product.NAME_MIN_LENGTH - 1]).replace("0円", "a");
chips.setName(newName);
}
@Test(expected = IllegalArgumentException.class)
public void setName_IllegalArgumentException_If_param_If_param_has_more_than_MAX_characters() {
String newName = new String(new char[Product.NAME_MAX_LENGTH + 1]).replace("0円", "a");
chips.setName(newName);
}
}
Category.java
public class Category {
public static final int NAME_MIN_LENGTH = 2;
public static final int NAME_MAX_LENGTH = 20;
private String name;
private String color;
public Category() {
}
public String getName() {
return name;
}
public void setName(String name) {
if (name == null) {
throw new IllegalArgumentException(ErrorMessages.NAME_NULL);
}
name = name.replaceAll(System.getProperty("line.separator"), "");
name = name.replaceAll("\r|\n", "");
name = name.toLowerCase().trim();
if (!name.matches("^[.,a-zA-Z0-9 ]+$")) {
throw new IllegalArgumentException(ErrorMessages.NAME_NOT_ALPHANUMERIC);
}
if (InputValidator.isLengthLessThan(name, NAME_MIN_LENGTH)) {
throw new IllegalArgumentException(ErrorMessages.CATEGORY_NAME_MIN_LENGTH);
}
if (InputValidator.isLengthGreaterThan(name, NAME_MAX_LENGTH)) {
throw new IllegalArgumentException(ErrorMessages.CATEGORY_NAME_MAX_LENGTH);
}
this.name = name;
}
public String getColor() {
return color;
}
public void setColor(String color) {
if (color == null) {
throw new IllegalArgumentException(ErrorMessages.COLOR_NULL);
}
Pattern pattern = Pattern.compile("^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$");
Matcher matcher = pattern.matcher(color);
if (!matcher.matches()) {
throw new IllegalArgumentException(ErrorMessages.COLOR_NOT_HEXADECIMAL);
}
this.color = color;
}
@Override
public int hashCode() {
int hash = 7;
hash = 23 * hash + Objects.hashCode(this.name);
hash = 23 * hash + Objects.hashCode(this.color);
return hash;
}
@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
if (!(obj instanceof Category)) {
return false;
}
final Category other = (Category) obj;
return (Objects.equals(this.name, other.name))
&& (Objects.equals(this.color, other.color));
}
@Override
public String toString() {
return this.name;
}
}
InputValidator.java
public class InputValidator {
public static boolean isLengthGreaterThan(Object obj, int maxLength) {
return String.valueOf(obj).length() > maxLength;
}
public static boolean isLengthLessThan(Object obj, int minLength) {
return String.valueOf(obj).length() < minLength;
}
}
1 Answer 1
Use try-with-resources
Use try-with-resources when working with resources like database connections, statements, file handlers.
Odd throw-away objects
This kind of code is quite strange:
ResultSet result = statement.executeQuery(); while (result.next()) { product = new Product(); product.setEan(result.getLong("ean")); product.setHope(result.getInt("hope")); product.setName(result.getString("name")); }
That is,
if there are multiple results,
this will overwrite product
in each iteration with a different product.
Effectively, the value of product
will be the last match.
I think you want to replace the while
with an if
.
Too much boilerplate, repeated
This pattern is repeated several times:
@Override public Product getProductByEan(long ean) throws DatabaseException { String query = "SELECT * FROM product WHERE ean = ?"; Product product = null; initiateStatement(query); try { statement.setLong(1, ean); ResultSet result = statement.executeQuery(); while (result.next()) { product = new Product(); product.setEan(result.getLong("ean")); product.setHope(result.getInt("hope")); product.setName(result.getString("name")); } } catch (SQLException e) { throw new DatabaseException(ErrorMessages.DATABASE_FAULT_IN_QUERY, e); } finally { closeConnection(); } if (product == null) { throw new ProductNotFoundException(ErrorMessages.PRODUCT_NOT_FOUND_EAN); } return product; }
A couple of things wrong with it:
- Depends on objects manipulated outside, such as
connection
,statement
- Depends on specific sequence of manipulations that the compiler cannot enforce: call
initiateStatement
first, do the job, callcloseConnection
at the end - Tedious repetition of the try-catch in each such method
It would be better to rewrite this in a way that connection
and statement
are not be member fields, not shared by multiple methods.
You could achieve that, and reduce boilerplate code,
by introducing a query runner interface:
interface QueryRunner<T> {
T query(PreparedStatement statement) throws SQLException;
}
Next, add a utility method that takes a QueryRunner
instance,
and encapsulates the details of managing a connection:
private <T> T query(String query, QueryRunner<T> runner, String message) throws DatabaseException {
try (
Connection connection = DriverManager.getConnection(url);
PreparedStatement statement = connection.prepareStatement(query);
) {
try {
return runner.query(statement);
} catch (SQLException e) {
throw new DatabaseException(message, e);
}
} catch (SQLException e) {
throw new DatabaseException(ErrorMessages.DATABASE_NOT_FOUND, e);
}
}
private <T> T queryForObject(String query, QueryRunner<T> runner, String message) throws DatabaseException {
return query(query, runner, message);
}
Armed with these,
writing the getProductByEan
method,
and the other similar methods can be simplified,
with less boilerplate code:
public Product getProductByEan(long ean) throws DatabaseException {
String query = "SELECT * FROM product WHERE ean = ?";
Product product = queryForObject(query, new QueryRunner<Product>() {
@Override
public Product query(PreparedStatement statement) throws SQLException {
statement.setLong(1, ean);
ResultSet result = statement.executeQuery();
if (result.next()) {
Product product = new Product();
product.setEan(result.getLong("ean"));
product.setHope(result.getInt("hope"));
product.setName(result.getString("name"));
return product;
}
return null;
}
}, ErrorMessages.DATABASE_FAULT_IN_QUERY);
if (product == null) {
throw new ProductNotFoundException(ErrorMessages.PRODUCT_NOT_FOUND_EAN);
}
return product;
}
Using this approach,
connection
and statement
are no longer member variables,
their scopes are minimized, which is much safer.
For add/delete/update methods that don't return an object, you can create another helper method:
private void update(String sql, QueryRunner<Void> runner, String message) throws DatabaseException {
query(sql, runner, message);
}
And in the implementation of the anonymous QueryRunner<Void>
class,
you can return null
.
Make members final
when you can
The url
variable in the repository classes can be final.
In fact, they should be private static final String URL
.
Explore related questions
See similar questions with these tags.