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

implemented dao and model for book #402

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
8 changes: 8 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@
</maven.checkstyle.plugin.configLocation>
</properties>

<dependencies>
<dependency>
<groupId>mysql</groupId>
<artifactId>mysql-connector-java</artifactId>
<version>8.0.30</version>
</dependency>
</dependencies>

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

import java.math.BigDecimal;
import java.util.List;
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);
List<Book> books = List.of(
new Book(null, "Book1", new BigDecimal(100)),
new Book(null, "Book2", new BigDecimal(150)),
new Book(null, "Book3", new BigDecimal(50))
Comment on lines +15 to +17

Choose a reason for hiding this comment

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

Creating Book objects with null id is a bit misleading since it's not immediately clear why the id is set to null. If the id is meant to be generated by the database, then it would be better to have a constructor that does not require an id parameter at all, which would make the intent clearer.

);

books.forEach(bookDao::create);
System.out.println("Created books:");
books.forEach(System.out::println);
System.out.println();

Long bookId = 2L;
System.out.println("Book with id = [" + bookId + "]");
bookDao.findById(bookId).ifPresent(System.out::println);
System.out.println();

System.out.println("All books in the table:");
bookDao.findAll().forEach(System.out::println);
System.out.println();

Book anyBook = books.stream().findAny().orElseThrow();

Choose a reason for hiding this comment

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

Using findAny() without any filtering or additional context is non-deterministic and may lead to inconsistent behavior during different runs of the application. If the goal is to update a random book, consider adding a comment to clarify this choice. Otherwise, use a more deterministic approach, such as selecting the first element or using a specific criterion for selection.

anyBook.setTitle("Updated title");
Book anyUpdatedBook = bookDao.update(anyBook);
System.out.println("Updated book:");
System.out.println(anyUpdatedBook);
System.out.println();

boolean isDeleted = bookDao.deleteById(anyUpdatedBook.getId());
System.out.println("Is book with id = ["
+ anyUpdatedBook.getId() + "] deleted = ["
+ isDeleted + "]");
}
}
17 changes: 17 additions & 0 deletions src/main/java/mate/academy/dao/BookDao.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package mate.academy.dao;

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

