-
Notifications
You must be signed in to change notification settings - Fork 300
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단계] 쥬니(전정준) 미션 제출합니다. #271
Changes from all commits
e59ac21
1c468a3
42de918
3125b49
cffc879
4abaf57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,121 +1,72 @@ | ||
package com.techcourse.dao; | ||
|
||
import com.techcourse.domain.User; | ||
import org.springframework.jdbc.core.JdbcTemplate; | ||
import java.util.List; | ||
import javax.sql.DataSource; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import javax.sql.DataSource; | ||
import java.sql.Connection; | ||
import java.sql.PreparedStatement; | ||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
import java.util.List; | ||
import org.springframework.jdbc.core.JdbcTemplate; | ||
import org.springframework.jdbc.core.RowMapper; | ||
|
||
public class UserDao { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(UserDao.class); | ||
|
||
private final DataSource dataSource; | ||
|
||
public UserDao(final DataSource dataSource) { | ||
this.dataSource = dataSource; | ||
private static final RowMapper<User> rowMapper = | ||
rs -> new User( | ||
rs.getLong("id"), | ||
rs.getString("account"), | ||
rs.getString("password"), | ||
rs.getString("email") | ||
); | ||
Comment on lines
+14
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2번 질문에 대한 제 생각을 먼저 말씀드리겠습니다. (어느 코드에 달지 모르겠어서 여기 달게용❤️)
위 이유들로 현실적으로 리플렉션을 사용해도 구현은 쉽지 않을 것 같습니다. 그리고 저는 엔티티의 생성을 어디서 하는지는 JdbcTemplate을 역할을 어디까지 볼 것인가의 차이인 것 같습니다. 현재는 엔티티의 생성을 RowMapper에게 시키기고 그를 Dao에서 JdbcTemplate쪽으로 넘겨주게 됩니다. 그래서 현재로서는 쥬니처럼 RowMapper를 통해서 객체 생성을 쿼리문의 어떤 결과의 값으로 구성할지 DAO단에서 정의하는 것이 저는 괜찮은 그림이라고 생각합니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그리고 추가적으로 쥬니가 말한 Class type을 매개변수로 넘겨주는 select count(*) from user; 위와 같이 각 로우의 결과가 단일로 나올 때 사용하기 위함입니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 와 정성 미쳤다 !!!!!!!!!! 고마워요 루카 😘 클래스 타입을 받는 것에 대한 오해가 있었군요 ㅎㅎㅎㅎㅎ 이정도 정성이면 리뷰 늦은거 용서해드립니다 ^^ 중요한 부분은 아니지만, 답변 중에 궁금한 점이 있어서요 ~!
JPA는 라이브러리인가요 ? 프레임워크인가요 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 쥬니 맞는말씀이세요!! 그래서 만약 유저가 RowMapper같이 생성을 설정하지 않고 |
||
|
||
private final JdbcTemplate jdbcTemplate; | ||
|
||
public UserDao(DataSource dataSource) { | ||
this.jdbcTemplate = new JdbcTemplate(dataSource); | ||
} | ||
|
||
public UserDao(final JdbcTemplate jdbcTemplate) { | ||
this.dataSource = null; | ||
public UserDao(JdbcTemplate jdbcTemplate) { | ||
this.jdbcTemplate = jdbcTemplate; | ||
} | ||
|
||
public void insert(final User user) { | ||
final var sql = "insert into users (account, password, email) values (?, ?, ?)"; | ||
|
||
Connection conn = null; | ||
PreparedStatement pstmt = null; | ||
try { | ||
conn = dataSource.getConnection(); | ||
pstmt = conn.prepareStatement(sql); | ||
|
||
log.debug("query : {}", sql); | ||
|
||
pstmt.setString(1, user.getAccount()); | ||
pstmt.setString(2, user.getPassword()); | ||
pstmt.setString(3, user.getEmail()); | ||
pstmt.executeUpdate(); | ||
} catch (SQLException e) { | ||
log.error(e.getMessage(), e); | ||
throw new RuntimeException(e); | ||
} finally { | ||
try { | ||
if (pstmt != null) { | ||
pstmt.close(); | ||
} | ||
} catch (SQLException ignored) {} | ||
|
||
try { | ||
if (conn != null) { | ||
conn.close(); | ||
} | ||
} catch (SQLException ignored) {} | ||
} | ||
public void insert(User user) { | ||
String sql = "insert into users (account, password, email) values (?, ?, ?)"; | ||
|
||
log.debug("query : {}", sql); | ||
|
||
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail()); | ||
} | ||
|
||
public void update(final User user) { | ||
// todo | ||
public void update(User user) { | ||
String sql = "UPDATE users SET account = ?, password = ?, email = ? WHERE id = ?"; | ||
|
||
log.debug("query : {}", sql); | ||
|
||
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId()); | ||
} | ||
|
||
public List<User> findAll() { | ||
// todo | ||
return null; | ||
String sql = "SELECT * from users"; | ||
|
||
log.debug("query : {}", sql); | ||
|
||
return jdbcTemplate.query(sql, rowMapper); | ||
} | ||
|
||
public User findById(final Long id) { | ||
final var sql = "select id, account, password, email from users where id = ?"; | ||
|
||
Connection conn = null; | ||
PreparedStatement pstmt = null; | ||
ResultSet rs = null; | ||
try { | ||
conn = dataSource.getConnection(); | ||
pstmt = conn.prepareStatement(sql); | ||
pstmt.setLong(1, id); | ||
rs = pstmt.executeQuery(); | ||
|
||
log.debug("query : {}", sql); | ||
|
||
if (rs.next()) { | ||
return new User( | ||
rs.getLong(1), | ||
rs.getString(2), | ||
rs.getString(3), | ||
rs.getString(4)); | ||
} | ||
return null; | ||
} catch (SQLException e) { | ||
log.error(e.getMessage(), e); | ||
throw new RuntimeException(e); | ||
} 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) {} | ||
} | ||
public User findById(Long id) { | ||
String sql = "select * from users where id = ?"; | ||
|
||
log.debug("query : {}", sql); | ||
|
||
return jdbcTemplate.queryForObject(sql, rowMapper, id); | ||
} | ||
|
||
public User findByAccount(final String account) { | ||
// todo | ||
return null; | ||
public User findByAccount(String account) { | ||
String sql = "select * from users where account = ?"; | ||
|
||
log.debug("query : {}", sql); | ||
|
||
return jdbcTemplate.queryForObject(sql, rowMapper, account); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,68 +1,74 @@ | ||
package com.techcourse.dao; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import com.techcourse.config.DataSourceConfig; | ||
import com.techcourse.domain.User; | ||
import com.techcourse.support.jdbc.init.DatabasePopulatorUtils; | ||
import java.util.List; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.Test; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import org.springframework.jdbc.core.JdbcTemplate; | ||
|
||
class UserDaoTest { | ||
|
||
private final JdbcTemplate jdbcTemplate = new JdbcTemplate(DataSourceConfig.getInstance()); | ||
private UserDao userDao; | ||
|
||
@BeforeEach | ||
void setup() { | ||
DatabasePopulatorUtils.execute(DataSourceConfig.getInstance()); | ||
|
||
jdbcTemplate.update("TRUNCATE TABLE users RESTART IDENTITY"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
userDao = new UserDao(DataSourceConfig.getInstance()); | ||
final var user = new User("gugu", "password", "[email protected]"); | ||
|
||
User user = new User("gugu", "password", "[email protected]"); | ||
userDao.insert(user); | ||
} | ||
|
||
@Test | ||
void findAll() { | ||
final var users = userDao.findAll(); | ||
List<User> users = userDao.findAll(); | ||
|
||
assertThat(users).isNotEmpty(); | ||
} | ||
|
||
@Test | ||
void findById() { | ||
final var user = userDao.findById(1L); | ||
userDao.insert(new User("gugu", "password", "[email protected]")); | ||
User user = userDao.findById(1L); | ||
|
||
assertThat(user.getAccount()).isEqualTo("gugu"); | ||
} | ||
|
||
@Test | ||
void findByAccount() { | ||
final var account = "gugu"; | ||
final var user = userDao.findByAccount(account); | ||
String account = "gugu"; | ||
User user = userDao.findByAccount(account); | ||
|
||
assertThat(user.getAccount()).isEqualTo(account); | ||
} | ||
|
||
@Test | ||
void insert() { | ||
final var account = "insert-gugu"; | ||
final var user = new User(account, "password", "[email protected]"); | ||
String account = "insert-gugu"; | ||
User user = new User(account, "password", "[email protected]"); | ||
userDao.insert(user); | ||
|
||
final var actual = userDao.findById(2L); | ||
User actual = userDao.findById(2L); | ||
|
||
assertThat(actual.getAccount()).isEqualTo(account); | ||
} | ||
|
||
@Test | ||
void update() { | ||
final var newPassword = "password99"; | ||
final var user = userDao.findById(1L); | ||
String newPassword = "password99"; | ||
User user = userDao.findById(1L); | ||
user.changePassword(newPassword); | ||
|
||
userDao.update(user); | ||
|
||
final var actual = userDao.findById(1L); | ||
User actual = userDao.findById(1L); | ||
|
||
assertThat(actual.getPassword()).isEqualTo(newPassword); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,93 @@ | ||
package org.springframework.jdbc.core; | ||
|
||
import java.sql.Connection; | ||
import java.sql.PreparedStatement; | ||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.NoSuchElementException; | ||
import javax.sql.DataSource; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import javax.sql.DataSource; | ||
|
||
public class JdbcTemplate { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(JdbcTemplate.class); | ||
|
||
private final DataSource dataSource; | ||
|
||
public JdbcTemplate(final DataSource dataSource) { | ||
public JdbcTemplate(DataSource dataSource) { | ||
this.dataSource = dataSource; | ||
} | ||
|
||
public void update(String sql, Object... args) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 파라미터에 final이 붙는 경우도 있고 붙지 않는 경우도 있는데, 혹시 기준이 있으실까용? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 실수인데... 아마 기존 코드가 붙어 있었어서... 파라미터에 final, var 키워드 사용하는 것은 개인적으로 좋아하지 않습니다 ! (귀찮거덩요) 제거하지 못한 final, var 키워드는 제거하여 반영하였습니다 ! |
||
try (Connection conn = dataSource.getConnection(); | ||
PreparedStatement pstmt = conn.prepareStatement(sql)) { | ||
|
||
initializePstmtArgs(pstmt, args); | ||
|
||
pstmt.executeUpdate(); | ||
} catch (SQLException e) { | ||
log.error(e.getMessage(), e); | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
|
||
private void initializePstmtArgs(PreparedStatement pstmt, Object... args) throws SQLException { | ||
for (int i = 0; i < args.length; i++) { | ||
pstmt.setObject(i + 1, args[i]); | ||
} | ||
} | ||
Comment on lines
+37
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PreparedSatement의 변수 설정하는 중복로직을 줄이기 위해서 선언해주셨네요.👍 근데 이 클래스 내에서 이 중복말고도, try-resource-catch 하는 과정들도 많이 중복되는 것으로 보여지는데요. 혹시 이 중복도 제거할 수 있는지 고려해보섰나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 미션 당시에는 크게 신경쓰지 않았던 것 같아요. (2단계가 리팩토링이길래 ㅎㅎ) 함수형 인터페이스를 어떻게 잘 사용해보면.. 해결할 수 있지 않을까 ? 싶긴 한데, 직접 해봐야 알 것 같네요 ! 먼저 2단계를 진행하시다가, 좋은 방법을 발견하시면 공유해주세요 ~! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 시도하다가 실패해가지고 조금 더 고민해봐야될 것 같아요!! ㅋㅋ |
||
|
||
public <T> List<T> query(String sql, RowMapper<T> rowMapper) { | ||
try (Connection conn = dataSource.getConnection(); | ||
PreparedStatement pstmt = conn.prepareStatement(sql); | ||
ResultSet rs = pstmt.executeQuery()) { | ||
|
||
List<T> results = new ArrayList<>(); | ||
|
||
while (rs.next()) { | ||
T result = rowMapper.mapRow(rs); | ||
|
||
results.add(result); | ||
} | ||
|
||
return results; | ||
} catch (SQLException e) { | ||
log.error(e.getMessage(), e); | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
|
||
public <T> T queryForObject(String sql, RowMapper<T> rowMapper, Object... args) { | ||
try (Connection conn = dataSource.getConnection(); | ||
PreparedStatement pstmt = getQueryPstmtForObject(sql, conn, args); | ||
ResultSet rs = pstmt.executeQuery()) { | ||
|
||
if (rs.next()) { | ||
T result = rowMapper.mapRow(rs); | ||
|
||
if (rs.next()) { | ||
throw new IllegalArgumentException("Incorrect Result Size ! Result must be one"); | ||
} | ||
|
||
return result; | ||
} | ||
|
||
throw new NoSuchElementException("Incorrect Result Size ! Result is null"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (위 예외 리뷰와 이어지는 얘기입니다) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메서드 명이 'queryForObject` 잖아요 ? null은 객체로 보기 힘들 것 같고, Optional은 객체로 볼 수는 있지만, 실제(?) 객체가 있는지 없는지 불분명하다는 의미가 있잖아요 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋네요!! |
||
} catch (SQLException e) { | ||
log.error(e.getMessage(), e); | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
|
||
private PreparedStatement getQueryPstmtForObject(String sql, Connection conn, Object... args) throws SQLException { | ||
PreparedStatement pstmt = conn.prepareStatement(sql); | ||
|
||
initializePstmtArgs(pstmt, args); | ||
|
||
return pstmt; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 불필요한 개행으로 보여집니다!! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아니이 팀 컨벤션이라고여 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️ |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package org.springframework.jdbc.core; | ||
|
||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
|
||
@FunctionalInterface | ||
public interface RowMapper<T> { | ||
|
||
T mapRow(ResultSet resultSet) throws SQLException; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (이 뒤로는 아마 다 컨벤션 관련 리뷰일 것 같습니다) 불필요한 개행은 지워보는 것이 어떨까용? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지금 괜찮을지도 백엔드 팀원들 무시하는 건가요 ? 11번 라인에서 중괄호가 닫히기 때문에, 10번 라인에 개행이 있는거죠 ! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️🙇♂️ |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logger와 RowMapper를 둘다 static으로 두셨는데요.
각각 static 변수로 두신 이유가 있으실까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 static 키워드를 사용할 때, 두 가지정도만 고민해보고 합당하면 사용하는 편인데요.
위 두 가지 조건을 고려했을 때, 문제 되는 부분이 없는 것 같다고 판단했어요.
현재 코드에서 static 키워드를 사용하지 않을 이유는 없다는 생각도 들었구요 ㅎㅎ (같은 말인가..?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇네요.
클래스 변수로 사용해도 딱히 상태가 변화하지 않으니까 굳이 인스턴스 생성할 때마다 생성하지 않아도 돼서 좋은 것 같아요!!