Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I did a task on working with JDBC. And implemented methods based on G… #421

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@
</maven.checkstyle.plugin.configLocation>
</properties>

<dependencies>
<dependency>
<groupId>com.mysql</groupId>
<artifactId>mysql-connector-j</artifactId>
<version>9.0.0</version>
</dependency>

</dependencies>

<build>
<plugins>
<plugin>
Expand Down
27 changes: 27 additions & 0 deletions src/main/java/mate/academy/ConnectionUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package mate.academy;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.util.Properties;
import mate.academy.exception.DataProcessingException;

public class ConnectionUtil {
private static final String DB_URL = "jdbc:mysql://localhost:3306/test";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the DB_URL is not fully defined here. Make sure to provide the complete database URL string.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DB_URL is incomplete, which will cause a SQLException when attempting to connect to the database. Make sure to provide the full JDBC URL in the format jdbc:mysql://<host>:<port>/<database_name>.

private static final Properties DB_PROPERTIES = new Properties();

static {
DB_PROPERTIES.put("user", "root");
DB_PROPERTIES.put("password", "Root1234");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing credentials in code is a security risk. Consider using environment variables or a configuration file to keep sensitive information out of the source code.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoding the database credentials in the source code is not a secure practice. It's better to load these values from an external configuration file or environment variables.


try {
Class.forName("com.mysql.cj.jdbc.Driver");
} catch (ClassNotFoundException e) {
throw new DataProcessingException("Can not load JDBC driver", e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it's good to load the JDBC driver class, consider adding an informative message about the JDBC driver that was not found to make the exception more helpful.

}
}

public static Connection getConnection() throws SQLException {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use schema's name in the DB_URL as you are configuring it while establishing a connection with the database. Ensure that the schema is specified in the URL if necessary, or configured through the DB_PROPERTIES.

return DriverManager.getConnection(DB_URL, DB_PROPERTIES);
}
Comment on lines +25 to +27

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the best practices, it's recommended to use try-with-resources for getting the Connection to ensure that it is closed properly. This can prevent potential resource leaks.

}
26 changes: 26 additions & 0 deletions src/main/java/mate/academy/Main.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,33 @@
package mate.academy;

import java.math.BigDecimal;
import java.util.List;
import java.util.Optional;
import mate.academy.dao.BookDao;
import mate.academy.lib.Injector;
import mate.academy.model.Book;

public class Main {
private static final Injector injector = Injector.getInstance("mate.academy");

public static void main(String[] args) {

BookDao bookDao = (BookDao) injector.getInstance(BookDao.class);

Book book = new Book(1L, "Java Programming", new BigDecimal("29.99"));
Book book1 = new Book(2L, "Java", new BigDecimal("30.25"));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not recommended to set the ID for the Book object manually when creating a new Book. The ID should be auto-generated by the database to ensure uniqueness and to prevent conflicts. Consider removing the ID parameter from the Book constructor and letting the database handle the ID generation.

bookDao.create(book);
bookDao.create(book1);

Optional<Book> retrievedBook = bookDao.findById(book.getId());
System.out.println(retrievedBook);

book.setTitle("Advanced Java Programming");
bookDao.update(book);

List<Book> allBooks = bookDao.findAll();
allBooks.forEach(System.out::println);

bookDao.deleteById(book.getId());
}
}
20 changes: 20 additions & 0 deletions src/main/java/mate/academy/dao/BookDao.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package mate.academy.dao;

import java.util.List;
import java.util.Optional;
import mate.academy.lib.Dao;
import mate.academy.model.Book;

@Dao
public interface BookDao {

Book create(Book book);

Optional<Book> findById(Long id);

List<Book> findAll();

Book update(Book book);

boolean deleteById(Long id);
}
111 changes: 111 additions & 0 deletions src/main/java/mate/academy/dao/BookDaoImpl.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package mate.academy.dao;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import mate.academy.ConnectionUtil;
import mate.academy.exception.DataProcessingException;
import mate.academy.lib.Dao;
import mate.academy.model.Book;

@Dao
public class BookDaoImpl implements BookDao {
private static final String CREATE_BOOK_QUERY
= "INSERT INTO books (title, price) VALUES (?, ?)";
private static final String FIND_BY_ID_QUERY = "SELECT * FROM books WHERE id = ?";
private static final String FIND_ALL_QUERY = "SELECT * FROM books";
private static final String UPDATE_BOOK_QUERY
= "UPDATE books SET title = ?, price = ? WHERE id = ?";
private static final String DELETE_BY_ID_QUERY = "DELETE FROM books WHERE id = ?";
Comment on lines +18 to +24

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to use uppercase for SQL keywords in your queries to follow SQL style best practices.


@Override
public Book create(Book book) {
try (Connection connection = ConnectionUtil.getConnection();
PreparedStatement statement =
connection.prepareStatement(CREATE_BOOK_QUERY,
Statement.RETURN_GENERATED_KEYS)) {

statement.setString(1, book.getTitle());
statement.setBigDecimal(2, book.getPrice());
statement.executeUpdate();

ResultSet resultSet = statement.getGeneratedKeys();
if (resultSet.next()) {
book.setId(resultSet.getLong(1));
}
return book;

} catch (SQLException e) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use informative exception messages. Instead of 'Failed to insert book into DB', include the book details to make the exception message more informative.

throw new DataProcessingException("Failed to insert book " + book + " into DB", e);
}
}
Comment on lines +26 to +46

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use PreparedStatement over Statement for the CREATE_BOOK_QUERY. Also, consider using Statement.RETURN_GENERATED_KEYS only in the create statement, which you are already doing correctly, but remember that it's not needed in other methods.


@Override
public Optional<Book> findById(Long id) {
try (Connection connection = ConnectionUtil.getConnection();
PreparedStatement statement = connection.prepareStatement(FIND_BY_ID_QUERY)) {
statement.setLong(1, id);
ResultSet resultSet = statement.executeQuery();
if (resultSet.next()) {
Book book = mapToBook(resultSet);
return Optional.of(book);
}
} catch (SQLException e) {
throw new DataProcessingException("Failed to retrieve book with id " + id, e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use informative exception messages. Instead of 'Failed to retrieve book with id', include the book id to make the exception message more informative.

}
return Optional.empty();
Comment on lines +49 to +61

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use if or while with resultSet.next() to avoid a potential NullPointerException if the result set is empty. You are doing this correctly here, but keep it in mind for consistency across all methods.

}

@Override
public List<Book> findAll() {
List<Book> books = new ArrayList<>();
try (Connection connection = ConnectionUtil.getConnection();
PreparedStatement statement = connection.prepareStatement(FIND_ALL_QUERY);
ResultSet resultSet = statement.executeQuery()) {
while (resultSet.next()) {
books.add(mapToBook(resultSet));
}
} catch (SQLException e) {
throw new DataProcessingException("Can't get all books from DB", e);
}

return books;
}
Comment on lines +65 to +78

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good that you used PreparedStatement over Statement in the findAll() method, following the best practice.


@Override
public Book update(Book book) {
try (Connection connection = ConnectionUtil.getConnection();
PreparedStatement statement = connection.prepareStatement(UPDATE_BOOK_QUERY)) {
statement.setString(1, book.getTitle());
statement.setBigDecimal(2, book.getPrice());
statement.setLong(3, book.getId());
statement.executeUpdate();
return book;
} catch (SQLException e) {
throw new DataProcessingException("Can't update book " + book, e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use informative exception messages. Instead of 'Can't update book', include the book details to make the exception message more informative.

}
}

@Override
public boolean deleteById(Long id) {
try (Connection connection = ConnectionUtil.getConnection();
PreparedStatement statement = connection.prepareStatement(DELETE_BY_ID_QUERY)) {
statement.setLong(1, id);
int rowsAffected = statement.executeUpdate();
return rowsAffected > 0;
} catch (SQLException e) {
throw new DataProcessingException("Failed to delete book with id " + id, e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use informative exception messages. Instead of 'Failed to delete book with id', include the book id to make the exception message more informative.

}
Comment on lines +95 to +103

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't return true all the time in the deleteById method. You're correctly returning a boolean value depending on the preparedStatement.executeUpdate() result, which is a good practice.

}

Book mapToBook(ResultSet resultSet) throws SQLException {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving the logic for mapping the ResultSet to a Book object into a separate private method to avoid code duplication as suggested in the checklist. This method should be used in findById and findAll methods.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you are converting ResultSet to Book, it's better to use either setters or a constructor consistently rather than both. In this case, you are using the constructor to set title and price, and then using a setter to set the id. Choose one approach for consistency.

return new Book(resultSet.getLong("id"),
resultSet.getString("title"),
resultSet.getBigDecimal("price"));
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When converting ResultSet to Book, it's good practice to create an object using a constructor or setters, but not both. Here, you are using the constructor, which is consistent and recommended.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package mate.academy.exception;

public class DataProcessingException extends RuntimeException {
public DataProcessingException(String message, Throwable cause) {
super(message, cause);
}
}
48 changes: 48 additions & 0 deletions src/main/java/mate/academy/model/Book.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package mate.academy.model;

import java.math.BigDecimal;

public class Book {
private Long id;
private String title;
private BigDecimal price;

public Book(Long id, String title, BigDecimal price) {
this.id = id;
this.title = title;
this.price = price;
}

public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}

public String getTitle() {
return title;
}

public void setTitle(String title) {
this.title = title;
}

public BigDecimal getPrice() {
return price;
}

public void setPrice(BigDecimal price) {
this.price = price;
}

@Override
public String toString() {
return "Book{"
+ "id=" + id
+ ", title='" + title + '\''
+ ", price=" + price
+ '}';
}
}
7 changes: 7 additions & 0 deletions src/main/resourses/init_db.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE books (
id INT NOT NULL AUTO_INCREMENT,
title VARCHAR(255) NOT NULL,
price DECIMAL(10, 2) NOT NULL,
PRIMARY KEY (id)
);

Loading