-
Notifications
You must be signed in to change notification settings - Fork 438
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
a8bf40b
bf4d1ab
2caaacf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
private static final Properties DB_PROPERTIES = new Properties(); | ||
|
||
static { | ||
DB_PROPERTIES.put("user", "root"); | ||
DB_PROPERTIES.put("password", "Root1234"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not use schema's name in the |
||
return DriverManager.getConnection(DB_URL, DB_PROPERTIES); | ||
} | ||
Comment on lines
+25
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} |
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(); | ||
book.setTitle("Java Programming"); | ||
book.setPrice(new BigDecimal("29.99")); | ||
bookDao.create(book); | ||
|
||
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()); | ||
} | ||
} |
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); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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("Can't insert book into DB", e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The exception message when inserting a book should be more informative. Include the book object that failed to be inserted, for example, 'Can't insert book ' + book + ' into DB'. |
||
} | ||
} | ||
|
||
@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 = new Book(); | ||
book.setId(resultSet.getLong("id")); | ||
book.setTitle(resultSet.getString("title")); | ||
book.setPrice(resultSet.getBigDecimal("price")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are using setters to set the properties of the Book object after using the default constructor. According to the checklist, it is recommended to use either setters or the constructor to set all properties, but not both. Consider creating a constructor in the Book class that takes all the properties as parameters, or use the default constructor and setters consistently. |
||
return Optional.of(book); | ||
} | ||
} catch (SQLException e) { | ||
throw new DataProcessingException("Can't get book by ID", e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The exception message is not informative enough. According to the checklist, it's recommended to include the id of the book that failed to be retrieved. For example, 'Can't get book by id ' + id. |
||
} | ||
return Optional.empty(); | ||
} | ||
|
||
@Override | ||
public List<Book> findAll() { | ||
List<Book> books = new ArrayList<>(); | ||
try (Connection connection = ConnectionUtil.getConnection(); | ||
Statement statement = connection.createStatement()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use PreparedStatement instead of Statement for the 'findAll' method, even if the query does not have parameters. This is a best practice for several reasons, including better performance and security against SQL injection attacks. |
||
ResultSet resultSet = statement.executeQuery(FIND_ALL_QUERY); | ||
while (resultSet.next()) { | ||
Book book = new Book(); | ||
book.setId(resultSet.getLong("id")); | ||
book.setTitle(resultSet.getString("title")); | ||
book.setPrice(resultSet.getBigDecimal("price")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code duplicates the logic of converting a ResultSet to a Book object which is present in both 'findById' and 'findAll' methods. To avoid code duplication, you could move this logic into a separate private method that constructs a Book object from a ResultSet and then call this method in both 'findById' and 'findAll'. |
||
books.add(book); | ||
} | ||
} catch (SQLException e) { | ||
throw new DataProcessingException("Can't get all books from DB", e); | ||
} | ||
|
||
return books; | ||
} | ||
|
||
@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", e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The exception message when updating a book should also be more informative. Include the book object that failed to be updated, for example, 'Can't update book ' + book. |
||
} | ||
} | ||
|
||
@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("Can't delete book by ID", e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, the exception message when deleting a book is not informative enough. Include the id of the book that failed to be deleted, for example, 'Can't delete book by ID ' + id. |
||
} | ||
} | ||
} |
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); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package mate.academy.model; | ||
|
||
import java.math.BigDecimal; | ||
|
||
public class Book { | ||
private Long id; | ||
private String title; | ||
private BigDecimal 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 | ||
+ '}'; | ||
} | ||
} |
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) | ||
); | ||
|
There was a problem hiding this comment.
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.