Conversation
저는 이번 PR에서의
제안주신 InvalidStateException vs BusinessException 구분 기준에 대해서도 매우 공감합니다. |
좋은 피드백 감사합니다 🙏 예를 들어 roomParticipant.cancelParticipation()이라는 표현은 실제로 참여가 취소될 것처럼 읽히지만, 내부적으로는 권한 검증 후 예외만 던지는 구조이기 때문에, 사용자의 입장에서 의도와 구현 사이에 혼동이 생길 수 있다고 판단했습니다. 이러한 관점에서 저는 validateCancelable()나 ensureCancelable()처럼 검증 메서드임을 명확히 드러내는 네이밍이 유지보수나 확장성 측면에서 더 안전하다고 생각하고 있습니다. @seongjunnoh 혹시 성준님은 이 부분에 대해 어떻게 생각하시나요? 팀 컨벤션 논의 차원에서도 함께 이야기 나눠보면 좋을 것 같습니다! |
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
간단한 리뷰 남겼습니다~~
그리고 현준님이 개발해주신 방 참가신청/참가취소/모집마감 api 의 Service 코드를 보면서 공유하고 싶은 고민거리가 생겼습니다
현재 모든 adapter의 도메인 조회 메서드에서 Optional<도메인> 이 아닌 도메인 을 반환하고, 메서드 내부에서 jpa entity 가 존재하지 않을 경우 EntityNotFoundException을 throw 하는 식으로 구현되어 있습니다.
그런데 이렇게 반환타입이 Optional 이 아니니 현준님이 개발하신 APi의 Service 로직처럼 엔티티의 존재 유무에 따라 분기처리가 필요할 경우 service 에 try-catch가 다수 발생하거나, Optional<도메인> 을 반환하는 메서드를 추가하는 상황이 발생하고 있습니다.
제가 처음에 전체적인 service-port-adapter 의 구조를 생각할 때는 service는 에외처리에 대한 책임에서 자유로운게 좋지않나 라는 생각에 adapter 의 find 관련 메서드의 반환타입을 Optional 이 아니라 도메인으로 설정했었는데, 지금보니 생각보다 Service 에서 Optional<도메인> 의 상태에 따라 분기처리가 발생하는 로직이 꽤 나오는 거 같습니다.
- adapter 의 find 관련 메서드의 반환타입을 Optional<도메인> 으로 수정 및 관련 예외 처리 + 분기처리는 Service 에서 진행
- adapter 에 find 관련 메서드를 Optional<도메인> 을 반환하는 메서드, 도메인을 반환하는 현재 방식의 메서드 이렇게 2개를 뚫어놓기
- Service 코드의 가독성, 중복 코드 발생 가능성 등을 감수하고 Service에서 try-catch 구문을 활용해 분기처리를 진행한다
가볍게 생각해봤을때 위 3가지의 결론이 있을거 같은데, 관련해서 논의를 한번 해보면 좋을것 같습니다!
저는 지금 현준님이 구현해주신 2번 방식처럼 Optional<도메인> 을 반환하는 메서드를 필요시 추가하는 것도 좋은것 같습니다!
(1번 처럼 모든 find 메서드의 반환타입을 통일하면 깔끔할것 같지만, Service 레이어에 매번 NOT_FOUND exception 관련 코드가 추가됨 + 지금 코드를 언제 다 수정?? 이라는 이슈가 있을 거 같네요)
| private final RoomParticipantCommandPort roomParticipantCommandPort; | ||
| private final RoomCommandPort roomCommandPort; | ||
|
|
||
| //todo 모집 마감시 방 참여자들에게 모집 마감 알림 전송 |
| // 방 참여자가 방 모집 마감을 요청한 경우 | ||
| public void closeRoomRecruit() { | ||
| if (checkRole(RoomParticipantRole.MEMBER)) { | ||
| throw new BusinessException(ErrorCode.ROOM_RECRUIT_CANNOT_CLOSED, | ||
| new IllegalArgumentException("사용자는 방의 멤버입니다. 오직 호스트만 모집 마감이 가능합니다.")); | ||
| } | ||
| } |
There was a problem hiding this comment.
LGTM
추후에 RoomParticipant 를 RoomMember, RoomHost 라는 2개의 도메인으로 분리하는 것도 논의해보면 좋을거 같습니다!
현재는 RoomParticipant 라는 도메인이 내부에 필드로 MEMBER, HOST를 구분하는 Role을 가지고 있는데, 아예 jpa -> 도메인으로의 매핑시에 jpa의 role을 보고 RoomMember, RoomHost 이렇게 2개의 도메인으로 매핑하면 전반적으로 코드의 가독성 + member/host의 도메인 로직 책임 분리 가 훨씬 좋아지지 않을까 싶습니다
There was a problem hiding this comment.
좋은 리뷰 감사합니다!!
다만 현재 저희 서비스에서 RoomHost와 RoomMember 간의 역할 차이가 실질적으로 드러나는 부분이 모집 마감 또는 참여 취소 API 정도에 한정되어 있어, 지금 단계에서 두 도메인을 분리하는 것이 적절한지는 조금 더 고민이 필요할 것 같습니다.
추후에 요구사항 중 ‘특정 사용자 퇴장’이나 ‘방 정보 수정’ 등 역할에 따른 책임이 명확히 구분되는 기능이 추가된다면, 도메인 분리로 인한 이점이 분명히 생길 것이라 생각합니다. 그런 시점에서 다시 논의해보면 좋을 것 같습니다!
저는 개인적으로 @hd0rable 님 말처럼 특정 객체가 제공하는 메서드는 해당 객체가 수행, 제공할 수 있는 기능을 나타내는게 좋다고 생각합니다. 이러면 자연스럽게 서비스 레이어에서는 비즈니스 로직을 객체들의 communication을 통해 구현할 수 있는(서비스에서 Object의 데이터를 직접 활용해 로직을 구현하는게 아니라) 이점이 있다고 생각하기 때문입니다. 그런데 지금 현준님이 구현하신 Service 로직을 보니, 'HOST는 방 참여 취소 안됨', 'MEMBER는 방 모집마감 안됨' 이라는 요구사항을 구현하실 때
이런 플로우인 거 같은데, 이러면 그냥 도메인에게 Role 값을 전달받은 후 로직을 전부 Service 에서 처리하는 건 어떤가요?? 예시코드class RoomJoinService {
,,,
private void handleCancel() {
// 1. if (participant.getRole().equals('HOST')) : throw BusinessException
// 2. 이후 방 참여 취소 로직 마저 수행 (= soft delete)
}
}
class RoomRecruitCloseService {
,,,
public closeRoomRecruit( ,,, ) {
,,,
// 방 모집 마감
if (roomParticipant.getRole().equals("MEMBER")) : throw BusinessException
// 이후 방 모집 마감 로직 마저 수행
}
}현재 방 참여/취소/모집 마감 의 Service 코드 흐름 상, 위 예시 코드처럼 RoomParticipant 내에 네이밍이 애매한 메서드를 굳이 생성하지 않고, Service 에서 MEMBER, HOST 상태에 따른 분기처리를 수행하는 것이 훨씬 가독성이 올라간다고 생각합니다! |
넵 다시 생각해보니 도메인에서 상태변화를 하는 것이 아니니 굳이 도메인 계층에서 검증하지 않고 서비스 쪽에서 담당해도 될 것 같네요! 좋은 리뷰 감사합니다~ |
저는 현재 상황에서는 두 번째 방법, 즉 Optional<도메인>을 반환하는 메서드를 필요에 따라 추가하는 방식이 가장 적절하다고 생각합니다. 그 이유는, 저희는 기능별로 Service 클래스를 명확하게 분리하고 있고, 레이어드 아키텍처를 사용했다면 Service 내에서 EntityNotFoundException을 직접 처리해야 할 경우가 많았을 것입니다. 하지만 현재는 헥사고날 아키텍처를 도입하여 adapter에서 예외 처리를 책임지도록 구현하고 있기 때문에, 중복 코드가 Service 레이어에 반복적으로 등장하는 문제를 adapter 단에서 재사용을 통해 어느 정도 해소할 수 있습니다. 따라서, 도메인의 존재 여부에 따라 분기 처리가 필요한 경우에만 Optional을 반환하는 메서드를 adapter에 추가하고, 그 외에는 기존처럼 도메인을 바로 반환받는 방식을 유지하는 것이 좋을 것 같습니다. 이렇게 하면 코드의 재사용성과 유지보수성을 높이면서도 불필요한 예외 처리 중복을 피할 수 있다고 생각합니다. |
|
@seongjunnoh @hd0rable 성준님이 노션에 정리해주신 것 토대로 4번째 방법 도입해서 한번 적용시켜보았습니다! 한번 확인 부탁드릴게요~ |
![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
🧹 Nitpick comments (2)
src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java (1)
25-26: 코드 스타일을 개선해주세요.파일 끝의 불필요한 빈 줄들을 제거하여 코드 일관성을 유지해주세요.
- - +src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java (1)
60-71: 도메인 객체에서 엔티티로의 필드 매핑이 올바르게 구현되었습니다
updateFrom메소드가 도메인 객체의 모든 필드를 적절히 매핑하고 있으며, 이전의updateMemberCount메소드보다 포괄적인 업데이트를 지원합니다. 메소드 체이닝을 위한this반환도 적절합니다.다만 다음 사항들을 고려해보세요:
room파라미터에 대한 null 체크가 없습니다room.getHashedPassword()가 null일 경우 엔티티의 password 필드에 null이 할당될 수 있습니다public RoomJpaEntity updateFrom(Room room) { + if (room == null) { + throw new IllegalArgumentException("Room cannot be null"); + } this.title = room.getTitle(); this.description = room.getDescription(); this.isPublic = room.isPublic(); this.password = room.getHashedPassword(); this.roomPercentage = room.getRoomPercentage(); this.startDate = room.getStartDate(); this.endDate = room.getEndDate(); this.recruitCount = room.getRecruitCount(); this.memberCount = room.getMemberCount(); return this; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/main/java/konkuk/thip/book/domain/SavedBooks.java(3 hunks)src/main/java/konkuk/thip/record/application/service/RecordCreateService.java(3 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java(2 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java(3 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomGetMemberListService.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomJoinService.java(2 hunks)src/main/java/konkuk/thip/room/application/service/RoomRecruitCloseService.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomShowPlayingDetailViewService.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomShowRecruitingDetailViewService.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomVerifyPasswordService.java(1 hunks)src/main/java/konkuk/thip/room/domain/Room.java(3 hunks)src/main/java/konkuk/thip/room/domain/RoomParticipant.java(1 hunks)src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java(3 hunks)src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomCloseJoinApiTest.java(1 hunks)src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java(6 hunks)src/test/java/konkuk/thip/room/domain/RoomTest.java(2 hunks)src/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.java(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java
- src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java
🚧 Files skipped from review as they are similar to previous changes (9)
- src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java
- src/test/java/konkuk/thip/room/domain/RoomTest.java
- src/main/java/konkuk/thip/room/domain/RoomParticipant.java
- src/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java
- src/main/java/konkuk/thip/room/application/service/RoomRecruitCloseService.java
- src/main/java/konkuk/thip/room/domain/Room.java
- src/main/java/konkuk/thip/record/application/service/RecordCreateService.java
- src/test/java/konkuk/thip/room/adapter/in/web/RoomCloseJoinApiTest.java
- src/main/java/konkuk/thip/room/application/service/RoomJoinService.java
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#57
File: src/test/java/konkuk/thip/room/domain/RoomTest.java:0-0
Timestamp: 2025-07-08T16:30:33.771Z
Learning: Room 도메인에서 startDate는 현재 날짜 이후여야 하는 도메인 규칙이 있어서, 테스트에서 만료된 상태를 시뮬레이션하려면 reflection을 사용해야 한다.
src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java (1)
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#75
File: src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryRepositoryImpl.java:50-83
Timestamp: 2025-07-14T14:19:38.796Z
Learning: Vote와 VoteItem 엔티티는 자주 함께 사용되므로, N+1 문제를 방지하기 위해 양방향 매핑과 fetch join을 고려하는 것이 좋습니다. 특히 기록장 조회 API 등에서도 함께 사용될 가능성이 높습니다.
src/main/java/konkuk/thip/room/application/service/RoomShowPlayingDetailViewService.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java (2)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java (2)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
🧬 Code Graph Analysis (2)
src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java (1)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)
src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java (1)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)
🔇 Additional comments (16)
src/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.java (1)
4-4: 서비스 계층에서 BusinessException 사용이 적절합니다.PR 토론에서 논의된 예외 처리 전략에 따라 서비스 계층에서는
BusinessException을, 도메인 계층에서는InvalidStateException을 사용하는 것으로 합의되었습니다.UserFollowService는 서비스 계층이므로BusinessException으로 변경하는 것이 올바른 접근입니다.Also applies to: 148-148, 160-160
src/main/java/konkuk/thip/book/domain/SavedBooks.java (1)
3-3: 도메인 계층에서 InvalidStateException 사용이 적절합니다.PR 토론에서 합의된 예외 처리 전략에 따라 도메인 계층에서는
InvalidStateException을 사용하여 도메인 규칙 위반을 나타내는 것이 올바릅니다.SavedBooks는 도메인 클래스이고, 여기서 수행되는 검증들(중복 책 검사, 저장 가능성 검증, 삭제 가능성 검증)은 모두 도메인 불변성을 보장하는 로직이므로InvalidStateException사용이 적절합니다.Also applies to: 21-21, 30-30, 37-37
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java (2)
18-18: Optional 임포트 추가가 적절합니다.새로운 Optional 반환 메서드를 위해 필요한 임포트가 올바르게 추가되었습니다.
63-67: Optional 반환 메서드 구현이 우수합니다.PR 논의에서 언급된 "옵션 2: 어댑터에서 Optional을 반환하는 메서드와 예외를 던지는 메서드 두 가지 버전 제공" 방식을 잘 구현했습니다. 메서드명에
Optional접미사를 사용하여 반환 타입을 명확히 나타내고, 깔끔한 매핑 로직을 제공합니다.src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java (3)
3-4: 필요한 임포트가 올바르게 추가되었습니다.새로운 Optional 반환 메서드와 기본 메서드 구현을 위해 필요한 임포트들이 적절히 추가되었습니다.
Also applies to: 8-8
12-12: 메서드 시그니처 변경이 적절합니다.PR 논의에서 언급된 "어댑터 find 메서드들이
Optional<Domain>을 반환하도록 변경" 방식을 잘 구현했습니다. 메서드명의Optional접미사로 반환 타입을 명확히 나타냅니다.
14-17: 기본 메서드 구현이 우수합니다.기존 코드의 호환성을 유지하면서 예외 던지기 패턴을 제공하는 훌륭한 설계입니다. Optional 메서드를 활용하여 DRY 원칙을 준수하고, 일관된 예외 처리를 제공합니다. 이는 PR 논의에서 선호된 "옵션 2" 방식을 잘 구현한 것입니다.
src/main/java/konkuk/thip/room/application/service/RoomVerifyPasswordService.java (1)
23-23: 메서드 변경이 적절하며 에러 처리 일관성을 개선합니다.
findById에서getByIdOrThrow로의 변경은 방 비밀번호 검증 로직에 적합합니다. 존재하지 않는 방에 대한 비밀번호 검증은 의미가 없으므로, 예외를 발생시키는 것이 더 명확한 에러 처리 방식입니다.src/main/java/konkuk/thip/room/application/service/RoomShowRecruitingDetailViewService.java (1)
35-35: 일관성 있는 리팩토링으로 에러 처리가 개선되었습니다.모집중인 방의 상세 정보 조회 시 방이 존재하지 않으면 예외를 발생시키는 것이 적절합니다. 다른 Room 관련 서비스들과 일관된 에러 처리 패턴을 따르고 있습니다.
src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java (1)
60-60: 투표 검증 로직에서 적절한 에러 처리 개선입니다.투표 생성 시 방 검증 단계에서 방이 존재하지 않으면 즉시 예외를 발생시키는 것이 올바른 접근입니다. 이후 도서 페이지 검증 로직이 의미 있게 수행되려면 유효한 방 객체가 필요합니다.
src/main/java/konkuk/thip/room/application/service/RoomGetMemberListService.java (1)
31-31: 방 멤버 목록 조회에서 적절한 에러 처리 개선입니다.방 멤버 목록을 조회하기 위해서는 방이 반드시 존재해야 하므로,
getByIdOrThrow사용이 적절합니다. 이후 참여자 조회 및 사용자 정보 배치 쿼리 로직이 의미 있게 수행될 수 있습니다.src/main/java/konkuk/thip/room/application/service/RoomShowPlayingDetailViewService.java (1)
35-35: 진행중인 방 상세 조회에서 일관된 에러 처리 패턴을 완성합니다.진행중인 방의 상세 정보를 조회할 때 방이 존재하지 않으면 예외를 발생시키는 것이 적절합니다. 모든 Room 관련 서비스에서 일관된 에러 처리 패턴을 적용하여 코드베이스의 일관성이 향상되었습니다.
src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java (4)
3-3: 필요한 import 문들이 적절히 추가되었습니다새로운 기능을 위한
EntityNotFoundException,Optional, 그리고ErrorCode.ROOM_NOT_FOUNDimport가 올바르게 추가되었습니다.Also applies to: 7-7, 9-9
13-13: Optional 반환 타입으로의 변경이 아키텍처 개선에 도움이 됩니다
findById메소드의 반환 타입을Optional<Room>으로 변경한 것은 PR 목표에서 언급된 repository 메소드 설계 개선사항과 일치합니다. 이는 서비스 레이어에서 엔티티 존재 여부를 명시적으로 처리할 수 있게 해줍니다.
15-18: 편의성을 위한 getByIdOrThrow 메소드가 잘 구현되었습니다PR 목표에서 논의된 "두 가지 버전 제공" 접근법을 따라 Optional을 반환하는
findById와 예외를 던지는getByIdOrThrow를 모두 제공하는 것은 좋은 설계입니다. 기존 코드의 안정성을 유지하면서 유연성을 제공합니다.default 메소드 구현도 적절하며,
ROOM_NOT_FOUND에러 코드를 사용한 예외 처리가 일관성 있게 되어 있습니다.
24-24: 메소드 이름 변경이 기능 확장을 잘 반영합니다
updateMemberCount에서update로 메소드명을 변경한 것은 이제 더 포괄적인 업데이트 기능을 제공한다는 점을 잘 반영합니다.RoomJpaEntity.updateFrom()메소드와 함께 여러 필드를 한 번에 업데이트할 수 있게 되었습니다.
| void updateMemberCount(Room room); | ||
|
|
||
| void updateRoomStartDate(Room room); | ||
| void update(Room room); |
| Room room = roomCommandPort.findByIdOptional(roomJoinCommand.roomId()) | ||
| .orElseThrow(() -> new BusinessException(ErrorCode.USER_CANNOT_JOIN_OR_CANCEL)); |
| private void validateCloseable(RoomParticipant roomParticipant) { | ||
| if (roomParticipant.isMember()) { | ||
| throw new BusinessException(ErrorCode.ROOM_RECRUIT_CANNOT_CLOSED, | ||
| new IllegalArgumentException("사용자는 방의 멤버입니다. 오직 호스트만 모집 마감이 가능합니다.")); | ||
| } | ||
| } |
| RoomParticipant roomParticipant = roomParticipantCommandPort.getByUserIdAndRoomIdOrThrow(command.userId(), command.roomId()); | ||
| Room room = roomCommandPort.getByIdOrThrow(record.getRoomId()); |
There was a problem hiding this comment.
p3 : 이 메서드 네이밍은 왜 findBy,,, 가 아니라 getBy,,,OrThrow 이죠?
There was a problem hiding this comment.
어 이거 성준님이 노션에 정리해준거 그대로 적용시킨건데.. 하핳
There was a problem hiding this comment.
내부적으로 Optional.get()을 호출하고 아니면 throw 하기 때문에 메서드 내부 동작도 잘 표현하지 않나라고 생각했습니다!
There was a problem hiding this comment.
아아 4번 방식 참고해서 이렇게 하신거군요
| roomJpaEntity.updateMemberCount(room.getMemberCount()); | ||
|
|
||
| roomJpaRepository.save(roomJpaEntity); | ||
| roomJpaRepository.save(roomJpaEntity.updateFrom(room)); |
| @Override | ||
| public Optional<RoomParticipant> findByUserIdAndRoomIdOptional(Long userId, Long roomId) { | ||
| return roomParticipantJpaRepository.findByUserIdAndRoomId(userId, roomId) | ||
| .map(roomParticipantMapper::toDomainEntity); | ||
| } |
현준님 이거 4번으로 간다면 모든 도메인의 Port에 대해서 도메인 조회 메서드를 수정해야할까요?? |
각자 맡은 도메인에 대해서 순차적으로 도입해도 될 것 같아요!! |
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
방 모집 마감 api의 흐름은 다음과 같습니다.
📸 스크린샷
💬 리뷰 요구사항
추가적으로 앞 pr에서 남겨주셨던 리뷰를 모두 반영했으니 같이 확인 부탁드릴게요!
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
버그 수정
테스트
기타