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

[JDBC 라이브러리 구현하기 - 1단계] 달리(김동연) 미션 제출합니다. #306

Merged
merged 11 commits into from
Sep 30, 2023

Conversation

waterricecake
Copy link

안녕하세요 JJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJ

이번 미션은 레벨 2 때 사용했던 JDBC template을 생각하면서 해봤습니다.

제가 일찍 미션시작을 눌러버려서 커밋이 여러개 올라가게 되었는데 제가 이번 미션에서 변경한 클래스는
app/src/main/java/com/techcourse/dao/UserDao.java
jdbc/src/main/java/org/springframework/jdbc/core/JdbcTemplate.java
jdbc/src/main/java/org/springframework/jdbc/core/RowMapper.java
이렇게 3가지 클래스입니다.

이번 미션에서 해보고 싶었는데 실패한 부분이 JDBC의 queryForObject에서 RowMapper<T> 대신 반환할 Class<T>를 받아서 실행하게 구현해보고 싶었습니다. 계획은

  1. query를 통해 찾을 resultSet의 column들의 타입들 알아낸다.
  2. 해당 타입들로 생성될 Constructor를 reflection을 통해 찾아온다.
  3. 해당 타입들을 resultSet에서 가져와서 객체를 생성하고 반환한다.
    까지가 계획이었습니다. 결과는 누구나 거창한 계획은 있다 맞기 전까지는...
    해당 타입들을 resultSet에서 getColumnTypeName을 통해 알 수는 있었지만 이 값들이 다 String으로 반환되서(진짜 이름만 알려줌) 이를 다시 Java class 타입으로 바꿀 방법을 모르겠더라고요... 혹시 좋은 방법이 있는지 여쭙고 싶습니다(물론 원시타입 정도는 일일이 넣어줄수는 있겠지만 뭔가 더 까리한 방법이 없을까 하여 여쭙습니다.)

@waterricecake waterricecake self-assigned this Sep 28, 2023
Copy link

@sosow0212 sosow0212 left a comment

Choose a reason for hiding this comment

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

안녕하세요 달리 ~
달리의 생각이 궁금해서 몇 가지 코멘트를 남겨보았습니다.
그럼 추석 잘 보내시고 다음에 봐요~

private final DataSource dataSource;
private final JdbcTemplate jdbcTemplate;

private final RowMapper<User> userRowMapper = (rs) -> new User(

Choose a reason for hiding this comment

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

static final이 아닌 final로 사용하신 이유가 있을까요? 달리의 생각이 궁금합니다!

Copy link
Author

@waterricecake waterricecake Sep 29, 2023

Choose a reason for hiding this comment

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

static 으로 했어야했는데 잊고있었습니다. 감사합니다 ㅎㅎ
cdcc90a

} finally {
try {
if (Objects.nonNull(resultSet)) {
resultSet.close();

Choose a reason for hiding this comment

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

resultSet close를 잘 해주셨네요~
한 가지 궁금한게 있는데요 try-with-resouces가 아닌 요기다 해주신 이유가 있을까요?

Copy link
Author

@waterricecake waterricecake Sep 29, 2023

Choose a reason for hiding this comment

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

ResultSetPreparedStatement.executeQuery() 통해 반환하기전 PreparedStatement.setObject()를 통해 PreparedStatement의 내부 parameter들을 설정해줘야 합니다. 이때 try-resource 내부에서 PreparedStatement.setObject()을 할 경우 Declaration, final or effectively final variable expected과 같은 이유로 set을 사용할 수 없게 됩니다. 때문에 두가지 방법을 생각하였는데

  1. String sql에 필요한 값들을 try문 전에 String.format을 통해 넣어준다.
  2. ResultSet만 따로 final을 통해 닫아준다.
    였습니다.
    이 경우 1번보다 2번이 좀더 여러 상황에 유연하게 대처할 수 있다는 생각이 들게되어서 선택하게 되었습니다.

Choose a reason for hiding this comment

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

ㅎㅎ 그렇군요 다음 단계에서 조금 더 고민을 해봅시다!

if (resultSet.next()) {
return rowMapper.rowMap(resultSet);
}
throw new NoSuchElementException();

Choose a reason for hiding this comment

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

이건 개인적으로 궁금한 부분인데요
값이 없나면 예외를 던지시는데 null을 던질 수도 있지만 예외를 날리는 것의 장점은 무엇이라고 생각하시나요?

Copy link
Author

@waterricecake waterricecake Sep 29, 2023

Choose a reason for hiding this comment

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

찾는 값이 없다라는 것은 클라이언트 측이 잘못된 값을 요청한 것이기 때문에 클라이언트 측에게 오류 메세지를 던지는 것이 맞지 않나라는 생각을 하게 되었고, 이 경우 사용자는 편하게 HandlerException을 통해 잘못된 요청입니다라는 결과를 보낼 수 있는 것이 장점이라 생각합니다.

null값을 반환할 경우 Optional을 통해 분기점을 만들 수 있어 여러 상황에 유기적으로 대처할 수 있지만, NPE는 최대한 피하는 것이 좋지 않을까 라는 생각도 들게 되어 예외를 던졌습니다.

Choose a reason for hiding this comment

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

오 달리의 의견 좋습니다~

i++;
}
pstmt.executeUpdate();
return 1;

Choose a reason for hiding this comment

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

오.. 현재 DAO에선 반환 값이 필요 없는 것 같아요.
어떤 의도로 이 1과 0을 반환하셨나요? 항상 1 혹은 0 값만 반환해주게 되는 것일까요?
만약 달리만의 뜻이 있다면 상수로 의미를 부여해주면 더 좋을 것 같습니다 ㅎㅎ

Copy link
Author

@waterricecake waterricecake Sep 29, 2023

Choose a reason for hiding this comment

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

물론 현재 상태에서는 UPDATE한 대상이 0과 1일 뿐이어서 성공 실패 정도로 반환되겠지만 원래 원했던것은 해당 쿼리가 영향을 준 db의 row들을 반환하고 싶었습니다. 이를 통해 사용자에게 영향이 간 데이터들은 이 정도이다를 알려줄려고 했습니다. insert의 경우는 사용자가 명확하게 생성되는 정보는 확실히 알수있고 select의 경우 가져오는 값을 알 수 있기에 상관이 없지만 update의 경우 where문을 통해 여러 데이터를 손댈수도 있기 때문에 이에대한 응답으로 알려줘야 한다고 생각하게 되었습니다.

근데 찾다보니까 에초에 PreparedStatement.executeUpdate()가 반환하는 값이 제가 원하는 값이더라고요. 그래서 그 값을 반환하고 그 외에는 예외처리를 해주었습니다.
a770057

Copy link

@sosow0212 sosow0212 left a comment

Choose a reason for hiding this comment

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

달리의 의견 잘 들었습니다~
다음 단계에서 더욱 개선해보면서 대화 나눠보면 좋을 것 같아요!
고생하셨습니다 ㅎㅎ 연휴 잘 보내세요~

@sosow0212 sosow0212 merged commit d2b3fd5 into woowacourse:waterricecake Sep 30, 2023
1 check failed
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