[DDING-109] Club 엔터티 formUrl 속성 제거 및 formId 속성 추가 #259
Conversation
Seooooo24
requested review from
wonjunYou,
5uhwann and
KoSeonJe
as code owners
Walkthrough이번 PR은 클럽 관련 도메인에서 모집 기간 및 폼 URL 정보를 제거하고, 대신 폼 엔티티를 활용하도록 DTO, 엔티티, 커맨드, 쿼리, 서비스 계층 전반에 걸쳐 수정된 사항입니다. 구체적으로, 기존의 모집 시작/종료 날짜 필드가 LocalDateTime에서 LocalDate로 변경되었으며, 관련 필드(formUrl)는 제거되고, 일부 응답 객체에서는 formId 필드가 추가되었습니다. 또한, 데이터베이스 스키마에서 해당 컬럼들을 삭제하였고, 폼 관리 로직을 위한 새로운 서비스 및 메서드가 도입되었습니다. 테스트 클래스는 주석 처리하여 비활성화되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Controller
participant FS as FacadeCentralClubServiceImpl
participant F as FormService
participant DB as Database
C->>FS: getMyClubInfo(userId)
FS->>DB: 클럽 정보 조회
FS->>F: getAllByClub(club)
F->>DB: 클럽에 대한 폼 조회
DB-->>F: 폼 리스트 반환
FS->>F: findActiveForm(forms)
alt 활성 폼 존재
F-->>FS: 활성 폼 반환
else 활성 폼 없음
FS->>F: getNewestForm(forms)
F-->>FS: 최신 폼 반환
end
FS->>C: MyClubInfoQuery(클럽, 폼) 반환
sequenceDiagram
participant C as Controller
participant FU as FacadeUserClubServiceImpl
participant F as FormService
participant DB as Database
C->>FU: findAllWithRecruitTimeCheckPoint(now)
FU->>DB: 클럽 리스트 조회
loop 각 클럽 처리
FU->>F: getAllByClub(club)
F->>DB: 클럽에 대한 폼 조회
DB-->>F: 폼 리스트 반환
FU->>F: findActiveForm(forms)
alt 활성 폼 존재
F-->>FU: 활성 폼 반환
else 활성 폼 없음
FU->>F: getNewestForm(forms)
F-->>FU: 최신 폼 반환
end
FU->>FU: UserClubQuery 객체 생성 (startDate, endDate, formId 포함)
end
FU->>C: UserClubListQuery 리스트 반환
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/command/UpdateClubInfoCommand.java (1)
23-23: formId에 대한 유효성 검증이 필요합니다.
formId가 필수 필드라면, null 체크나 유효성 검증 로직을 추가하는 것이 좋습니다.다음과 같이 검증 로직을 추가하는 것을 고려해보세요:
public Club toEntity() { + if (formId == null) { + throw new IllegalArgumentException("폼 ID는 필수 값입니다."); + } return Club.builder()Also applies to: 42-42
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/request/UpdateClubInfoRequest.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/MyClubInfoResponse.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/UserClubResponse.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/entity/Club.java(4 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/command/UpdateClubInfoCommand.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/query/MyClubInfoQuery.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/query/UserClubQuery.java(2 hunks)src/main/resources/db/migration/V39__alter_table_club_drop_add_colmn.sql(1 hunks)
🔇 Additional comments (5)
src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/query/UserClubQuery.java (1)
20-20: 변경사항이 적절합니다!
formUrl에서formId로의 변경이 일관되게 적용되었으며, 이는 딩동 플랫폼의 자체 폼 사용을 강제하는 PR의 목적에 부합합니다.Also applies to: 42-42
src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/query/MyClubInfoQuery.java (1)
20-20: 변경사항이 적절합니다!
UserClubQuery와 동일한 패턴으로formId필드가 일관되게 구현되었습니다.Also applies to: 43-43
src/main/resources/db/migration/V39__alter_table_club_drop_add_colmn.sql (1)
1-7: 데이터 마이그레이션 계획 확인 필요기존
form_url데이터를form_id로 변환하는 마이그레이션 계획이 필요합니다.다음 사항들을 확인해 주세요:
- 기존
form_url데이터를form_id로 매핑하는 방법- 매핑할 수 없는 데이터 처리 방안
- 데이터 손실 가능성 검토
src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/UserClubResponse.java (1)
40-41: 변경사항이 적절합니다!
formId필드의 스키마 문서화가 명확하고, 엔티티 변경사항과 일관성이 있습니다.src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/MyClubInfoResponse.java (1)
40-41: 변경사항이 적절합니다!
formId필드의 스키마 문서화가 명확하고,UserClubResponse와 일관성이 있습니다.
| @Schema(description = "모집 폼 아이디", example = "6") | ||
| Long formId, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
formId 필드에 @NotNull 제약조건이 필요합니다.
모집 폼이 필수라면, 다른 필수 필드들처럼 @NotNull 어노테이션과 적절한 메시지를 추가해야 합니다.
다음과 같이 수정하는 것을 추천드립니다:
@Schema(description = "모집 폼 아이디", example = "6")
+@NotNull(message = "모집 폼 아이디는 필수로 입력해야 합니다.")
Long formId,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Schema(description = "모집 폼 아이디", example = "6") | |
| Long formId, | |
| @Schema(description = "모집 폼 아이디", example = "6") | |
| @NotNull(message = "모집 폼 아이디는 필수로 입력해야 합니다.") | |
| Long formId, |
| @@ -70,7 +70,7 @@ public class Club extends BaseEntity { | |||
|
|
|||
| private String ideal; | |||
|
|
|||
| private String formUrl; | |||
| private Long formId; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
formId 유효성 검증 추가 필요
formId가 유효한 Form을 참조하는지 확인하는 로직이 필요합니다.
다음과 같은 검증 메서드 추가를 제안합니다:
private void validateFormId(Long formId) {
if (formId == null) {
throw new IllegalArgumentException("Form ID는 필수 값입니다.");
}
// TODO: Form 존재 여부 확인 로직 추가
}클라이언트에서 직접 formId 업데이트를 요청할 일이 없을 것 같아 삭제했습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/ddingdong/ddingdongBE/domain/club/entity/Club.java (1)
123-126: 메서드 이름과 실제 동작이 일치하지 않음
addClubMember메서드 내부에서 호출하는setClubFormConvenience는 메서드 이름과 일치하지 않는 동작을 수행하는 것으로 보입니다.메서드 이름을 실제 동작과 일치하도록 변경하거나, 메서드의 목적을 명확히 하는 주석을 추가해 주세요.
- clubMember.setClubFormConvenience(this); + clubMember.setClub(this); // 클럽 멤버와 클럽 간의 양방향 관계 설정
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ddingdong/ddingdongBE/domain/club/entity/Club.java(1 hunks)src/main/resources/db/migration/V39__alter_table_club_drop_colmn.sql(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/db/migration/V39__alter_table_club_drop_colmn.sql
🔇 Additional comments (1)
src/main/java/ddingdong/ddingdongBE/domain/club/entity/Club.java (1)
34-78:⚠️ Potential issueformId 필드 추가 필요
PR 목적에 따라
formUrl을 제거하고formId를 추가하는 것이 필요하지만,formId필드가 구현되지 않았습니다.다음과 같이
formId필드를 추가해 주세요:@Column(name = "deleted_at", columnDefinition = "TIMESTAMP") private LocalDateTime deletedAt; + private Long formId;이전 리뷰에서 제안된 대로
formId유효성 검증도 함께 추가해 주세요.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
src/main/java/ddingdong/ddingdongBE/domain/club/entity/Club.java (2)
75-96:⚠️ Potential issueForm 관련 필드 추가 필요
PR 목적에 따라
formUrl필드가 제거되었지만,formId필드가 추가되지 않았습니다. Form과의 연관관계를 위해 다음과 같은 변경이 필요합니다:@Entity public class Club extends BaseEntity { + @Column(name = "form_id") + private Long formId; @Builder private Club(Long id, User user, List<ClubMember> clubMembers, String name, String category, String tag, String leader, PhoneNumber phoneNumber, Location location, String regularMeeting, String introduction, String activity, - String ideal, Score score, LocalDateTime deletedAt) { + String ideal, Score score, LocalDateTime deletedAt, Long formId) { // ... existing assignments ... + this.formId = formId; } + private void validateFormId(Long formId) { + if (formId == null) { + throw new IllegalArgumentException("Form ID는 필수 값입니다."); + } + } }
98-109: 🛠️ Refactor suggestionupdateClubInfo 메서드에 formId 업데이트 로직 추가 필요
클럽 정보 업데이트 시
formId필드도 함께 업데이트되어야 합니다.public void updateClubInfo(Club club) { this.name = club.getName(); this.category = club.getCategory(); this.tag = club.getTag(); this.leader = club.getLeader(); this.phoneNumber = club.getPhoneNumber(); this.location = club.getLocation(); this.regularMeeting = club.getRegularMeeting(); this.introduction = club.getIntroduction(); this.activity = club.getActivity(); this.ideal = club.getIdeal(); + this.formId = club.getFormId(); }
🧹 Nitpick comments (3)
src/main/java/ddingdong/ddingdongBE/domain/club/service/FacadeUserClubServiceImpl.java (2)
36-41: findActiveForm 반복 호출 최소화 제안
현재 같은 메서드를 두 번 호출하고 있어 불필요한 호출을 줄이면 성능 및 가독성이 개선될 수 있습니다.다음과 같이 리팩터링할 수 있습니다:
-Form form = formService.findActiveForm(forms) != null - ? formService.findActiveForm(forms) - : formService.getNewestForm(forms); +Form activeForm = formService.findActiveForm(forms); +Form form = activeForm != null + ? activeForm + : formService.getNewestForm(forms);
49-52: findActiveForm 반복 호출 최소화 제안 (getClub 메서드)
동일한 로직에서 같은 메서드가 중복 호출됩니다. 변수에 할당하여 코드 중복을 줄이세요.-Form form = formService.findActiveForm(forms) != null - ? formService.findActiveForm(forms) - : formService.getNewestForm(forms); +Form activeForm = formService.findActiveForm(forms); +Form form = (activeForm != null) + ? activeForm + : formService.getNewestForm(forms);src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/MyClubInfoResponse.java (1)
25-28: Schema 예시 값 업데이트 필요
@Schema어노테이션의 example 값이 LocalDateTime 형식("2024-01-01 00:00")으로 되어있지만, 필드 타입이 LocalDate로 변경되었습니다. 날짜 형식을 맞춰주세요.-@Schema(description = "모집시작날짜", example = "2024-01-01 00:00") +@Schema(description = "모집시작날짜", example = "2024-01-01") -@Schema(description = "모집마감날짜", example = "2024-01-01 00:00") +@Schema(description = "모집마감날짜", example = "2024-01-01")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/ddingdong/ddingdongBE/domain/club/controller/UserClubController.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/request/UpdateClubInfoRequest.java(0 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/MyClubInfoResponse.java(3 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/UserClubResponse.java(4 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/entity/Club.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/FacadeCentralClubServiceImpl.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/FacadeUserClubService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/FacadeUserClubServiceImpl.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/command/UpdateClubInfoCommand.java(0 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/query/MyClubInfoQuery.java(3 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/query/UserClubQuery.java(3 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java(2 hunks)src/main/resources/db/migration/V40__alter_table_club_drop_colmn.sql(1 hunks)src/test/java/ddingdong/ddingdongBE/domain/club/service/FacadeUserClubServiceImplTest.java(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/command/UpdateClubInfoCommand.java
- src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/request/UpdateClubInfoRequest.java
✅ Files skipped from review due to trivial changes (2)
- src/test/java/ddingdong/ddingdongBE/domain/club/service/FacadeUserClubServiceImplTest.java
- src/main/resources/db/migration/V40__alter_table_club_drop_colmn.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/query/UserClubQuery.java
🔇 Additional comments (13)
src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java (1)
6-6: LGTM!FormStatus enum을 사용하여 폼의 상태를 확인하는 것은 적절한 접근 방식입니다.
src/main/java/ddingdong/ddingdongBE/domain/club/service/FacadeUserClubServiceImpl.java (5)
13-14: Form 및 LocalDate 관련 의존성 추가가 적절합니다.
Form 도메인 기능과 날짜 처리를 위해 필요한 import문으로 보입니다. 별다른 문제는 없어 보입니다.Also applies to: 17-17
31-31: FormService 의존성 주입 추가 확인
Form 연동 로직을 처리하기 위한 의존성 주입으로 보입니다. 정상적으로 설계되었습니다.
34-34: 메서드 파라미터가 LocalDate로 변경되었습니다.
시간 단위가 필요 없는지 확인이 필요합니다. 과거에 시·분·초가 필요한 로직이라면 영향도를 점검하세요.
55-55: form 인자 추가
UserClubQuery 생성 시 form을 넘기는 로직입니다. form이 null일 가능성도 고려하여, null-safe 처리가 필요할 수 있습니다.
61-67: checkRecruit 메서드의 form null 처리 확인
form이 null이면 BEFORE_RECRUIT를 반환하는 로직이 의도대로 동작하는지, 그리고 endDate가 null일 가능성은 없는지 확인이 필요합니다.src/main/java/ddingdong/ddingdongBE/domain/club/service/FacadeUserClubService.java (2)
5-5: LocalDate import 추가
날짜만 관리하기 위해 LocalDate를 사용하는 것으로 보입니다. 변경 방향에 맞게 잘 적용되었습니다.
10-10: LocalDate로 파라미터 수정 확인
과거에 시간 단위가 필요했다면, 문제없이 동작하는지 점검해 주세요.src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java (1)
19-22: 새로운 메서드 추가에 따른 예외 상황 검토
forms가 비어 있거나 여러 활성 폼이 있을 경우 어떻게 처리되는지 명확히 정의해야 합니다. null을 반환하는지, 예외를 던지는지 확인 부탁드립니다.src/main/java/ddingdong/ddingdongBE/domain/club/controller/UserClubController.java (1)
8-8: LGTM!모집 기간 체크를 위해 LocalDateTime에서 LocalDate로 변경한 것이 적절합니다. 모집 기간은 일자 단위로만 관리되므로 시간 정보는 불필요했습니다.
Also applies to: 22-22
src/main/java/ddingdong/ddingdongBE/domain/club/service/dto/query/MyClubInfoQuery.java (1)
38-39:❓ Verification inconclusive
Form 객체가 null일 때의 처리 방식을 검토해주세요.
현재 구현은 Form이 null일 경우 날짜도 null을 반환하도록 되어있습니다. 이로 인해 발생할 수 있는 문제들을 고려해야 합니다:
- 프론트엔드에서 null 날짜 처리가 되어있나요?
- 기본값을 설정하는 것이 더 나은 방식일 수 있습니다.
🏁 Script executed:
#!/bin/bash # Form이 null일 때 클라이언트 측에서 어떻게 처리하는지 확인 rg -A 5 "startDate.*null|endDate.*null" --type ts --type jsLength of output: 210
핵심: form null 처리 방식에 대한 프론트엔드 검증이 필요함
현재 MyClubInfoQuery.java에서는 form이 null일 경우
startDate와endDate를 null로 반환합니다. 다만, ripgrep 명령어 결과가 예상과 달리 검색된 파일이 없어 클라이언트 측의 null 날짜 처리 방식에 대한 확인이 제대로 이루어지지 않았습니다.
- 프론트엔드(클라이언트) 코드에서 null 날짜를 어떻게 처리하고 있는지 수동으로 또는 추가 스크립트를 통해 검증해 주세요.
- 만약 클라이언트에서 null 값 처리가 미흡하다면, 기본값 설정 등 대체 로직 도입을 고려해 보시길 바랍니다.
src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/UserClubResponse.java (1)
25-28:❓ Verification inconclusive
Schema 예시 값 검토가 필요합니다.
날짜 형식과 formId의 예시 값이 실제 사용되는 형식과 일치하는지 확인이 필요합니다:
- 날짜 형식이 "2024-01-01"로 되어있는데, 이는 프론트엔드에서 기대하는 형식과 일치하나요?
- formId 예시 값이 "6"으로 되어있는데, 이는 실제 사용되는 ID 범위를 잘 반영하나요?
Also applies to: 37-38
🌐 Web query:
Java LocalDate를 프론트엔드에서 처리할 때 권장되는 날짜 형식은 무엇인가요?💡 Result:
Java의
LocalDate를 프론트엔드에서 처리할 때 ISO 8601 표준 형식인 "yyyy-MM-dd"를 사용하는 것이 권장됩니다. 이 형식은 다음과 같은 장점을 가지고 있습니다:
표준화된 포맷
DateTimeFormatter.ISO_LOCAL_DATE로 기본 지원되며 별도의 파싱 로직 없이 처리 가능[2][9]- 예:
2024-03-14형태로 직관적인 표현프론트엔드 호환성
- JSON 직렬화/역직렬화 시 대부분의 라이브러리에서 기본 지원[3]
- JavaScript의
Date객체와 호환되어 추가 변환 불필요[3][6]백엔드-프론트엔드 협업 효율성
// Java 측 직렬화 예시 DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); String formattedDate = LocalDate.now().format(formatter);
- 프론트엔드에서 날짜 선택기(date picker) 컴포넌트와 즉시 호환[3][6]
다른 형식 필요 시 변환 용이성
// JavaScript 측 포맷 변경 예시 const date = new Date('2024-03-14'); console.log(date.toLocaleDateString('ko-KR')); // 2024. 3. 14.
- 클라이언트 측에서 locale 기반 포맷팅 가능[9][10]
오류 최소화
- 날짜 필드 누락 방지(년-월-일 전체 포함)[10]
LocalDate.parse()메서드로 간편한 유효성 검증 가능[2][4]특히 REST API 응답 시 시간 정보가 필요 없는 경우
LocalDateTime대신LocalDate사용을 권장하며[3][5], 필요시 클라이언트 측에서 추가 가공하는 것이 효율적입니다.Citations:
- 1: https://velog.io/@jjungyu12/Java-LocalDate-LocalTime-LocalDateTime-%ED%81%B4%EB%9E%98%EC%8A%A4
- 2: https://gyunny.tistory.com/25
- 3: https://velog.io/@myday0827/JAVA-LocalDate-LocalDateTime-%EB%82%A0%EC%A7%9C-%ED%8F%AC%EB%A7%B7%ED%8C%85
- 4: https://kujadev.tistory.com/60
- 5: https://yeo-computerclass.tistory.com/566
- 6: https://fvor001.tistory.com/149
- 7: https://lasbe.tistory.com/136
- 8: https://velog.io/@kkatalchae/Java8-Time-API-LocalDate-%EC%A0%95%EB%A6%AC
- 9: https://yanacoding.tistory.com/254
- 10: https://jojoldu.tistory.com/527
Schema 예시 값 검토 필요 – ISO 8601 형식 및 formId 값 확인
- 현재 날짜 예시
"2024-01-01"는 ISO 8601 표준 형식인"yyyy-MM-dd"를 따르므로, 프론트엔드에서 기대하는 형식과 일치하는지 다시 한 번 확인해 주세요.- 또한, 추가된
formId의 예시 값"6"이 실제 사용되는 ID 범위와 일치하는지 검토가 필요합니다.- 37-38 라인의 동일한 어노테이션 부분에도 같은 검토가 적용됩니다.
src/main/java/ddingdong/ddingdongBE/domain/club/controller/dto/response/MyClubInfoResponse.java (1)
50-51: LGTM! 날짜 타입 변경이 잘 반영되었습니다.
from메서드에서startDate와endDate필드에 대한 매핑이 올바르게 구현되었습니다.
| @Override | ||
| public Form findActiveForm(List<Form> forms) { | ||
| return forms.stream() | ||
| .filter(f -> FormStatus.getDescription(LocalDate.now(), f.getStartDate(), f.getEndDate()) == FormStatus.ONGOING) | ||
| .findFirst() | ||
| .orElse(null); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메서드의 안정성 개선이 필요합니다.
현재 구현에는 다음과 같은 잠재적인 문제가 있습니다:
- null을 반환하는 것은 NullPointerException의 위험이 있습니다.
- 여러 개의 활성 폼이 있을 경우 첫 번째 폼만 반환됩니다.
- 빈 리스트에 대한 명시적인 처리가 없습니다.
다음과 같이 개선하는 것을 제안드립니다:
@Override
public Form findActiveForm(List<Form> forms) {
+ if (forms == null || forms.isEmpty()) {
+ return Optional.empty();
+ }
return forms.stream()
.filter(f -> FormStatus.getDescription(LocalDate.now(), f.getStartDate(), f.getEndDate()) == FormStatus.ONGOING)
- .findFirst()
- .orElse(null);
+ .findFirst();
}또한 JavaDoc을 추가하여 메서드의 동작을 명확히 문서화하는 것을 추천드립니다:
/**
* 주어진 폼 목록에서 현재 활성화된 폼을 찾습니다.
* 여러 개의 활성 폼이 있을 경우 첫 번째 폼을 반환합니다.
*
* @param forms 검사할 폼 목록
* @return 활성화된 폼을 담은 Optional
*/| @Override | ||
| public Form getNewestForm(List<Form> forms) { | ||
| return forms.stream() | ||
| .max(Comparator.comparing(Form::getId)) | ||
| .orElse(null); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
최신 폼 조회 로직 개선이 필요합니다.
현재 구현에서 개선이 필요한 부분들입니다:
- null 반환은 안전하지 않습니다.
- ID를 기준으로 최신 여부를 판단하는 것은 생성 시간을 기준으로 하는 것보다 덜 정확할 수 있습니다.
다음과 같은 개선을 제안드립니다:
@Override
public Form getNewestForm(List<Form> forms) {
+ if (forms == null || forms.isEmpty()) {
+ return Optional.empty();
+ }
return forms.stream()
- .max(Comparator.comparing(Form::getId))
+ .max(Comparator.comparing(Form::getCreatedAt))
- .orElse(null);
+ .findFirst();
}또한 JavaDoc 추가를 추천드립니다:
/**
* 주어진 폼 목록에서 가장 최근에 생성된 폼을 찾습니다.
*
* @param forms 검사할 폼 목록
* @return 최신 폼을 담은 Optional
*/| Form form = formService.findActiveForm(forms) != null | ||
| ? formService.findActiveForm(forms) | ||
| : formService.getNewestForm(forms); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Form 조회 로직 개선이 필요합니다.
현재 구현에서 몇 가지 개선이 필요한 부분이 있습니다:
- 중복 호출 제거:
-Form form = formService.findActiveForm(forms) != null
- ? formService.findActiveForm(forms)
- : formService.getNewestForm(forms);
+Form activeForm = formService.findActiveForm(forms);
+Form form = activeForm != null ? activeForm : formService.getNewestForm(forms);- forms가 비어있을 때의 처리가 누락되어 있습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Form form = formService.findActiveForm(forms) != null | |
| ? formService.findActiveForm(forms) | |
| : formService.getNewestForm(forms); | |
| Form activeForm = formService.findActiveForm(forms); | |
| Form form = activeForm != null ? activeForm : formService.getNewestForm(forms); |
넵 모든 경우 테스트 해봤습니다! |
🚀 작업 내용
지원하기 폼지 연결을 띵동 자체 폼지로 강제하기 위하여
Club 엔터티 formUrl 속성 제거 및 formId 속성 추가하였습니다.
🤔 고민했던 내용
formUrl을 완전히 삭제해도 되는지 궁금합니다.
💬 리뷰 중점사항
Query 내에서 삼항연산자 사용하여 null을 처리했습니다.
formService에 getNewestForm(), findActiveForm() 메서드를 정의했습니다.
마지막 커밋 이전 커밋들은 혼자 삽질한 게 많아서... 마지막 커밋 서비스들 위주로 리뷰해주시면 될 것 같습니다.
Summary by CodeRabbit