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단계] 베로(김은솔) 미션 제출합니다. #339

Merged
merged 15 commits into from
Oct 3, 2023

Conversation

Cyma-s
Copy link

@Cyma-s Cyma-s commented Sep 30, 2023

안녕하세요, 두둠! 1단계 구현이 너무 늦었죠... 죄송합니다 🙇🏻‍♀️

이번 미션에서 JdbcTemplate 을 너무 참고하려다 보니까 구현이 많이 복잡해졌습니다 😂
이것도 되어야 하는데? 저것도 되어야 하는데? 하면서 추가하다보니 결국 다 추가하게 되었어요.

StatementCallback 으로 각각의 쿼리를 추상화하기 위해 노력해봤습니다. execute 메서드에서는 단순하게 doInStatement, doInPreparedStatement 를 실행하기만 합니다.
RowMapper 는 사용자화된 RowMapperUserRowMapper 와 단순한 객체에 값을 바인딩하는 SingleColumnRowMapper 를 만들어 주었습니다 😄
ResultSetExtractorResultSet 에서 각 행들을 RowMapper 로 매핑해주는 역할을 수행합니다.

고민되는 부분은 SingleColumnRowMapper 인데요, 리플렉션을 사용해서 필드에 알아서 객체들을 넣어주는 방식을 채택했습니다.
그러다보니 어쩔 수 없이 mapper 클래스에서 리플렉션을 쓰게 되었는데, 이게 정말 옳은 구조인지 확신이 안 서서 두둠의 의견이 궁금합니다 🤔

리뷰 감사합니다!

@Cyma-s Cyma-s changed the base branch from main to cyma-s September 30, 2023 11:23
Copy link

@younghoondoodoom younghoondoodoom left a comment

Choose a reason for hiding this comment

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

안녕하세요 베로~! 리뷰가 다소 늦었네요ㅠㅠ 고민을 많이 하신 흔적이 정말정말 많이 보이네요! 제 생각 간단하게 남겼으니까 참고 부탁드립니다 ㅎㅎ

고민되는 부분은 SingleColumnRowMapper 인데요, 리플렉션을 사용해서 필드에 알아서 객체들을 넣어주는 방식을 채택했습니다.
그러다보니 어쩔 수 없이 mapper 클래스에서 리플렉션을 쓰게 되었는데, 이게 정말 옳은 구조인지 확신이 안 서서 두둠의 의견이 궁금합니다 🤔

저는 jdbcTemplate을 호출하는 부분에서 CallBack 함수를 구현하게 할 것 같아요! 베로가 고민하셨던 포인트들이 다 해결될 것 같습니다.
템플릿 콜백 패턴에 대해서 공부해보시면 좋을 것 같아요! 토비의 스프링도 참고해보시면 좋을 것 같습니다~!

Comment on lines +68 to +80
class ExecuteStatementCallback implements StatementCallback<Object> {

@Override
public Object doInStatement(final Statement statement) throws SQLException {
statement.execute(sql);
return null;
}

@Override
public String getSql() {
return sql;
}
}

Choose a reason for hiding this comment

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

CallBack함수를 JdbcTemplate에서 구현해서 사용하고 있네요! jdbcTemplateCallBack 함수를 사용하는 이유는 jdbc를 이용할때 공통적인 부분은 Template으로 만들고, 변화하는 부분을 CallBack 함수로 정의하도록 하기 위함입니다. jdbcTemplate을 사용하는 곳에서 Callback함수를 직접 생성하도록 만들어 사용성을 편하게 개선하는건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

답변에 앞서 먼저 제 의도를 말씀 드릴게요 😄
ExecuteStatementCallbackJdbcTemplate 내부의 콜백입니다. 현재 ExecuteStatementCallback 같은 경우, 리턴 값이 다른 (ExecuteStatementCallback 은 null 을 리턴하지만 QueryStatementCallback은 리턴 값을 줘야 하듯이요) 쿼리를 좀 더 추상화하기 위해, 더 확장성 있게 만들기 위해 JdbcTemplate 의 콜백 함수로 구현해보았습니다!

