Skip to content

Conversation

@reddevilmidzy
Copy link
Member

안녕하세요 초코칩!!

리뷰어로 만나서 반갑습니다 :)

리뷰 잘 부탁드립니다!

@reddevilmidzy reddevilmidzy self-assigned this Oct 7, 2024
Copy link

@Chocochip101 Chocochip101 left a comment

Choose a reason for hiding this comment

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

안녕하세요, 레디! 리뷰어로 만나뵙게 되어 반갑습니다 😄

이번 미션 잘 구현해주셨네요!

관련해서 리뷰 남겼으니 확인해주세요!

Comment on lines 20 to 22
public UserDao(final DataSource dataSource) {
this.dataSource = dataSource;
this.jdbcTemplate = new JdbcTemplate(dataSource);
}

Choose a reason for hiding this comment

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

생성자를 재사용해도 좋을 것 같네요!

    public UserDao(final DataSource dataSource) {
        this(new JdbcTemplate(dataSource));
    }

Copy link
Member Author

Choose a reason for hiding this comment

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

반영하였습니다!
커밋

Comment on lines +30 to +36
final DataSource dataSource = mock(DataSource.class);
connection = mock(Connection.class);
preparedStatement = mock(PreparedStatement.class);
resultSet = mock(ResultSet.class);

when(dataSource.getConnection()).thenReturn(connection);
when(connection.prepareStatement(Mockito.anyString())).thenReturn(preparedStatement);

Choose a reason for hiding this comment

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

테스트에서 mock을 사용해주셨는데, 이유가 있나요? 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

일단 JdbcTemplate이라는 객체를 만들었으니 테스트는 해야겠는데 이 녀석이 DataSource에 의존성이 있어 의존성을 주입해줘야 했습니다. 이때 실제 객체를 사용하지 않고 테스트하는 방법 중에 mock을 사용하지 않고 fake 객체를 만들어 테스트를 할 수 있습니다.

하지만 제가 fake 객체를 만들지 않은 이유는 레벨2와 프로젝트를 진행하면서 fake 객체를 만들어서 테스트하는 것에 대해 큰 불만이 생겼었는데요, 일단 관리해야하는 코드가 2배로 늘어난다는 점과 테스트가 실패했을때 나의 fake 객체를 잘못 구현하여 테스트가 실패한건지 아니면 실제 로직이 잘못되었던 것인지 빠르게 확인이 어렵다는 점이 별로였습니다.

그래서 mock을 사용하여 jdbcTemplateTest를 진행하였습니다!

Comment on lines 16 to 17
rs.getLong("id"), rs.getString("account"),
rs.getString("password"), rs.getString("email")

Choose a reason for hiding this comment

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

자바 코드 컨벤션 스타일에 맞나요? 제가 알기론 개행이 인자마다 있는 것으로 알고 있습니다!

public interface RowMapper<T> {

@Nullable
T mapRow(ResultSet rs, int rowNum) throws SQLException;

Choose a reason for hiding this comment

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

rowNum이 전체적으로 안쓰이고 있는데, 정의해주신 의도가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

RowMapper 인터페이스 코드 보고 따라치다가 의도 없이 같이 만들어버렸네요 😵
현재는 필요없는 것 같아 제거해주었습니다!

Copy link

@Chocochip101 Chocochip101 left a comment

Choose a reason for hiding this comment

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

레디의 의견도 잘 들었고, 리뷰 반영도 잘 해주셨네요!

이만 approve 하겠습니다 :)

@Chocochip101 Chocochip101 merged commit c6c3e17 into woowacourse:reddevilmidzy Oct 9, 2024
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.

2 participants