-
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
create hw-jdbc #52
base: main
Are you sure you want to change the base?
create hw-jdbc #52
Conversation
src/main/java/mate/academy/Main.java
Outdated
|
||
bookDao.create(book); | ||
|
||
System.out.println(book); | ||
|
||
book.setPrice(BigDecimal.valueOf(500)); | ||
System.out.println(bookDao.update(book)); | ||
|
||
System.out.println(bookDao.findById(book.getId())); | ||
|
||
System.out.println(bookDao.findAll()); | ||
|
||
System.out.println(bookDao.deleteById(book.getId())); |
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.
Maybe empty lines is not good? (But it just my opinion)
while (resultSet.next()) { | ||
book = getBookFromResultSet(resultSet); | ||
} |
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.
while in getting one result
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.
not resolved
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.
Don't agree with you in this case. I think I need this while loop for iterating data in resultSet.
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.
Why you need to iterate if you get one value from result set?
"SELECT * FROM books WHERE id = ?" returns one row
you don`t need to iterate on something
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.
Good point. The book's id is unique. When you try to find it by id you can get only one row
Of course, while
works, in this case, the same as if
.
but from the spectator's perspective, it could trick.
updateStatement.setLong(3, book.getId()); | ||
updateStatement.setString(1, book.getTitle()); | ||
updateStatement.setBigDecimal(2, book.getPrice()); |
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.
maybe better use constants and regroup? (FIRST_PARAMETER...)
"id=" + id + | ||
", title='" + title + '\'' + | ||
", price=" + price + | ||
'}'; |
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.
did you install maven plugin in pom xml?
src/main/resources/init_db.sql
Outdated
id bigint(10) NOT NULL AUTO_INCREMENT, | ||
title varchar(225) NOT NULL, | ||
price bigint(10), |
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.
in checklist there is option to style .sql like this:
id bigint(10) NOT NULL AUTO_INCREMENT, | |
title varchar(225) NOT NULL, | |
price bigint(10), | |
id BIGINT(10) NOT NULL AUTO_INCREMENT, | |
title VARCHAR(225) NOT NULL, | |
price BIGINT(10), |
while (resultSet.next()) { | ||
book = getBookFromResultSet(resultSet); | ||
} |
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.
not resolved
while (resultSet.next()) { | ||
book = getBookFromResultSet(resultSet); | ||
} |
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.
Why you need to iterate if you get one value from result set?
"SELECT * FROM books WHERE id = ?" returns one row
you don`t need to iterate on something
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.
Good solution, let's improve it :)
private static final String INSERT_REQUEST = "INSERT INTO books(title, price) VALUES(?, ?)"; | ||
private static final String REQUEST_BOOK_FROM_DB = "SELECT * FROM books WHERE id = ?"; | ||
private static final String GET_ALL_REQUEST_BOOK = "SELECT * FROM books"; | ||
private static final String UPDATE_REQUEST = "UPDATE books SET title = ?, price = ? WHERE id = ?"; | ||
private static final String DELETE_BOOK_REQUEST = "DELETE FROM books WHERE 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.
while (resultSet.next()) { | ||
book = getBookFromResultSet(resultSet); | ||
} |
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.
Good point. The book's id is unique. When you try to find it by id you can get only one row
Of course, while
works, in this case, the same as if
.
but from the spectator's perspective, it could trick.
updateStatement.setLong(ID_INDEX, book.getId()); | ||
updateStatement.setString(TITLE_INDEX, book.getTitle()); | ||
updateStatement.setBigDecimal(PRICE_INDEX, book.getPrice()); |
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.
Does it suit your query? I mean, Does the id have first place in the query and etc?
deleteStatement.setLong(1, id); | ||
return deleteStatement.executeUpdate() > 0; | ||
} catch (SQLException e) { | ||
throw new DataProcessingException("Can't delete book by id " + id, e); |
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.
throw new DataProcessingException("Can't delete book by id " + id, e); | |
throw new DataProcessingException("Can't delete book by id = " + id, e); |
No description provided.