public interface BookDao {
Book create(Book book);

Optional<Book> findById(Long id);

List<Book> findAll();

Book update(Book book);

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

import java.math.BigDecimal;
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.dao.BookDao;
import mate.academy.exceptions.DataProcessingException;
import mate.academy.lib.Dao;
import mate.academy.model.Book;
import mate.academy.util.ConnectionUtil;

@Dao
public class BookDaoImpl implements BookDao {
@Override
public Book create(Book book) {
String query = "INSERT INTO books(title, price) VALUES (?, ?)";
try (Connection connection = ConnectionUtil.getConnection()) {
PreparedStatement statement =
connection.prepareStatement(query, Statement.RETURN_GENERATED_KEYS);
statement.setString(1, book.getTitle());

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 static queries with no parameters in findAll() method. Although prepareStatement is used here, it's a good practice to be consistent across all methods.

statement.setBigDecimal(2, book.getPrice());
int affectedRows = statement.executeUpdate();
Comment on lines +22 to +28

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 retrieving data from the ResultSet into a separate private method as mentioned in the checklist to avoid code duplication.

if (affectedRows < 1) {
throw new DataProcessingException("Expected to affect at least one row, "
+ "but got affected = [" + affectedRows + "]");
}
ResultSet resultSet = statement.getGeneratedKeys();
if (resultSet.next()) {
Long id = resultSet.getLong(1);
book.setId(id);

Choose a reason for hiding this comment

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

Instead of using resultSet.getLong(1), it's better to use resultSet.getObject(1, Long.class) to handle possible null values correctly.

}
} catch (SQLException e) {
throw new DataProcessingException("Failed to create a book = [" + book + "]", e);
}
return book;
}

@Override
public Optional<Book> findById(Long id) {
String query = "SELECT * FROM books WHERE id = ?";

Choose a reason for hiding this comment

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

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

try (Connection connection = ConnectionUtil.getConnection()) {
PreparedStatement statement = connection.prepareStatement(query);
statement.setLong(1, id);
ResultSet resultSet = statement.executeQuery();

Choose a reason for hiding this comment

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

You should use if or while with resultSet.next() to avoid potential NullPointerException when the result set is empty.

if (resultSet.next()) {
Book book = getBookFromResultSet(resultSet);
return Optional.of(book);
}
} catch (SQLException e) {
throw new DataProcessingException("Failed to find a book with id = [" + id + "]", e);
}
return Optional.empty();
}

@Override
public List<Book> findAll() {
String query = "SELECT * FROM books";
List<Book> books = new ArrayList<>();
try (Connection connection = ConnectionUtil.getConnection()) {
PreparedStatement statement = connection.prepareStatement(query);

Choose a reason for hiding this comment

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

Even though this is a static query with no parameters, it's better to use PreparedStatement over Statement for consistency and slight performance gains.

ResultSet resultSet = statement.executeQuery();
while (resultSet.next()) {
Book book = getBookFromResultSet(resultSet);
books.add(book);
}
} catch (SQLException e) {
throw new DataProcessingException("Failed to fetch all books", e);
}
return books;
}

@Override
public Book update(Book book) {
String query = "UPDATE books SET title = ?, price = ? WHERE id = ?";
try (Connection connection = ConnectionUtil.getConnection()) {
PreparedStatement statement = connection.prepareStatement(query);

Choose a reason for hiding this comment

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

The SQL keywords in your query should be uppercase to maintain readability and adhere to SQL style best practices.

statement.setString(1, book.getTitle());
statement.setBigDecimal(2, book.getPrice());
statement.setLong(3, book.getId());
int affectedRows = statement.executeUpdate();
if (affectedRows < 1) {
throw new DataProcessingException("Expected to affect at least one row, "
+ "but got affected = [" + affectedRows + "]");
}
} catch (SQLException e) {
throw new DataProcessingException("Failed to update a book = [" + book + "]", e);
}
return book;
}

@Override
public boolean deleteById(Long id) {
String query = "DELETE FROM books WHERE id = ?";
try (Connection connection = ConnectionUtil.getConnection()) {
PreparedStatement statement = connection.prepareStatement(query);
statement.setLong(1, id);
return statement.executeUpdate() > 0;

Choose a reason for hiding this comment

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

Instead of returning true all the time, let's return a boolean value depending on the preparedStatement.executeUpdate() result. This makes the method's behavior more accurate to whether the deletion actually happened.

} catch (SQLException e) {
throw new DataProcessingException("Failed to delete a book "
+ "with id = [" + id + "]", e);
}
}

private Book getBookFromResultSet(ResultSet resultSet) throws SQLException {
Long id = resultSet.getObject("id", Long.class);

Choose a reason for hiding this comment

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

Use resultSet.getObject("id", Long.class) to handle possible null values correctly.

String title = resultSet.getString("title");
BigDecimal price = resultSet.getBigDecimal("price");
return new Book(id, title, price);
}
}
11 changes: 11 additions & 0 deletions src/main/java/mate/academy/exceptions/DataProcessingException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package mate.academy.exceptions;

public class DataProcessingException extends RuntimeException {
public DataProcessingException(String message, Throwable e) {
super(message, e);
}

public DataProcessingException(String message) {
super(message);
}
}
3 changes: 3 additions & 0 deletions src/main/java/mate/academy/lib/Dao.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package mate.academy.lib;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface Dao {
}
47 changes: 47 additions & 0 deletions src/main/java/mate/academy/model/Book.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
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 + '}';
}
}
30 changes: 30 additions & 0 deletions src/main/java/mate/academy/util/ConnectionUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package mate.academy.util;

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

public class ConnectionUtil {
private static final Properties DB_PROPERTIES;
private static final String URL = "jdbc:mysql://localhost:3306/mate";

Choose a reason for hiding this comment

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

The URL seems to be incomplete. It should specify the database that you are connecting to. For example, 'jdbc:mysql://localhost:3306/your_database_name'.

private static final String USER = "root";
private static final String PASSWORD = "password";
Comment on lines +12 to +13

Choose a reason for hiding this comment

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

Storing the database username and password as plain text in the source code is not secure. Consider using environment variables or a configuration file to store sensitive information securely.

private static final String DRIVER = "com.mysql.cj.jdbc.Driver";

static {
DB_PROPERTIES = new Properties();
DB_PROPERTIES.put("user", USER);
DB_PROPERTIES.put("password", PASSWORD);
try {
Class.forName(DRIVER);
} catch (ClassNotFoundException e) {
throw new DataProcessingException("Failed to load JDBC driver", e);
}
}

public static Connection getConnection() throws SQLException {
return DriverManager.getConnection(URL, DB_PROPERTIES);
}
Comment on lines +27 to +29

Choose a reason for hiding this comment

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

It is good practice to handle the SQLException that might be thrown from getConnection method and convert it to a more informative custom exception, similar to what you have done in the static block with ClassNotFoundException.

}
5 changes: 5 additions & 0 deletions src/main/resources/init_db.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CREATE TABLE books(
id bigint AUTO_INCREMENT PRIMARY KEY,
title varchar(255) NOT NULL,
price DECIMAL(10, 2) NOT NULL
);
Loading