[JDBC 라이브러리 구현하기 - 1, 2단계] 조엘(조영상) 미션 제출합니다. #48

joelonsw · 2021-09-28T00:57:50Z

안녕하세요 중간곰!!!!
잘 지내고 있지 형??? 🙌🙌🙌

이번 미션 어렵네요,,, 제가 생각보다 추상화시키는 것이나 함수형 인터페이스 사용등에 익숙하지 않다는 것을 깨달았어요.
토비의 스프링 3장을 참고하면서 공부하면서 미션했네요.
여전히 아주 잘 와닿지는 않는 것 같아요.
템플릿 콜백 패턴 같은 것등을 과연 나 혼자 코드 짤 때 생각하고 도입할 수 있을까? 라고 하면 그건 아직 아닌 것 같네요ㅜㅜ

조금 혼자 더 해보면 좋겠지만 중간곰 피드백 받으면서 개선해 나가려고 합니다~
소중한 시간 써주셔서 감사합니다 💪💪

- CheckedException을 UncheckedException으로 변경 - try-with-resources 활용

sihyung92 · 2021-09-28T04:51:31Z

동갑내기끼리 걸렸네요!! 화이팅 :))

ggyool

테코톡 고생했어 👏👏👏
쉽게 설명해줘서 너무 좋았어. 잘 모르던 내용이었는데 도움이 많이 됐음 🙇‍♂️🙇‍♂️🙇‍♂️

바쁜 와중에도 구현을 잘 해줘서, 코드적으로는 남길 부분이 몇 개 없는데
조엘의 생각이 궁금해서 몇개의 리뷰를 남겼어. 천천히 수정하고 편하게 리뷰 요청해 줘!

이건 여유가 있으면 할만한 것들을 남기는데, 조엘의 의지대로 하면 좋을 듯

JdbcTemplate 중복 제거
JdbcTemplate 테스트 만들기
DatabasePopulatorUtils.java 깔끔하게 정리하기

JdbcTemplate 테스트를 하기 어려울 텐데, 그만큼 깨닫는 부분도 많을 거라 생각해💪

ggyool · 2021-09-30T06:21:57Z

