-
Notifications
You must be signed in to change notification settings - Fork 1
#26 송금 요청 수락/거절 기능 #27
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
Conversation
f-lab-maverick
left a comment
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.
깔끔하게 잘 작성해주셨네요~ 약간의 수정사항 확인 부탁드립니다.
| final var accounts = accountLockingService.getAccountsWithLockOrdered(remittanceRequest.getSenderId(), remittanceRequest.getReceiverId()); | ||
| final var senderAccount = remittanceRequest.getSenderId().equals(accounts.get(0).getMemberId()) ? accounts.get(0) : accounts.get(1); | ||
| final var receiverAccount = remittanceRequest.getReceiverId().equals(accounts.get(0).getMemberId()) ? accounts.get(0) : accounts.get(1); |
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.
이 코드는 좀 더 객체지향적인 방식으로 변경하면 안정성있는 코드로 변경할 수 있습니다.
현재는 lock ordering을 위해 일단 파라미터를 넘기면 리스트형태의 순서를 모르는 값이 리턴되는 형태이고, 거기서 수신자와 송금자를 찾는 방식입니다. 이 방식은 일단 명확하지 않죠. 리턴된값중에 혹시나 다른 값이 섞인다면 엉뚱한 계좌 값을 송금자라고 인식할 수 있으니까요.
이런 경우 컬랙션을 직접 다루지 말고 클래스로 다루는것이 좀 더 안전한 코드 작성 방식입니다.
| final var accounts = accountLockingService.getAccountsWithLockOrdered(remittanceRequest.getSenderId(), remittanceRequest.getReceiverId()); | |
| final var senderAccount = remittanceRequest.getSenderId().equals(accounts.get(0).getMemberId()) ? accounts.get(0) : accounts.get(1); | |
| final var receiverAccount = remittanceRequest.getReceiverId().equals(accounts.get(0).getMemberId()) ? accounts.get(0) : accounts.get(1); | |
| final LockedAccounts accounts = accountLockingService.getAccountsWithLockOrdered(remittanceRequest.getSenderId(), remittanceRequest.getReceiverId()); | |
| final var senderAccount = lockedAccount.getSenderAccount(); | |
| final var receiverAccount = lockedAccount.getReceiverAccount(); |
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.
여러 개의 잠금 계좌 객체를 다뤄야 하는 상황에서, LockedAccounts 내에서 송금자와 수신자 계좌를 찾는 로직을 캡슐화할 수 있어 좋은 구조인 것 같습니다. 좋은 인사이트 감사합니다 😊
| public void handleExpiration() { | ||
| // 요청 만료 | ||
| } |
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.
아직 완성되지 않은 메서드는 제거해주세요. 완성된 이후에 올려주시길 부탁드립니다~
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.
제거하였습니다!
| @Transactional(readOnly = true) | ||
| public Account getByMemberIdForUpdate(final long senderId) { |
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.
X Lock의 획득을 위한 메서드인데 readOnly 옵션을 주는게 적절할까요?
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.
X Lock을 사용하는 쿼리는 데이터를 수정하기 위한 목적의 조회이므로 readOnly 옵션 제거하였습니다!
f-lab-maverick
left a comment
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.
고생하셨습니다. 머지해주셔도 좋습니다.
@f-lab-maverick
안녕하세요! 송금 요청 수락/거절 기능 개발 완료되어 리뷰 요청 드립니다.
주요 변경 사항
상세 구현 내용
handleAcceptance(requestId, receiverId): 수락 처리handleRejection(requestId, receiverId): 거절 처리→ 두 메서드 모두 수신자 권한 및 요청 상태 검증을 포함하며, 처리 완료 후 상태 히스토리를 저장합니다.