[JDBC 라이브러리 구현하기 - 1단계] 파워(송재백) 미션 제출합니다. #272
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
안녕하세요 파워! |
LJW25
approved these changes
Sep 28, 2023
Comment on lines
14
to
+15
| private final DataSource dataSource; | ||
| private final JdbcTemplate jdbcTemplate; |
There was a problem hiding this comment.
DataSource를 더이상 사용하지 않는 것으로 보이는데, 가지고 있을 이유가 있을까요?
| } | ||
|
|
||
| public void update(final User user) { | ||
| // todo | ||
| String sql = "update users set id = ?, account = ?, password = ?, email = ? where id = ?"; |
There was a problem hiding this comment.
#컨벤션 아래 메소드에도 동일하게 적용되는 사항인데, sql 변수에 대해서 접근제어자와 타입을 통일해 주는건 어떨까요?
Comment on lines
+48
to
+52
| return jdbcTemplate.query(sql, rs -> new User( | ||
| rs.getLong(1), | ||
| rs.getString(2), | ||
| rs.getString(3), | ||
| rs.getString(4))); |
Comment on lines
+44
to
+65
| } finally { | ||
| try { | ||
| if (rs != null) { | ||
| rs.close(); | ||
| } | ||
| } catch (SQLException ignored) { | ||
| } | ||
|
|
||
| try { | ||
| if (pstmt != null) { | ||
| pstmt.close(); | ||
| } | ||
| } catch (SQLException ignored) { | ||
| } | ||
|
|
||
| try { | ||
| if (conn != null) { | ||
| conn.close(); | ||
| } | ||
| } catch (SQLException ignored) { | ||
| } | ||
| } |
There was a problem hiding this comment.
close를 위한 코드가 복잡한데, 다른 방식으로 줄일 수 있을 것 같아요 😊
Comment on lines
+7
to
+10
| public interface RowMapper<T> { | ||
|
|
||
| T mapRow(ResultSet rs) throws SQLException; | ||
| } |
There was a problem hiding this comment.
저는 rowNum을 사용하지 않아도 된다는 생각은 미처 못했는데, 디테일한 부분까지 신경쓰시는군요 👍👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 이오
step2가 리팩터링이여서 기능 돌아갈정도로만 구현했습니다,,
저는 RowMapper에서 rowNum을 사용하지않았는데
spring-projects/spring-framework#7796 (comment)
RowMapper rowNum에 관한 Pr을 확인했을때
라는 이유때문에 대부분 사용하지않지만 남겨둔것으로 보였습니다.
그래서 저는 이때까지 rowNum을 사용해본적이없고 오히려 불편하기도하며 코드가 더러워지는것같아서 제거했습니다,,
flyway jdbc도 rowNum없이 인터페이스를 정의했네요,,
이번 미션 잘 부탁드립니다! 👍