app/src/main/java/com/techcourse/dao/UserDao.java

    }

    public void update(User user) {
-        // todo
+        final String sql = "update users set account = (?), password = (?), email = (?) where id = (?)";


물음표를 괄호 안에 넣어주신 이유가 있을까요?
궁금하네요 🤔

개인 취향일 수 있는데 괄호로 감싸면 여기에 변수가 들어가야 하는구나가 더 눈에 잘 띈다고 생각해서요!
미완성된 쿼리문이라는 것이 조금 더 눈에 잘 들어온달까요?

ggyool · 2021-09-30T06:31:57Z

app/src/main/java/com/techcourse/repository/UserRepository.java

+
+import java.util.Optional;
+
+public interface UserRepository {


UserRepository 인터페이스를 만드셨군요!
Controller 에서 UserRepositoryImpl 과 InMemoryUserRepository 중 유연하게 선택하도록 인터페이스를 만드셨다고 생각이 들어요.

이름이 걸리긴 하지만 UserDao implements UserRepository 를 할 수도 있었을 것 같은데, 그렇게 하지 않은 이유가 궁금하네요 🤔

우선 현재 구조를 가장 건드리지 않으면서 코드를 변경하고 싶었어요!
UserDao는 현재 처럼 DB와 가장 가까이서 데이터 접근만 담당하고, Optional<User> findByAccount 와 같은 Optional 처리 등은 Repository에서 담당하는 구조로요!

사실 UserDao나 UserRepository나 하는 역할이 정말 거의 차이가 없어서 합칠 수는 있지만, 기존 코드와의 상생을 위해 이런 구조로 가져갔습니다!

ggyool · 2021-09-30T06:33:45Z

app/src/main/java/com/techcourse/repository/InMemoryUserRepository.java

@@ -6,7 +6,7 @@
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;

-public class InMemoryUserRepository {
+public class InMemoryUserRepository implements UserRepository {

    private static final Map<String, User> database = new ConcurrentHashMap<>();


Suggested change

private static final Map<String, User> database = new ConcurrentHashMap<>();

private final Map<String, User> database = new ConcurrentHashMap<>();

InMemoryRepository를 객체화하여 사용하는 방향으로 바꿨으니, 인스턴스 멤버로 바꿔줘야 자연스러운 것 같아요.
각각의 객체가 새로운 데이터 베이스를 생성하여 사용하는 방식으로요.

제가 느끼는 어색함은 아래 같은 상황이에요.

InMemoryUserRepository a = new InMemoryUserRepository(); a.save(user); InMemoryUserRepository b = new InMemoryUserRepository(); // 새로 생성한 객체에 이미 user가 들어가 있음

만약 하나의 데이터베이스를 사용하기 위해 static 을 사용하고 싶으면, 싱글턴으로 관리하면 좋을 것 같습니다!

동의합니다!
피드백 반영했습니다~
6682f9c

ggyool · 2021-09-30T06:53:22Z

jdbc/src/main/java/nextstep/jdbc/JdbcTemplate.java

+    }
+
+    public int update(String sql, Object... args) {
+        return executeUpdate(connection -> makePreparedStatement(sql, connection, args));


connection 으로 PreparedStatement를 만들고 인자를 셋팅하는 부분을 StatementStrategy 함수형 인터페이스로 만들어 주셨는데요.
update, queryForObject, query 메서드에서 같은 전략을 사용하고 있는 것을 보면 함수형 인터페이스를 만든 장점이 떨어지는 것 같아요.

jdbcTemplate 내부에서만 사용하는 콜백이니, 메서드 분리만으로 충분하지 않을까?? 라는 생각입니다. 의견이 궁금해요~!

좋은 지적 감사합니다! 완벽하게 이해했는지는 확실치 않지만 StatementStrategy라는 함수형 인터페이스의 생성을 메서드로 분리해봤어요!! 일일이 StatementStrategy를 생성해주던 코드보다는 훨씬 유연해진 것 같네요!
269c7e9

ggyool · 2021-09-30T06:54:21Z

jdbc/src/main/java/nextstep/jdbc/JdbcTemplate.java

+            if (rs.next()) {
+                return rowMapper.mapRow(rs);
+            }
+            return null;


아래 executeQuery() 메서드와 해당 부분만 다르니, 콜백으로 분리하면 정말 좋은 구조라는 생각이 들어요 !

네 콜백으로 분리해보았습니다~ 감사합니다!

ggyool · 2021-09-30T06:59:05Z

app/src/main/java/com/techcourse/repository/UserRepositoryImpl.java

+
+    @Override
+    public Optional<User> findByAccount(String account) {
+        return Optional.of(userDao.findByAccount(account));


Suggested change

return Optional.of(userDao.findByAccount(account));

return Optional.ofNullable(userDao.findByAccount(account));

이 부분 실수가 있네요!

감사합니다!!

- `@BeforeEach` 대신 `@BeforeAll`로 변경

- PreparedStatementCreatorFactory, ResultSetExtractorFactory 생성

joelonsw · 2021-10-05T15:15:23Z

JdbcTemplate의 코드들을 리팩토링 해봤습니다!
콜백을 생성하는 로직을 별도의 클래스로 분리하였어요.

JdbcTemplate의 테스트 코드도 작성해보았어요.
h2 testImplemenation 으로 의존성을 추가했는데요.
제가 만든 Jdbc 라이브러리가 특정 데이터베이스에 의존하지 않았으면 좋겠다 생각했는데,
h2 의존성 없이는 테스트하기가 참 어려워지더라고요.
testImplemtation으로 h2를 추가하는 것으로 합의(?)를 봤던 것 같아요!

중간곰의 피드백 덕분에 고민해볼 수 있던 주제가 많아서 좋았습니다 :) 감사합니다 :)

sonarcloud · 2021-10-05T15:28:29Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

ggyool

워낙 깔끔하게 만들어 주셔서 크게 남길 리뷰가 없네요 😓
테코톡 하느라 바쁜 와중에 미션 하느라 고생하셨습니다 👏
조엘 화이팅 💪💪💪

ggyool · 2021-10-06T15:45:44Z

app/src/main/java/com/techcourse/support/jdbc/init/DatabasePopulatorUtils.java

+        try (Connection connection = dataSource.getConnection();
+             Statement statement = connection.createStatement()) {


깔끔하게 정리해 주셨네요👍👍

ggyool · 2021-10-06T16:22:41Z

jdbc/src/test/java/nextstep/jdbc/JdbcTemplateTest.java

+            this.price = price;
+        }
+    }


제가 만든 Jdbc 라이브러리가 특정 데이터베이스에 의존하지 않았으면 좋겠다 생각했는데, h2 의존성 없이는 테스트하기가 참 어려워지더라고요.

이 부분이 걸린다면, 구현한 몇몇 핵심 로직을 메서드 단위의 테스트로 만들어 보는 게 어떨까요 ?
목 테스트이기 때문에 메서드 내부 구현을 알고 테스트를 만드는 어색함이 있지만

이러한 테스트를 만들어 놓음으로써 안정적인 라이브러리가 만들어진다고 생각해요.
동료가 코드를 이해하지 못하고 제멋대로 수정하면 이 테스트가 깨질 테니까요.
그리고 가독성을 좋게 만들어 놓는다면, 자신이 만든 코드를 남이 읽었을 때 이해를 돕는 이정표의 역할도 할 수 있다고 생각해요.
아래 코드는 예시입니다 !

ResultSetExtractorTest.java

@DisplayName("ResultSet 에서 단일 오브젝트를 추출한다.") @Test void extractObjectFromResultSet() throws SQLException { // given ResultSet resultSet = mock(ResultSet.class); RowMapper<Object> rowMapper = mock(RowMapper.class); Object object = new Object(); given(resultSet.next()).willReturn(true); given(rowMapper.mapRow(resultSet)).willReturn(object); ResultSetExtractor<Object> resultSetExtractor = ResultSetExtractorFactory.objectResultSetExtractor(rowMapper); // when Object extractObject = resultSetExtractor.extract(resultSet); // then assertThat(extractObject).isNotNull(); then(resultSet).should(times(1)).next(); then(rowMapper).should(times(1)).mapRow(resultSet); } @DisplayName("ResultSet 이 비어있으면 null을 반환한다.") @Test void extractObjectFromEmptyResultSet() throws SQLException { // given ResultSet resultSet = mock(ResultSet.class); RowMapper<Object> rowMapper = mock(RowMapper.class); Object object = new Object(); given(resultSet.next()).willReturn(false); given(rowMapper.mapRow(resultSet)).willReturn(object); ResultSetExtractor<Object> resultSetExtractor = ResultSetExtractorFactory.objectResultSetExtractor(rowMapper); // when Object extractObject = resultSetExtractor.extract(resultSet); // then assertThat(extractObject).isNull(); then(resultSet).should(times(1)).next(); then(rowMapper).should(times(0)).mapRow(resultSet); }

mocking을 도입하여 테스트를 진행하는 방법도 좋네요!! 감사합니다~~👍👍👍

ggyool · 2021-10-06T16:28:41Z

jdbc/src/main/java/nextstep/jdbc/JdbcTemplate.java

@@ -8,7 +9,6 @@
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.ArrayList;
 import java.util.List;

 public class JdbcTemplate {


JdbcTemplate 클래스가 너무 깔끔해졌네요 👍👍

joelonsw added 8 commits September 25, 2021 18:20

docs(README): 구현할 기능 목록 작성

3febdb4

feat: UserDao 메서드 구현

cfd420a

feat: JdbcTemplate 구현

b0bb419

docs(README): 구현할 기능 목록 작성

61a405f

refactor: JdbcTemplate 리팩토링

16dcfe0

- CheckedException을 UncheckedException으로 변경 - try-with-resources 활용

test: Junit Reflection 관련 테스트 작성

0366a42

refactor: UserRepository 인터페이스 분리

9f86816

refactor: UserRepositoryImpl로 데이터 저장하도록 변경

63213a3

ggyool suggested changes Sep 30, 2021

View reviewed changes

joelonsw added 9 commits October 5, 2021 09:09

refactor: StatementStrategy 생성을 메서드로 분리

269c7e9

test(UserDaoTest): setUp을 전체 테스트 중 한번만 수행하도록 변경

608ec14

- `@BeforeEach` 대신 `@BeforeAll`로 변경

refactor(JdbcTemplate): 콜백 생성 로직을 util 클래스로 분리

ba458a4

- PreparedStatementCreatorFactory, ResultSetExtractorFactory 생성

refactor(InMemoryUserRepository): 싱글턴으로 객체 관리하도록 변경

6682f9c

fix(UserRepositoryImpl): ofNullable로 Null 처리

b157ec0

refactor: Jdbc 예외 패키지 생성

92da071

refactor(DatabasePopulatorUtils): try-with-resource 도입

5dc1b22

build(jdbc:gradle): testImplementation에 h2 추가

4ee974b

test: JdbcTemplateTest 추가

e3a3219

ggyool approved these changes Oct 6, 2021

View reviewed changes

ggyool merged commit 4ead23f into woowacourse:papimonlikelion Oct 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JDBC 라이브러리 구현하기 - 1, 2단계] 조엘(조영상) 미션 제출합니다. #48

[JDBC 라이브러리 구현하기 - 1, 2단계] 조엘(조영상) 미션 제출합니다. #48

joelonsw commented Sep 28, 2021

sihyung92 commented Sep 28, 2021 •

edited

Loading

ggyool left a comment

ggyool Sep 30, 2021

joelonsw Oct 4, 2021

ggyool Sep 30, 2021

joelonsw Oct 5, 2021

ggyool Sep 30, 2021

joelonsw Oct 5, 2021

ggyool Sep 30, 2021

joelonsw Oct 5, 2021

ggyool Sep 30, 2021

joelonsw Oct 5, 2021

ggyool Sep 30, 2021

joelonsw Oct 5, 2021

joelonsw commented Oct 5, 2021

sonarcloud bot commented Oct 5, 2021

ggyool left a comment

ggyool Oct 6, 2021

ggyool Oct 6, 2021 •

edited

Loading

joelonsw Oct 7, 2021

ggyool Oct 6, 2021

	private static final Map<String, User> database = new ConcurrentHashMap<>();
	private final Map<String, User> database = new ConcurrentHashMap<>();

	return Optional.of(userDao.findByAccount(account));
	return Optional.ofNullable(userDao.findByAccount(account));

		try (Connection connection = dataSource.getConnection();
		Statement statement = connection.createStatement()) {

[JDBC 라이브러리 구현하기 - 1, 2단계] 조엘(조영상) 미션 제출합니다. #48

[JDBC 라이브러리 구현하기 - 1, 2단계] 조엘(조영상) 미션 제출합니다. #48

Conversation

joelonsw commented Sep 28, 2021

sihyung92 commented Sep 28, 2021 • edited Loading

ggyool left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelonsw commented Oct 5, 2021

sonarcloud bot commented Oct 5, 2021

ggyool left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggyool Oct 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sihyung92 commented Sep 28, 2021 •

edited

Loading

ggyool Oct 6, 2021 •

edited

Loading