[JDBC 라이브러리 구현하기 - 1단계] 루카(백경환) 미션 제출합니다. #302
Conversation
There was a problem hiding this comment.
안녕하세요 루카!!
깔끔하게 잘 짜주셔서 편하게 리뷰했어요!
1단계에서는 충분히 다 된것같아서 바로 Approve 하고 머지할께요!! 코멘트 남긴것들이 있는데, 해당 부분은 보고 다음 리뷰요청 pr이나 이 pr에 답남겨주시면 확인할께요!
추가로 아래는 루카가 주신 질문에 대한 답입니다!
- 단일 로우 조회의 예외상황을 만드셨나요?
일단 저는 루카처럼 Optional로 있거나 없거나에 대한 표현을 하였고, 여러개인지는 확인도 안하고 그냥 하나 반환하도록 했었네요..ㅎㅎ
루카의 질문을 보고 저도 좀 찾아보았습니다. 찾아본 자료는 실제 JdbcTemplate 문서 JdbcTemplate.queryForObject를 확인해보았습니다.
확인을 해보면 조회해서 나온 쿼리가 딱 1가지가 아닌 경우에는 예외를 발생시킨다고 하네요.
그래서 원래 코드의 구현을 생각해보면 두가지 경우 (0개인경우, 2개이상인 경우) 모두 프레임워크에서 예외를 발생시켜 주는방향이 좋은 것 같습니다.
이건 JdbcTemplate의 실제 구현에 대한 내용이였고, 왜 이럴까에 대한 제 생각은 아래와 같습니다.
우선 이번미션의 2단계 내용에서도 보이듯이 예외처리에 대한 역할은 (어플리케이션)개발자가 아닌 라이브러리가 하는게 적합하다고 생각합니다. 해당 조회메소드를 호출했다는것은 조회메소드에서 1개의 결과가 나오는것을 기대했다는 의미인데 ,여기에서 결과가 없다면 이것은 예외상황이 될거고, 하나를 조회한다는 의도를 가진 라이브러리에서 예외처리를 해서 던져주는것이 적합한 방법이겠구나 라는 생각입니다. 만약 안그런다면 해당 라이브러리를 사용하는 개발자가 매번 예외처리를 해줘야하는 불편함이 생길 것 같네요.
다음으로 여러개일때 예외는 메소드 명에서 1개만 가져온다는 의미를 담으니 1개가 아니면 예외를 터트려야 하나 하는 그정도의 생각이네요 아직은...
- jdbcTemplate의 중복 제거
전 루카보다 중복제거를 고민을 못했네요..ㅠㅜㅋㅋㅋㅋㅋㅋ
이부분은 저도 조금 더 생각을해보고, 리팩터링을 해보며 정리해서 아이디어가 떠오르면 바로바로 공유해볼께요!!
연휴에도 열심히 1단계 하니라 고생 많았어요 루카!!
즐거운 추석 보내시고 다음단계도 화이팅해요!!
| try (final Connection connection = dataSource.getConnection(); | ||
| final PreparedStatement preparedStatement = initializePreparedStatement(connection, sql, objects); | ||
| final ResultSet resultSet = preparedStatement.executeQuery()) { | ||
| log.debug("query : {}", sql); |
There was a problem hiding this comment.
기본적으로 log.debug가 dao에서 사용되고 있는데, 여기에만 로깅이 남아있네요.
둘중 한간데에서 해줄 수 있도록 통일해주면 좋을 것 같아요.
추가로 루카는 프레임워크와 어플리케이션 코드 중 해당 로깅은 어디에서 하는것이 적합하다고 생각하는지 궁금해요!!
| private PreparedStatement initializePreparedStatement(final Connection connection, | ||
| final String sql, | ||
| final Object[] objects) throws SQLException { | ||
| final PreparedStatement preparedStatement = connection.prepareStatement(sql); | ||
| for (int i = 0; i < objects.length; i++) { | ||
| preparedStatement.setObject(i + 1, objects[i]); | ||
| } | ||
| return preparedStatement; | ||
| } |
There was a problem hiding this comment.
저는 해당 반복구문 분리에대한 생각을 깊게 안했었는데..!! 최고네요!! 👍
| public final RowMapper<User> rowMapper = resultSet -> new User( | ||
| resultSet.getLong(1), | ||
| resultSet.getString(2), | ||
| resultSet.getString(3), | ||
| resultSet.getString(4) | ||
| ); |
There was a problem hiding this comment.
해당 rowMapper가 public 인 이유가 있을까요??? dao내부에서만 쓰이닌 private하는게 좋을 것 같아요.
그러고 getLong, getString에서 int index를 기반으로 사용하셨는데, String 파라미터로 하여 select한 이름이 아닌 인덱스로 순서로 로우맵퍼를 만든 이유가 궁금합니다.
저는 가독성적으로도 스트링값으로 입력을 해주는것이 어떤가 생각을 하고, 인덱스로 하면 sql을 작성할때 순서를 보장해서 적어주도록 신경을 써줘야 할 것 같습니다.
(물론 스트링으로 해도 동일한 이름으로 해줘야한다는 부분은 신경써야 하지만요...)
안녕하세요 필립!!!!
필립이 리뷰어라니 너무 영광입니다. 잘 지내죠? 보고싶네요.
더 빨리 리뷰 보냈어야했는데, 이것저것 하느라 조금 늦어졌네요!
이번 단게에서는 DataSource를 사용하여 DB에 쿼리문을 전송할 때, 사용할 수 있는 JdbcTemplate을 구현해보았는데요.
미션에서 작성되어있던 것처럼 이 라이브러리를 사용하는 개발자가 고려를 할 부분을
이 세가지에만 집중해도 사용할 수 있게끔 기능을 구현해보았는데요.
그래서 제가 이번에 작성한 메서드들에는 3가지 인자가 있을 수 있습니다,
또한 3가지 메서드는 각각 다음과 같은 역할을 합니다.
제 구현에 대한 설명은 이정도로 할 수 있을 것 같습니다!
그리고 약간 궁금한 점이 2가지 있는데요.
1. 단일 로우 조회의 예외상황을 만드셨나요?
결과가 2개면 예외를 발생시키셨나요? 0개면 예외를 발생키셨나요?
어떻게 해야 사용자가 더 편하고 직관적으로 이 라이브러리르 사용할 수 있을까 고민해봤을 때 저는 Optional로 감싸서 없으면 empty를 반환하고 2개이상이면 그중 1번째 것을 반환하도록 하였습니다.
필립의 생각은 어떠신가요?
2. jdbcTemplate의 중복 제거
jdbcTempatle에서 각 메서드 마다
이런 작업이 각 주된 기능들 전, 후에 처리가 되는데 이에 대한 중복 제거는 고려해보신게 있으신가요??
저는 잘 감이 안오더라구요 ㅠㅠ
긴 글 읽어주셔서 감사합니다.
늘 보고싶네요 필립 추석 잘 보내세요
리뷰 잘 부탁드립니다.