[3주차] 예약 API 구현 및 Redisson Lock 구현#70
Open
jmParkGit wants to merge 5 commits intohanghae-skillup:jmParkGitfrom
Open
[3주차] 예약 API 구현 및 Redisson Lock 구현#70jmParkGit wants to merge 5 commits intohanghae-skillup:jmParkGitfrom
jmParkGit wants to merge 5 commits intohanghae-skillup:jmParkGitfrom
Conversation
youngxpepp
approved these changes
Jan 28, 2025
youngxpepp
left a comment
youngxpepp
left a comment
There was a problem hiding this comment.
안녕하세요 재민님. 이건홍 코치라고 합니다ㅎㅎ
과제하느라 고생하셨고 코드 리뷰 보면서 궁금하신 점 있다면 코멘트 남겨주세요.
감사합니다!
좋았던 점
- redisson의 tryLock 잠금을 통해 영화 예매에 대한 데이터 무결성을 지켰습니다.
아쉬운 점
- 잠금을 잘 구현해주셨는데 이에 대한 테스트가 없어서 아쉽네요ㅠㅠ 꼭 코드를 실행해보셨으면 좋겠습니다.
- 요구사항 중에 FCM으로 메시지 발송을 하는 내용이 있는데 이 부분이 없어서 아쉽네요! 요구사항이 몇 개 더 있는데,, 3주차 시나리오에 맞춰서 구현을 하셨다면 어땠을까요! 아쉽습니다!!
Comment on lines
+49
to
+55
| Movie movie = movieRepository.findById(id).orElseThrow( | ||
| () -> new NullPointerException("There is no id at DB.") | ||
| ); | ||
|
|
||
| if(movie==null) { | ||
| return movie; | ||
| } |
There was a problem hiding this comment.
Suggested change
| Movie movie = movieRepository.findById(id).orElseThrow( | |
| () -> new NullPointerException("There is no id at DB.") | |
| ); | |
| if(movie==null) { | |
| return movie; | |
| } | |
| Movie movie = movieRepository.findById(id).orElseThrow( | |
| () -> new NullPointerException("There is no id at DB.") | |
| ); |
movie에 대해 orElseThrow를 하셨다면 movie가 null일 수는 없겠네요!
if 조건문은 없어도 되지 않을까요?ㅎㅎ
Comment on lines
+60
to
+63
| if (!acquireLock) { | ||
| System.out.println("Lock get fail"); | ||
| return movie; | ||
| } |
There was a problem hiding this comment.
예약이 실패됐다 라는 내용이 사용자에게 전달되려면 어떻게 해야 할까요?
Comment on lines
+52
to
+53
| @Column | ||
| private Boolean[] seats = new Boolean[MAX_SEATS]; |
There was a problem hiding this comment.
혹시 이 부분 테스트 해보셨을까요?
boolean 배열이 디비에 저장되기 위해선 적절한 변환 과정이 선언되어야 할듯 합니다.
| return movie; | ||
| } | ||
|
|
||
| RLock lock = redissonClient.getLock(id.toString()); |
There was a problem hiding this comment.
영화 전체에 대한 잠금이 걸려있네요!
영화 - 상영관 - 좌석 (5x5)
좌석은 상영관 별로 25개씩 있고, 상영관은 여러개 존재할 수 있어요.
잠금을 여러 개로 나눈다면 사용자가 덜 기다릴 수도 있지 않을까요?
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
[3주차] 예약 API 구현 및 Redisson Lock 사용
작업 내용
발생했던 문제와 해결 과정을 남겨 주세요.
이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.
리뷰 포인트
기타 질문