임시저장 시 필드값이나 게임 갯수가 모자라도 저장되도록 구현 #798
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough
Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/main/java/balancetalk/game/domain/TempGame.java (1)
57-64: 옵션 업데이트 로직이 개선되었습니다.기존 옵션 업데이트와 새 옵션 추가를 명확하게 구분하여 처리하고 있습니다. 다만, 다음 사항을 고려해보시기 바랍니다:
- 기존 옵션이 새 옵션보다 많은 경우 처리가 필요할 수 있습니다.
- 불필요한 옵션을 제거하는 로직 추가를 고려해보세요.
public void updateTempGame(TempGame newTempGame) { this.description = newTempGame.getDescription(); + // 불필요한 옵션 제거 + while (this.tempGameOptions.size() > newTempGame.getTempGameOptions().size()) { + this.tempGameOptions.remove(this.tempGameOptions.size() - 1); + } IntStream.range(0, newTempGame.getTempGameOptions().size()).forEach(i -> { if (i < this.tempGameOptions.size()) { TempGameOption currentOption = this.tempGameOptions.get(i); TempGameOption newOption = newTempGame.getTempGameOptions().get(i); currentOption.updateTempGameOption(newOption); } else { TempGameOption newOption = newTempGame.getTempGameOptions().get(i); this.addTempGameOption(newOption); } }); }src/main/java/balancetalk/game/application/TempGameService.java (1)
47-51: 임시 저장 시 빈 게임 목록 처리가 추가되었습니다.빈 게임 목록 처리가 잘 구현되었습니다. 다만, 다음 사항들을 고려해보시기 바랍니다:
- 로깅 추가를 통한 모니터링 개선
- 빈 게임 목록으로 저장되는 경우에 대한 사용자 피드백
if (request.getTempGames() == null || request.getTempGames().isEmpty()) { + log.info("임시 저장: 빈 게임 목록으로 저장됨 - 회원 ID: {}", member.getId()); TempGameSet tempGameSet = request.toEntity(member); tempGameSetRepository.save(tempGameSet); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/main/java/balancetalk/game/application/TempGameService.java(1 hunks)src/main/java/balancetalk/game/domain/TempGame.java(1 hunks)src/main/java/balancetalk/game/domain/TempGameOption.java(0 hunks)src/main/java/balancetalk/game/domain/TempGameSet.java(0 hunks)src/main/java/balancetalk/game/dto/TempGameSetDto.java(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/balancetalk/game/domain/TempGameOption.java
- src/main/java/balancetalk/game/domain/TempGameSet.java
🧰 Additional context used
📓 Learnings (2)
src/main/java/balancetalk/game/dto/TempGameSetDto.java (1)
Learnt from: jschoi-96
PR: JECT-Study/PICK-O-Server#713
File: src/main/java/balancetalk/game/domain/TempGameSet.java:65-71
Timestamp: 2024-11-19T13:42:37.389Z
Learning: `TempGameSet.java`의 `updateTempGameSet` 메서드에서 `tempGames`의 크기에 대한 추가 검증은 불필요하다. 이 크기 검사는 이미 메서드 호출 전에 수행되고 있다.
src/main/java/balancetalk/game/domain/TempGame.java (1)
Learnt from: jschoi-96
PR: JECT-Study/PICK-O-Server#713
File: src/main/java/balancetalk/game/domain/TempGame.java:52-55
Timestamp: 2024-11-14T15:21:06.978Z
Learning: In `TempGame.java`, the `updateTempGame` method already checks the array size before iteration, preventing `IndexOutOfBoundsException`.
🔇 Additional comments (2)
src/main/java/balancetalk/game/domain/TempGame.java (1)
49-52: 새로운 도우미 메서드가 잘 구현되었습니다.
양방향 연관관계 설정이 올바르게 구현되어 있습니다.
src/main/java/balancetalk/game/dto/TempGameSetDto.java (1)
18-18: 임시 저장 시 게임 수 제한이 제거되었습니다.
PR의 목적에 맞게 게임 수 제한이 제거되었습니다. 하지만 다음 사항을 검토해주세요:
- 프론트엔드에서도 관련 제한이 제거되었는지 확인이 필요합니다.
- 향후 게임 수 제한이 필요할 수 있으므로, 설정 값으로 관리하는 것을 고려해보세요.
Hanjaemo
approved these changes
Dec 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
💡 작업 내용
💡 자세한 설명
필드 제약 조건 삭제
필드 제약 조건을 삭제하여 제목, 선택지와 같은 값들이 공백이여도 저장되도록 구현했습니다. 또한, 기존에 게임 숫자가 10개가 되는지 체크하는 예외처리 로직이 있었는데 삭제했습니다.
게임이 비어있는 경우 예외처리
Cannot invoke "java.util.List.stream()" because "tempGames" is null과 같은 NPE 에러가 발생하기 때문에 예외처리를 해줬습니다.선택지를 1개만 저장했을 때 예외처리
이런식으로 선택지를 하나만 저장하고, 새롭게 불러와서 선택지를 추가할 때
Index 1 out of bounds for length 1문제가 발생했습니다.따라서, else문을 통해서 사이즈가 다른 경우엔, 추가되는 게임 옵션만 추가해주도록 구현했습니다.
📗 참고 자료 (선택)
📢 리뷰 요구 사항 (선택)
🚩 후속 작업 (선택)
✅ 셀프 체크리스트
closes #791
Summary by CodeRabbit
New Features
TempGame클래스에 게임 옵션 추가 기능 추가.Bug Fixes
Documentation
Refactor
Style