[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 이산 미션 제출합니다.#363
[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 이산 미션 제출합니다.#363Rix01 wants to merge 25 commits intowoowacourse:rix01from
Conversation
- 궁성 구현 - 점수 계산 - 게임 종료 로직 추가
- Palace를 Enum으로 구현 - isInPalace라는 메서드를 구현하여 position 넣었을 때 궁성 영역 판별
- CommonMoveRule: addPalaceRoutes를 통해 대각선 경로를 가져와 일반 경로와 통합 - 왕/사: 기본 이동을 상하좌우로 한정하고, canMove에서 궁성 내 이동 제한 로직 추가 - 마/상: 기존에도 대각선이 가능하므로 궁성 규칙에서 제외 - 졸: 전진 방향의 대각선 경로만 허용하도록 필터링
- 한나라는 1.5점 더 가진 상태로 계산 - PieceType이 점수를 가짐 - ScoreResponse라는 DTO 구현
- Piece에 있는 isKing(), isCho()를 활용하여 초나라 왕이 살아있는지 여부 판단하여 승자 판정
- 상차림 설정 직후 및 매턴 종료 시 저장
- 게임 시작 전 '새 게임' 및 '이어하기' 선택 옵션 추가 - 이어하기 시 진행 중인 게임 목록을 ID 내림차순으로 최대 5개 제공 - 선택된 게임 ID를 바탕으로 게임 및 기물 불러오기 - GameEntity를 GameResponse DTO로 변환하여 View에 전달
- while 조건문 안에 position이 null일 경우 계속 돌아가도록 하여 잘못된 값 입력 시 null 반환하는 형식으로 수정
- schema.sql에서 IF NOT EXISTS 적용하여 없을 때만 테이블 만들도록 수정 - Application 실행할 때마다 schema.sql 알아서 실행되게 수정
- 테스트 시작 및 종료 세팅 - 새로운 게임 저장 테스트 코드 작성 - 진행중인 게임 업데이트 테스트 코드 작성 - 진행중인 게임 가져오기 테스트 코드 작성 - 게임 ID로 게임 조회 테스트 코드 작성
- 테스트 시작 및 종료 세팅 - 기물 정보 한꺼번에 저장하는 테스트 코드 작성 - 기물 정보 한꺼번에 삭제하는 테스트 코드 작성 - 기물 정보 한꺼번에 조회하는 테스트 코드 작성
- 테스트 시작 및 종료 세팅 - 새로운 장기 게임 저장 테스트 작성 - 진행중인 장기 게임 업데이트 테스트 작성 - 존재하지 않는 게임 가져오기 테스트 작성 - 저장되어 있는 보드 가져오기 테스트 작성
- 축약어 수정 - public, private 메서드 위치 수정
pkeugine
left a comment
pkeugine
left a comment
There was a problem hiding this comment.
안녕하세요 이산, PK입니다.
이번 미션 아주 잘 진행해주셨네요.
정말 좋아요.
지금 머지해도 딱히 이상하지 않은 상태인 것 같아요. 💯
테스트가 실패하긴 하는데,
이건 실패할거라고 예상하지 못하셨을 것 같아서 이해합니다.
오히려 왜 실패하는지 학습해보면 좋을 것 같고,
꼭 해결하지 않아도 괜찮습니다.
간단한 리뷰 몇 가지 남겼고요,
남길 리뷰가 많지 않아서 이산이 남긴 질문에 대한 답변에 신경썼습니다.
만약 이해하기 어려우면 코멘트 또는 DM 주세요.
후딱 읽어보시고
반영 및 확인이 완료되면 얼른 리뷰요청 주세요.
그 때는 머지하겠습니다.
해피해킹하세요!!! 😸 😸 😸
| @@ -2,43 +2,53 @@ | |||
|
|
|||
| 장기 미션 저장소 | |||
There was a problem hiding this comment.
사이클1에서 잘 리팩토링된 코드를 통해 사이클2를 진행하다보니 궁성 영역은 생각보다 수월하게 할 수 있었습니다!
아주 좋네요! 👍 👍
제가 봐도 이번 사이클 정말 잘 진행해주신게 느껴집니다.
There was a problem hiding this comment.
더 나아가 커밋 수, 그리고 변경 범위가 필요한만큼만 발생한 것 같아서 더 좋네요 💯
| @@ -38,14 +37,13 @@ private boolean isDestinationPo(Board board, Position destination) { | |||
|
|
|||
| private boolean hasOneObstacleAndNotPo(Board board, List<Position> route) { | |||
| int count = 0; | |||
| } | ||
|
|
||
| dependencies { | ||
| implementation 'com.h2database:h2:2.4.240' |
There was a problem hiding this comment.
H2 선택 잘 해주셨습니다.
저도 H2 정말 좋아해요. :)
그런데 아마 예상하지 못하셨겠지만,
제 컴퓨터에서는 이산의 DB 테스트가 모두 실패합니다.
협업, 배포, CI/CD, 그리고 인프라 구축 관련 요소에서는 이런 환경 구성이 매우 중요한데요,
이 참에 한 번 왜 DB 관련 테스트가 제 컴퓨터에서는 실패하는지 알아보시고 수정해봐도 좋을 듯 해요.
힌트를 드리면 H2는 다양한 모드로 동작시킬 수 있습니다.
메모리 위에 올리는 것도 가능하고요. (in-memory)
여기 build.gradle에서도 implementation 말고 test 관련 의존성을 추가할 수도 있으니
한 번 잘 바꿔보면 어떨까 싶어요.
| while (!isGameOver) { | ||
| playGame(team, board); | ||
| team = team.next(); | ||
| playGame(turn, board); |
There was a problem hiding this comment.
게임을 진행 할 때 턴 검증이 없어서 상대 기물을 움직일 수 있어요!
이 부분 수정이 필요할 듯 합니다.
| public static boolean isInAnyPalace(Position position) { | ||
| return CHO.isInPalace(position) || HAN.isInPalace(position); | ||
| } |
There was a problem hiding this comment.
Q. static은 언제 쓰는 게 현명한 것인가요?
지금 이산의 방식 좋다고 생각합니다.
먼저 짚고 싶은 부분이 있습니다.
대안을 설명하시면서 'Position이 Palace를 알고 있다는 사실이 이상하다고 느껴진다'고 하셨는데,
지금 Position의 import문을 한 번 확인해보세요.
이미 Palace.isInAnyPalace()를 호출하고 있어서,
static이든 instance method든 Position이 Palace에 의존하는 건 마찬가지입니다.
오히려 저는 static 으로 유지해도, 아니면 대안으로 바꿔도 다 충분히 좋다고 생각합니다.
static 메서드 자체에 대해서는,
Palace의 CHO와 HAN은 enum이기 때문에 결국 고정된 객체예요.
isInAnyPalace는 이 고정된 두 객체에 대해 질의하는 편의 메서드인데,
이런 용도의 static은 유틸리티 클래스의 static과는 성격이 다릅니다.
이미 현명하게 잘 만들어주셨다고 생각해요. :)
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public class JanggiRepository { |
There was a problem hiding this comment.
Q. 트랜잭션 적용에 관하여
제가 알기로 이번 미션에서는 트랜잭션까지 고려할 필요는 없습니다.
애초에 DB 를 제대로 연결하고, JDBC 관련 새로운 api로 SQL을 다루는 것만으로도 엄청 난이도가 높거든요.
그래도 만약 해본다면
말해주신 것처럼 만들어주신 repository 에서 구현해보는게 매우 적절해보입니다.
DAO가 활용되고 있는 곳이니까요.
구체적으로 왜 그런지 그 이유를 한 번 고민해보셔도 좋을 듯 하네요.
service = transactional 적용하는 곳 이 아닌,
지금 이산이 생각하신 repository 에 적용하려는 확실한 이유를 찾는다면 굉장히 좋은 학습이 될 것 같습니다!
(생각보다 간단한 이유입니다)
더 나아가 트랜잭션을 적용할 때 꼭 레이어가 구분될 필요도 없습니다. (이거 뭔가 우리 만나서 얘기했던 것 같네요 ㅋㅋㅋ)
어떻게 보면 그냥 'DB에 언제 commit 할지 내가 정하겠다’를 코드로 표현하는 거라서요.
애초에 '트랜잭션을 적용한다’는 표현은 너무 @Transactional 어노테이션을 적용하는걸 의식한 표현이고,
그냥 '하나의 트랜잭션으로 묶겠다’는게 좀 더 정확한 표현인 것 같습니다.
위의 말이 와닿지 않거나 추상적으로 들린다면 AI 에게 이 말을 그대로 복붙해서 같이 함께 학습해보시죠 :)
| GameEntity game1 = new GameEntity(null, Team.HAN, false); | ||
| Long savedId1 = gameDao.save(game1); | ||
| GameEntity game2 = new GameEntity(null, Team.CHO, false); | ||
| Long savedId2 = gameDao.save(game2); | ||
| GameEntity game3 = new GameEntity(null, Team.CHO, false); | ||
| Long savedId3 = gameDao.save(game3); | ||
| GameEntity game4 = new GameEntity(null, Team.HAN, false); | ||
| Long savedId4 = gameDao.save(game4); | ||
| GameEntity game5 = new GameEntity(null, Team.CHO, false); | ||
| Long savedId5 = gameDao.save(game5); | ||
| GameEntity game6 = new GameEntity(null, Team.HAN, false); | ||
| Long savedId6 = gameDao.save(game6); |
| @BeforeEach | ||
| void 테스트_시작() { | ||
| DbConnector.initDatabase(); | ||
| gameDao = new GameDao(); | ||
| } | ||
|
|
||
| @AfterEach | ||
| void 테스트_종료() { | ||
| try (Connection connection = DbConnector.getConnection(); | ||
| Statement statement = connection.createStatement()) { | ||
| statement.execute("TRUNCATE TABLE pieces"); | ||
| statement.execute("SET REFERENTIAL_INTEGRITY FALSE"); | ||
| statement.execute("TRUNCATE TABLE games"); | ||
| statement.execute("SET REFERENTIAL_INTEGRITY TRUE"); | ||
| } catch (SQLException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| } |
There was a problem hiding this comment.
오 테스트 시작과 끝에 추가 작업을 하게 해주셨군요?
이 작업은 어떤 이유로 하게 되었나요?
틀렸다는건 아니고, 오히려 잘했는데 그냥 이산의 생각을 듣고 싶어서 남기는 리뷰입니다.
| } | ||
|
|
||
| @Test | ||
| @DisplayName("진행 중인 게임을 최대 5개 가져올 수 있다") |
There was a problem hiding this comment.
테스트 이름에 중복이 있어요.
그리고 왜 이렇게 두 테스트가 존재하는지 처음 봤을 때는 명확하게 이해하기 힘든 상황인데,
알기 쉽게 바꿔봐도 좋을 듯 해요.
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.CsvSource; | ||
|
|
||
| public class PalaceTest { |
2 participants
안녕하세요 피케이! 이산입니다⛰️
금요일에 직접 만나뵙게 되어 매우 영광이었습니다!! 항상 감사드립니다.
장기 사이클2를 월요일까지 해야 한다는 말을 들어서 우선은 기능 요구사항에 있는 것들을 중점으로 구현을 완료했습니다. 그래서 장군, 멍군, 외통수를 알려주는 기능을 넣으려다가 우선은 빼놓고 진행했습니다.
사이클1에서 잘 리팩토링된 코드를 통해 사이클2를 진행하다보니 궁성 영역은 생각보다 수월하게 할 수 있었습니다!
DB의 경우 간단하게 사용하기 편리한 H2를 사용했습니다.
체크 리스트
test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?어떤 부분에 집중하여 리뷰해야 할까요?
1. Palace 확인 로직의 위치와 static 활용에 대하여(static은 언제 쓰는 게 현명한 것인가요?)
현재 궁성 여부를 확인하는
isInAnyPalace메서드를 static으로 구현해 사용 중입니다. 장기판의 궁성 위치는 변하지 않는 고정된 규칙이라고 생각했기 때문에 static을 이용해도 될 것 같아서 위와 같이 만들어서 사용했습니다.추가적인 대안으로, Position 클래스 안에
checkIsInAnyPalace와 같은 private 메서드를 추가해서위와 같은 방식으로 구현하여 Position이 스스로 궁성 여부를 판단하게 하면 static은 뺄 수 있다고 생각했습니다. 하지만 Position이 Palace를 알고 있다는 사실이 이상하다고 느껴지는데 어떤 방식이 좋은 것인지 고민입니다.
2. DAO와 Repository의 역할 분리에 관하여
이번 장기 미션을 수행하면서 DTO, DAO를 모두 사용해보았습니다.
DTO는 기존에도 사용해본 경험이 있었지만 DAO의 경우, 이번에 정말 처음 사용해보게 된 것 같습니다.
기존에는 sql문을 DAO 없이 Repository 안에서 다 처리를 해버리는 방식으로 해봤었는데, 확실히 DAO에서 sql로 DB와 직접 소통을 하고 Repository에서는 DAO들을 조합해서 사용하니 코드가 훨씬 깔끔하게 잘 정리가 되는 것 같다는 느낌을 받았습니다.
DAO를 써본다고 써본 것인데 제가 한 방식이 올바른 방식인지 궁금합니다.
3. 엔티티의 구분과 식별성에 관하여 (도메인 엔티티 vs DB 엔티티)
제가 이번 미션을 진행하면서 엔티티에 관하여 헷갈렸던 부분이 있었습니다.
엔티티는 고유한 식별성을 가지고 있는 객체라고 생각했는데,
그렇게 생각을 해봤을 때 DB와 1:1로 매핑이 되는
DB 엔티티의 경우 id를 명시적인 필드로 가지게 됩니다.하지만 비즈니스 로직을 처리하는
도메인 엔티티의 경우 id를 필수적으로 가지고 있진 않지만 메모리상에서 주소값을 통해 식별성을 유지하기 때문에 엔티티의 역할을 수행한다고 생각했습니다.따라서 저는 엔티티를 생각할 때 도메인 엔티티와 DB 엔티티로 구분해서 생각을 해보았는데, 이에 대해 제가 제대로 이해한 것이 맞을까요?
4. 트랜잭션 적용에 관하여
사이클2를 구현하는 과정에서 많은 크루들이 트랜잭션에 관하여 많이 고민하는 것들을 지켜봤습니다.
저는 아직 트랜잭션을 미적용해본 상황이지만, 기존에 트랜잭션을 사용해봤을 때는 서비스 계층에서 트랜잭션을 수행했었던 것 같은데 이번 미션에서는 따로 서비스 계층을 두지 않았는데 어디에서 트랜잭션을 적용해보면 좋을지 궁금합니다. Repository에서 적용해보는 것이 좋을까요?
그리고 이번 미션에 있어서 트랜잭션을 어느 정도 깊이까지 알아가면 좋을지 궁금합니다.