feat: 스터디 V2 팩토리 구현 #855
feat: 스터디 V2 팩토리 구현 #855
Conversation
Walkthrough이 풀 리퀘스트는 스터디 V2 도메인에 대한 새로운 기능을 추가합니다. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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
CodeRabbit Configuration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/RandomAttendanceNumberGenerator.java (1)
14-14: 성능 최적화를 위한 제안
SecureRandom.getInstanceStrong()은 암호화 작업에 적합하지만, 단순 출석 번호 생성에는 과도할 수 있습니다. 또한 매번 새로운 인스턴스를 생성하는 것은 비효율적입니다.다음과 같이 수정하는 것을 제안합니다:
+ private static final Random RANDOM = new Random(); + public int generate() { - return SecureRandom.getInstanceStrong().ints(1000, 10000).findFirst().orElseThrow(); + return RANDOM.nextInt(1000, 10000); }src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyFactory.java (1)
49-51: 스트림 연산 개선이 필요합니다.스트림 연산이 부작용(side-effect)을 발생시키고 있으며,
totalRound에 대한 유효성 검사가 없습니다.다음과 같이 수정하는 것을 제안합니다:
+ if (totalRound <= 0) { + throw new IllegalArgumentException("총 회차 수는 0보다 커야 합니다."); + } + - IntStream.rangeClosed(1, totalRound) - .forEach(round -> StudySessionV2.createEmpty(round, attendanceNumberGenerator.generate(), study)); + List<StudySessionV2> sessions = IntStream.rangeClosed(1, totalRound) + .mapToObj(round -> StudySessionV2.createEmpty( + round, + attendanceNumberGenerator.generate(), + study + )) + .collect(Collectors.toList());src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudySessionV2.java (1)
33-34: position 필드에 대한 제약조건 추가가 필요합니다.position 필드는 1 이상의 값만 가질 수 있어야 합니다.
다음과 같이 제약조건을 추가하는 것을 제안합니다:
@Comment("회차 순서") + @Column(nullable = false) + @Min(value = 1, message = "회차 순서는 1 이상이어야 합니다.") private Integer position;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AttendanceNumberGenerator.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/RandomAttendanceNumberGenerator.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyFactory.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudySessionV2.java(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AttendanceNumberGenerator.java
| @SneakyThrows | ||
| public int generate() { | ||
| return SecureRandom.getInstanceStrong().ints(1000, 10000).findFirst().orElseThrow(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 처리 방식 개선이 필요합니다.
@SneakyThrows를 사용하면 중요한 예외가 숨겨질 수 있습니다. 명시적인 예외 처리를 고려해 주세요.
다음과 같이 수정하는 것을 제안합니다:
- @SneakyThrows
- public int generate() {
- return SecureRandom.getInstanceStrong().ints(1000, 10000).findFirst().orElseThrow();
+ public int generate() {
+ try {
+ return SecureRandom.getInstanceStrong().ints(1000, 10000).findFirst().orElseThrow();
+ } catch (Exception e) {
+ throw new IllegalStateException("출석 번호 생성 중 오류가 발생했습니다.", e);
+ }
+ }📝 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.
| @SneakyThrows | |
| public int generate() { | |
| return SecureRandom.getInstanceStrong().ints(1000, 10000).findFirst().orElseThrow(); | |
| } | |
| public int generate() { | |
| try { | |
| return SecureRandom.getInstanceStrong().ints(1000, 10000).findFirst().orElseThrow(); | |
| } catch (Exception e) { | |
| throw new IllegalStateException("출석 번호 생성 중 오류가 발생했습니다.", e); | |
| } | |
| } |
| public StudyV2 create( | ||
| StudyType type, | ||
| String title, | ||
| String description, | ||
| String descriptionNotionLink, | ||
| Semester semester, | ||
| Integer totalRound, | ||
| DayOfWeek dayOfWeek, | ||
| LocalTime startTime, | ||
| LocalTime endTime, | ||
| Period applicationPeriod, | ||
| String discordChannelId, | ||
| String discordRoleId, | ||
| Member mentor, | ||
| AttendanceNumberGenerator attendanceNumberGenerator) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
매개변수가 너무 많습니다.
현재 create 메서드는 14개의 매개변수를 가지고 있어 가독성과 유지보수성이 떨어집니다.
다음과 같이 DTO를 사용하는 것을 제안합니다:
@Getter
@Builder
public class StudyCreateCommand {
private final StudyType type;
private final String title;
private final String description;
private final String descriptionNotionLink;
private final Semester semester;
private final Integer totalRound;
private final DayOfWeek dayOfWeek;
private final LocalTime startTime;
private final LocalTime endTime;
private final Period applicationPeriod;
private final String discordChannelId;
private final String discordRoleId;
private final Member mentor;
}그리고 메서드 시그니처를 다음과 같이 변경하세요:
public StudyV2 create(StudyCreateCommand command, AttendanceNumberGenerator attendanceNumberGenerator)| public static void createEmpty(Integer position, Integer lessonAttendanceNumber, StudyV2 studyV2) { | ||
| StudySessionV2.builder() | ||
| .title(title) | ||
| .description(description) | ||
| .position(position) | ||
| .lessonAttendanceNumber(lessonAttendanceNumber) | ||
| .lessonPeriod(lessonPeriod) | ||
| .assignmentDescriptionLink(assignmentDescriptionLink) | ||
| .assignmentPeriod(assignmentPeriod) | ||
| .studyV2(studyV2) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
팩토리 메서드의 반환 값과 유효성 검사가 필요합니다.
createEmpty 메서드가 void를 반환하고 있으며, 입력값에 대한 유효성 검사가 없습니다.
다음과 같이 수정하는 것을 제안합니다:
- public static void createEmpty(Integer position, Integer lessonAttendanceNumber, StudyV2 studyV2) {
- StudySessionV2.builder()
- .position(position)
- .lessonAttendanceNumber(lessonAttendanceNumber)
- .studyV2(studyV2)
- .build();
+ public static StudySessionV2 createEmpty(Integer position, Integer lessonAttendanceNumber, StudyV2 studyV2) {
+ if (position == null || position <= 0) {
+ throw new IllegalArgumentException("회차 순서는 0보다 커야 합니다.");
+ }
+ if (lessonAttendanceNumber == null) {
+ throw new IllegalArgumentException("출석 번호는 null일 수 없습니다.");
+ }
+ if (studyV2 == null) {
+ throw new IllegalArgumentException("스터디 정보는 null일 수 없습니다.");
+ }
+
+ return StudySessionV2.builder()
+ .position(position)
+ .lessonAttendanceNumber(lessonAttendanceNumber)
+ .studyV2(studyV2)
+ .build();
+ }📝 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.
| public static void createEmpty(Integer position, Integer lessonAttendanceNumber, StudyV2 studyV2) { | |
| StudySessionV2.builder() | |
| .title(title) | |
| .description(description) | |
| .position(position) | |
| .lessonAttendanceNumber(lessonAttendanceNumber) | |
| .lessonPeriod(lessonPeriod) | |
| .assignmentDescriptionLink(assignmentDescriptionLink) | |
| .assignmentPeriod(assignmentPeriod) | |
| .studyV2(studyV2) | |
| .build(); | |
| } | |
| public static StudySessionV2 createEmpty(Integer position, Integer lessonAttendanceNumber, StudyV2 studyV2) { | |
| if (position == null || position <= 0) { | |
| throw new IllegalArgumentException("회차 순서는 0보다 커야 합니다."); | |
| } | |
| if (lessonAttendanceNumber == null) { | |
| throw new IllegalArgumentException("출석 번호는 null일 수 없습니다."); | |
| } | |
| if (studyV2 == null) { | |
| throw new IllegalArgumentException("스터디 정보는 null일 수 없습니다."); | |
| } | |
| return StudySessionV2.builder() | |
| .position(position) | |
| .lessonAttendanceNumber(lessonAttendanceNumber) | |
| .studyV2(studyV2) | |
| .build(); | |
| } |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/RandomAttendanceNumberGenerator.java (2)
6-9: Javadoc 문서를 더 상세하게 보완해 주세요.현재 문서는 기본적인 설명만 포함하고 있습니다. 다음 정보들을 추가하면 좋을 것 같습니다:
- 생성되는 번호의 범위 (1000-9999)
- SecureRandom을 사용한다는 점
- 예외 발생 가능성
예시:
/** * 네 자리의 랜덤한 출석번호를 생성합니다. * * @return 1000에서 9999 사이의 랜덤한 정수 * @throws IllegalStateException SecureRandom 인스턴스 생성 실패 시 */
11-12: 상수 정의를 개선해 주세요.다음 사항들을 고려해 주세요:
- 상수들이 public으로 선언되어 있는데, private으로 충분할 것 같습니다.
- 각 상수의 의미를 설명하는 Javadoc 주석이 있으면 좋겠습니다.
- public static final int MIN_ORIGIN = 1000; - public static final int MAX_BOUND = 10000; + /** 출석번호의 최소값 (1000) */ + private static final int MIN_ORIGIN = 1000; + /** 출석번호의 최대값 (제외) */ + private static final int MAX_BOUND = 10000;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/RandomAttendanceNumberGenerator.java(1 hunks)
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/RandomAttendanceNumberGenerator.java (2)
15-15: 예외 처리 방식 개선이 필요합니다.
@SneakyThrows를 사용하면 중요한 예외가 숨겨질 수 있습니다. 명시적인 예외 처리를 고려해 주세요.
16-21: 🛠️ Refactor suggestiongenerate() 메서드의 구현을 더 견고하게 만들어 주세요.
현재 구현은 간결하지만 다음과 같은 개선사항들을 고려해 보시면 좋겠습니다:
- SecureRandom 인스턴스를 재사용하여 성능을 개선
- findFirst().orElseThrow() 대신 구체적인 예외 메시지 제공
- 스레드 안전성 보장
+ private static final SecureRandom secureRandom; + + static { + try { + secureRandom = SecureRandom.getInstanceStrong(); + } catch (Exception e) { + throw new IllegalStateException("보안 난수 생성기 초기화 실패", e); + } + } + @Override - @SneakyThrows public int generate() { - return SecureRandom.getInstanceStrong() - .ints(MIN_ORIGIN, MAX_BOUND) - .findFirst() - .orElseThrow(); + synchronized (secureRandom) { + return secureRandom + .ints(MIN_ORIGIN, MAX_BOUND) + .findFirst() + .orElseThrow(() -> + new IllegalStateException("출석번호 생성 실패")); + } }Likely invalid or redundant comment.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/main/java/com/gdschongik/gdsc/domain/common/vo/Semester.java (1)
24-28: 생성자 구현이 안전하게 되어있습니다.private 접근 제어자와 Builder 패턴을 통해 객체 생성을 제어하고 있습니다. 다만, 입력값 검증이 추가되면 좋을 것 같습니다.
다음과 같이 입력값 검증을 추가하는 것을 고려해보세요:
@Builder(access = AccessLevel.PRIVATE) private Semester(int academicYear, SemesterType semesterType) { + if (academicYear < 0) { + throw new IllegalArgumentException("학년도는 0보다 작을 수 없습니다."); + } + if (semesterType == null) { + throw new IllegalArgumentException("학기 타입은 null일 수 없습니다."); + } this.academicYear = academicYear; this.semesterType = semesterType; }src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java (2)
25-26: 날짜 상수의 유연성 개선이 필요합니다.하드코딩된 날짜는 테스트의 유지보수를 어렵게 만들 수 있습니다.
현재 시간을 기준으로 상대적인 날짜를 생성하는 방식을 고려해보세요:
- public static final Period STUDY_APPLICATION_PERIOD = - Period.of(LocalDateTime.of(2025, 3, 1, 0, 0), LocalDateTime.of(2025, 3, 14, 23, 59)); + public static final Period STUDY_APPLICATION_PERIOD = Period.of( + LocalDateTime.now().withDayOfMonth(1).withHour(0).withMinute(0), + LocalDateTime.now().withDayOfMonth(14).withHour(23).withMinute(59));
15-19: 상수들의 논리적 그룹화가 필요합니다.연관된 상수들이 분산되어 있어 코드의 가독성이 떨어집니다.
상수들을 논리적 그룹으로 구성하는 것이 좋습니다. 내부 static 클래스를 사용하여 그룹화하는 것을 제안드립니다:
public class StudyConstant { private StudyConstant() {} public static class BasicInfo { public static final String STUDY_TITLE = "스터디 제목"; public static final String STUDY_DESCRIPTION = "스터디 설명"; public static final String STUDY_DESCRIPTION_NOTION_LINK = "https://gdschongik.com/2025-1-backend-study"; public static final Semester STUDY_SEMESTER = Semester.of(2025, SemesterType.FIRST); public static final int TOTAL_ROUND = 8; } // ... 다른 그룹들 }src/test/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyFactoryTest.java (2)
16-21: 테스트 더블 클래스의 위치 개선이 필요합니다.
FixedAttendanceNumberGenerator는 여러 테스트에서 재사용될 수 있는 테스트 더블입니다.별도의 테스트 지원 클래스로 분리하여 재사용성을 높이는 것을 추천드립니다:
// src/test/java/com/gdschongik/gdsc/domain/studyv2/support/TestAttendanceNumberGenerator.java public class TestAttendanceNumberGenerator implements AttendanceNumberGenerator { private final int fixedNumber; public TestAttendanceNumberGenerator(int fixedNumber) { this.fixedNumber = fixedNumber; } @Override public int generate() { return fixedNumber; } }
31-45: 테스트 데이터 설정의 중복을 제거해야 합니다.
studyFactory.create()호출 시 동일한 파라미터 설정이 반복됩니다.테스트 데이터 생성을 도와주는 빌더나 헬퍼 메서드를 추가하는 것을 추천드립니다:
private StudyV2 createDefaultStudy(Member mentor, AttendanceNumberGenerator generator, int totalRound) { return studyFactory.create( StudyType.OFFLINE, STUDY_TITLE, STUDY_DESCRIPTION, STUDY_DESCRIPTION_NOTION_LINK, STUDY_SEMESTER, totalRound, DAY_OF_WEEK, STUDY_START_TIME, STUDY_END_TIME, STUDY_APPLICATION_PERIOD, STUDY_DISCORD_CHANNEL_ID, STUDY_DISCORD_ROLE_ID, mentor, generator); }Also applies to: 59-73, 88-102
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/gdschongik/gdsc/domain/common/vo/Semester.java(2 hunks)src/test/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyFactoryTest.java(1 hunks)src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java(2 hunks)
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/common/vo/Semester.java (2)
Line range hint
13-21: 클래스 설계가 잘 되어있습니다!값 객체(VO)로서 불변성을 보장하고, JPA와의 통합을 위한 적절한 어노테이션이 사용되었습니다.
30-35: 정적 팩토리 메서드가 잘 구현되어 있습니다.
of메서드를 통해 객체 생성 의도가 명확하게 드러나며, 빌더 패턴을 내부적으로 사용하여 구현 세부사항을 잘 캡슐화하고 있습니다.
| public static final String STUDY_DISCORD_CHANNEL_ID = "12345678"; | ||
| public static final String STUDY_DISCORD_ROLE_ID = "12345678"; |
There was a problem hiding this comment.
Discord ID와 같은 민감한 정보는 환경 변수로 관리하는 것이 좋습니다.
Discord 채널 ID와 역할 ID를 상수로 하드코딩하는 것은 보안상 위험할 수 있습니다.
테스트용 값이라도 실제 ID 형식을 따르는 것이 좋으며, 프로덕션 환경에서는 환경 변수로 관리하는 것을 권장드립니다.
| @Test | ||
| void 스터디_생성시_설정한_총_회차만큼_스터디회차가_생성된다() { | ||
| // given | ||
| Member mentor = fixtureHelper.createMentor(1L); | ||
| AttendanceNumberGenerator generator = new FixedAttendanceNumberGenerator(); | ||
| int totalRound = 8; | ||
|
|
||
| // when | ||
| StudyV2 study = studyFactory.create( | ||
| StudyType.OFFLINE, | ||
| STUDY_TITLE, | ||
| STUDY_DESCRIPTION, | ||
| STUDY_DESCRIPTION_NOTION_LINK, | ||
| STUDY_SEMESTER, | ||
| totalRound, | ||
| DAY_OF_WEEK, | ||
| STUDY_START_TIME, | ||
| STUDY_END_TIME, | ||
| STUDY_APPLICATION_PERIOD, | ||
| STUDY_DISCORD_CHANNEL_ID, | ||
| STUDY_DISCORD_ROLE_ID, | ||
| mentor, | ||
| generator); | ||
|
|
||
| // then | ||
| assertThat(study.getStudySessions()).hasSize(8); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 케이스 보완이 필요합니다.
현재 테스트는 기본적인 케이스만 다루고 있습니다. 경계값이나 예외 상황에 대한 테스트가 누락되어 있습니다.
다음과 같은 테스트 케이스 추가를 제안드립니다:
@Test
void 총_회차가_0이하일_경우_예외가_발생한다() {
// given
Member mentor = fixtureHelper.createMentor(1L);
AttendanceNumberGenerator generator = new TestAttendanceNumberGenerator(1000);
int invalidTotalRound = 0;
// when & then
assertThatThrownBy(() -> studyFactory.create(
StudyType.OFFLINE,
STUDY_TITLE,
STUDY_DESCRIPTION,
STUDY_DESCRIPTION_NOTION_LINK,
STUDY_SEMESTER,
invalidTotalRound,
DAY_OF_WEEK,
STUDY_START_TIME,
STUDY_END_TIME,
STUDY_APPLICATION_PERIOD,
STUDY_DISCORD_CHANNEL_ID,
STUDY_DISCORD_ROLE_ID,
mentor,
generator))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("총 회차는 1 이상이어야 합니다");
}Also applies to: 51-79, 81-108
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
Summary by CodeRabbit
새로운 기능
개선 사항
이러한 변경사항은 스터디 관리 및 출석 추적 프로세스를 더욱 효율적으로 만들어줍니다.