두둠이 말씀하셨던 JdbcTemplate 을 사용하는 곳에서 콜백 함수를 생성한다는 것의 의미를 좀 더 설명해주실 수 있을까요? 제가 의도를 잘못 이해했을 수도 있을 것 같아 혹시나 하는 마음에 재설명 요청드립니다 🙇‍♀️

Choose a reason for hiding this comment

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

@Override
    public <T> T query(final String sql, StatementCallback<T> statementCallback) throws DataAccessException {
        return execute(statementCallback, sql);
    }

이런식으로 구현해볼 수 있다고 생각했습니다! ResultSetExtractor가 사용곳마다 달라야한다면, 그것 또한 StatementCallback에서 구현해서 사용할 수 있을 것 같습니다!


T doInStatement(final Statement statement) throws SQLException;

String getSql();

Choose a reason for hiding this comment

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

sql은 그냥 jdbcTemplate의 인자로 받는건 어떨까요? 무조건은 아니지만 콜백함수는 함수형 인터페이스로 만드는게 좋다고 생각합니다. 람다를 사용하면 코드가 더 깔끔해지는게 이유입니다!

Copy link
Author

Choose a reason for hiding this comment

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

기존에는 execute 를 private 메서드로 사용할 생각으로 이렇게 구현한 것이었는데, 지금 상황에서는 굳이 그럴 필요는 없을 것 같아서 변경했습니다!!

Copy link
Author

Choose a reason for hiding this comment

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

StatementCallbackPreparedStatementCallback 을 동시에 변경하면서 깨달았는데, sql 을 매개변수로 받아서 넘겨주게 되면 java 가 모호한 호출이라고 해서 구분하지 못하는 문제가 발생했습니다. 함수형 인터페이스를 람다로 사용 시 메서드 인자 구분이 불가능하더라고요. 그래서 일단 그대로 sql 을 사용하도록 두었습니다.
저는 해결 방법이 생각이 안 나서 그대로 두는 걸 선택했는데, 두둠이 생각하시는 좋은 방법이 있다면 알려주시면 감사하겠습니다 😊

Choose a reason for hiding this comment

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

 @Override
    public <T> T execute(final StatementCallback<T> callback, String sql) throws DataAccessException {
        log.info("execute: {}", sql);
        final var connection = getConnection();

        try {
            final var preparedStatement = connection.createStatement();

            return callback.doInStatement(preparedStatement);
        } catch (SQLException e) {
            log.error(e.getMessage(), e);
            throw new DataAccessException(e);
        }
    }

이런 식으로 직접 받아서 사용할 수 있을 것 같습니다! StatementCallback의 getSql을 사용하는 것이 조금 마음에 걸렸습니다 ㅎㅎ

Copy link
Author

@Cyma-s Cyma-s Oct 5, 2023

Choose a reason for hiding this comment

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

저도 저렇게 통합하도록 시도를 해봤는데, 자바가 execute (StatementCallback<T> callback, String sql)execute (PreparedStatementCallback<T> callback, String sql) 형태의 두 가지 메서드가 동시에 존재하니까 모호한 메서드 호출로 인식해서 두 메서드를 구분하지 못하더라구요 😂
일단은 분리한 채로 두었습니다 🫥

jdbc/src/test/java/nextstep/jdbc/JdbcTemplateTest.java Outdated Show resolved Hide resolved
Copy link

@younghoondoodoom younghoondoodoom left a comment

Choose a reason for hiding this comment

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

안녕하세요 베로~! 요구사항 잘 반영해주셨네요ㅎㅎ 베로의 질문에 대한 답변과 제 의견 간단하게 남겼으니까 읽어보시고 (원하시면) 다음 단계에 반영해주시면 감사하겠습니다!
고생하셨습니다🙌

@younghoondoodoom younghoondoodoom merged commit 3d91cca into woowacourse:cyma-s Oct 3, 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