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

resolve jdbc-intro #356

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

YouraSoliar
Copy link

No description provided.

Copy link

@okuzan okuzan left a comment

Choose a reason for hiding this comment

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

@YouraSoliar YouraSoliar requested a review from okuzan May 25, 2024 07:09
statement.setBigDecimal(2, book.getPrice());
statement.setLong(3, book.getId());

int rowsAffected = statement.executeUpdate();

Choose a reason for hiding this comment

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

Suggested change
int rowsAffected = statement.executeUpdate();
if (affectedRows < 1) {.....

Copy link

Choose a reason for hiding this comment

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

not fixed, throw exception in this situation, please don't ignore comments - if you disagree or don't understand them, ask in chat on the platform

System.out.println("Rows updated: " + rowsAffected);
return book;
} catch (SQLException e) {
throw new DataProcessingException("Can not find all books", e);

Choose a reason for hiding this comment

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

wrong message

System.out.println("Rows deleted: " + rowsAffected);
return rowsAffected > 0;
} catch (SQLException e) {
throw new DataProcessingException("Can not find all books", e);

Choose a reason for hiding this comment

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

the same


statement.setLong(1, id);

int rowsAffected = statement.executeUpdate();

Choose a reason for hiding this comment

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

Suggested change
int rowsAffected = statement.executeUpdate();
return statement.executeUpdate() > 0;

try (Connection connection = ConnectionUtil.getConnection();
PreparedStatement statement = connection.prepareStatement(sql)) {

int rowsAffected = statement.executeUpdate();

Choose a reason for hiding this comment

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

the same

System.out.println("Deleted " + rowsAffected + " rows from table books");
return rowsAffected > 0;
} catch (SQLException e) {
throw new DataProcessingException("Can not delete all books", e);

Choose a reason for hiding this comment

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

the same

import java.util.Properties;
import mate.academy.exception.DataProcessingException;

public class ConnectionUtil {

Choose a reason for hiding this comment

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

it's better to create util package and replace it there


private static void testCreate() {
System.out.println("\nCREATE");
Book bookHarry = new Book("Harry", new BigDecimal(1222));

Choose a reason for hiding this comment

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

Let's create List books = List.of(new Book(.... and use it in books.stream().map(...


private static void testFindAll() {
System.out.println("\nFIND ALL");
for (var book : service.findAllBooks()) {

Choose a reason for hiding this comment

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

service.findAllBooks().stream().forEach(System.out::println);

BookDao bookDao = (BookDao) injector.getInstance(BookDao.class);
service = new BookServiceImpl(bookDao);

testCreate();

Choose a reason for hiding this comment

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

rename methods or don't create separate methods

statement.setBigDecimal(2, book.getPrice());
statement.setLong(3, book.getId());

int rowsAffected = statement.executeUpdate();
Copy link

Choose a reason for hiding this comment

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

not fixed, throw exception in this situation, please don't ignore comments - if you disagree or don't understand them, ask in chat on the platform

statement.setLong(3, book.getId());

int rowsAffected = statement.executeUpdate();
System.out.println("Rows updated: " + rowsAffected);
Copy link

Choose a reason for hiding this comment

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

remove this

@YouraSoliar YouraSoliar requested a review from okuzan May 26, 2024 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants