Skip to content
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

refactor: 스터디 엔티티 수정 #466

Merged
merged 16 commits into from
Oct 15, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public class Round extends BaseEntity {
@JoinColumn(name = "round_id", nullable = false)
private List<RoundOfMember> roundOfMembers = new ArrayList<>();

private Integer weekNumber;

protected Round() {
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.yigongil.backend.domain.study;

import javax.persistence.EmbeddedId;
import javax.persistence.Entity;
import javax.persistence.JoinColumn;
import javax.persistence.ManyToOne;
import javax.persistence.MapsId;

@Entity
public class ProgressDayOfWeek {

@EmbeddedId
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3: 이렇게 복합키를 사용할 수 있는 줄은 처음 알았네요 👍
엔티티의 id로 쓰인다는 점에서 클래스명을 ProgressDayOfWeekId로 바꾸는 게 더 직관적일 거 같은데 어떻게 생각하시나요??

private ProgressDayOfWeekKey id;

@ManyToOne
@MapsId("studyId")
@JoinColumn(name = "study_id")
private Study study;

}


Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.yigongil.backend.domain.study;

import java.io.Serializable;
import javax.persistence.Column;
import javax.persistence.Embeddable;
import lombok.Getter;

@Getter
@Embeddable
public class ProgressDayOfWeekKey implements Serializable {

@Column(name = "study_id")
private Long studyId;

@Column(name = "progress_day_of_week")
private int progressDayOfWeek;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3: 이게 스터디를 주 몇 일 하는지 나타내는 값인가요?? 왜 복합키로 사용한 건지 궁금합니다!


}
129 changes: 37 additions & 92 deletions backend/src/main/java/com/yigongil/backend/domain/study/Study.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@
import com.yigongil.backend.domain.member.Member;
import com.yigongil.backend.domain.round.Round;
import com.yigongil.backend.domain.roundofmember.RoundOfMember;
import com.yigongil.backend.exception.CannotStartException;
import com.yigongil.backend.exception.InvalidMemberSizeException;
import com.yigongil.backend.exception.InvalidNumberOfMaximumStudyMember;
import com.yigongil.backend.exception.InvalidProcessingStatusException;
import com.yigongil.backend.exception.InvalidStudyNameLengthException;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import javax.persistence.Column;
Expand Down Expand Up @@ -59,20 +56,12 @@ public class Study extends BaseEntity {
@Enumerated(value = EnumType.STRING)
private ProcessingStatus processingStatus;

@Column(nullable = false)
private LocalDateTime startAt;

private LocalDateTime endAt;

@Column(nullable = false)
private Integer totalRoundCount;

@Column(nullable = false)
private Integer periodOfRound;

@Column(nullable = false)
@Enumerated(value = EnumType.STRING)
private PeriodUnit periodUnit;
private int minimumWeeks;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p5: 뭔가 속이 시원한데요 ㅋㅋ

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

객체도 지워버리시죠


@Column(nullable = false)
private Integer currentRoundNumber;
Expand All @@ -95,11 +84,8 @@ public Study(
ProcessingStatus processingStatus,
LocalDateTime startAt,
LocalDateTime endAt,
Integer totalRoundCount,
Integer periodOfRound,
Integer currentRoundNumber,
List<Round> rounds,
PeriodUnit periodUnit
List<Round> rounds
) {
name = name.strip();
validateNumberOfMaximumMembers(numberOfMaximumMembers);
Expand All @@ -111,13 +97,33 @@ public Study(
this.processingStatus = processingStatus;
this.startAt = startAt;
this.endAt = endAt;
this.totalRoundCount = totalRoundCount;
this.periodOfRound = periodOfRound;
this.periodUnit = periodUnit;
this.currentRoundNumber = currentRoundNumber == null ? 1 : currentRoundNumber;
this.rounds = rounds == null ? new ArrayList<>() : rounds;
}

public static Study initializeStudyOf(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3: 빌더, update 메서드를 비롯하여 minimumWeeks의 값이 할당되는 부분이 없는 이유는 너무 변경지점이 많아서 일단 보류한 것인가요??

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

민트의 코멘트를 보고 빌더가 있는데 왜 이걸 이렇게 썼지? 생각해보니 Period와 Round를 문자열만 받아 생성하기 어려워서였군요ㅋㅋㅋ.. 두 객체에 문자열을 받아 생성할 수 있는 책임을 넘겨주고 빌더에 .period(new Period(PeriodOfRound)) 처럼 스트링을 넘겨주는 편이 좋아보이는데 이때는 왜 이렇게 했을까요??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우린 확실히 뉴비였네요

String name,
String introduction,
Integer numberOfMaximumMembers,
LocalDateTime startAt,
Integer totalRoundCount,
String periodOfRound,
Member master
) {
Study study = Study.builder()
.name(name)
.numberOfMaximumMembers(numberOfMaximumMembers)
.startAt(startAt)
.totalRoundCount(totalRoundCount)
.periodOfRound(PeriodUnit.getPeriodNumber(periodOfRound))
.periodUnit(PeriodUnit.getPeriodUnit(periodOfRound))
.introduction(introduction)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이친구는 삭제가 필요하겠군요

.processingStatus(ProcessingStatus.RECRUITING)
.build();
study.rounds = Round.of(totalRoundCount, master);
return study;
}

private void validateNumberOfMaximumMembers(Integer numberOfMaximumMembers) {
if (numberOfMaximumMembers < MIN_MEMBER_SIZE || numberOfMaximumMembers > MAX_MEMBER_SIZE) {
throw new InvalidNumberOfMaximumStudyMember(
Expand All @@ -143,29 +149,6 @@ private void validateName(String name) {
}
}

public static Study initializeStudyOf(
String name,
String introduction,
Integer numberOfMaximumMembers,
LocalDateTime startAt,
Integer totalRoundCount,
String periodOfRound,
Member master
) {
Study study = Study.builder()
.name(name)
.numberOfMaximumMembers(numberOfMaximumMembers)
.startAt(startAt)
.totalRoundCount(totalRoundCount)
.periodOfRound(PeriodUnit.getPeriodNumber(periodOfRound))
.periodUnit(PeriodUnit.getPeriodUnit(periodOfRound))
.introduction(introduction)
.processingStatus(ProcessingStatus.RECRUITING)
.build();
study.rounds = Round.of(totalRoundCount, master);
return study;
}

public Integer calculateAverageTier() {
return getCurrentRound().calculateAverageTier();
}
Expand Down Expand Up @@ -214,60 +197,11 @@ private void finishStudy() {
this.processingStatus = ProcessingStatus.END;
}

public void startStudy() {
if (processingStatus != ProcessingStatus.RECRUITING) {
throw new CannotStartException("시작할 수 없는 상태입니다.", id);
}
if (sizeOfCurrentMembers() == ONE_MEMBER) {
throw new CannotStartException("시작할 수 없는 상태입니다.", id);
}
this.startAt = LocalDateTime.now();
this.processingStatus = ProcessingStatus.PROCESSING;
initializeRoundsEndAt();
}

private void initializeRoundsEndAt() {
rounds.sort(Comparator.comparing(Round::getRoundNumber));
LocalDateTime date = LocalDateTime.of(startAt.toLocalDate(), LocalTime.MIN);
for (Round round : rounds) {
date = date.plusDays(calculateStudyPeriod());
round.updateEndAt(date);
}
}

public int calculateStudyPeriod() {
return periodOfRound * periodUnit.getUnitNumber();
}

public String findPeriodOfRoundToString() {
return periodUnit.toStringFormat(periodOfRound);
}

public Member getMaster() {
return getCurrentRound().getMaster();
}

public void updateInformation(
Member member,
String name,
Integer numberOfMaximumMembers,
LocalDateTime startAt,
Integer totalRoundCount,
String periodOfRound,
String introduction
) {
validateMaster(member);
validateStudyProcessingStatus();
this.name = name;
this.numberOfMaximumMembers = numberOfMaximumMembers;
this.startAt = startAt;
this.totalRoundCount = totalRoundCount;
this.periodOfRound = PeriodUnit.getPeriodNumber(periodOfRound);
this.periodUnit = PeriodUnit.getPeriodUnit(periodOfRound);
this.introduction = introduction;
}

private void validateStudyProcessingStatus() {
protected void validateStudyProcessingStatus() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2: V1,V2를 없앴으니 접근제어자가 private이어도 좋을 거 같습니다!

if (!isRecruiting()) {
throw new InvalidProcessingStatusException("현재 스터디의 상태가 모집중이 아닙니다.",
processingStatus.name());
Expand Down Expand Up @@ -306,4 +240,15 @@ public Round getCurrentRound() {
public boolean isMaster(Member member) {
return getCurrentRound().isMaster(member);
}

protected void updateInformation(Member member, String name, Integer numberOfMaximumMembers, LocalDateTime startAt, String introduction) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2: 이 메서드도 private이어도 될 거 같습니다!

validateName(name);
validateNumberOfMaximumMembers(numberOfMaximumMembers);
validateMaster(member);
validateStudyProcessingStatus();
this.name = name;
this.numberOfMaximumMembers = numberOfMaximumMembers;
this.startAt = startAt;
this.introduction = introduction;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update도 이렇게 말고 변수마다 구현해줄 걸 그랬네요ㅋㅋㅋㅋㅋ.. 단일 책임 원칙을 경험에서 배워갑니다.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
ALTER TABLE round
ADD COLUMN week_number integer;
ALTER TABLE study
ADD COLUMN minimum_weeks integer NOT NULL;

create table progress_day_of_week
(
study_id bigint not null,
progress_day_of_week integer not null,
primary key (study_id, progress_day_of_week)
);

ALTER TABLE progress_day_of_week
ADD CONSTRAINT FKbto00gu5esijcq0wtn13fd7pd
FOREIGN KEY (study_id) REFERENCES study(id);

ALTER TABLE study DROP COLUMN period_of_round;
ALTER TABLE study DROP COLUMN period_unit;
ALTER TABLE study DROP COLUMN total_round_count;
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,6 @@

class StudyTest {

@Test
void 스터디_주기를_계산한다() {
Study study1 = StudyFixture.자바_스터디_모집중.toStudy();
Study study2 = StudyFixture.자바_스터디_진행중.toStudy();

assertAll(
() -> assertThat(study1.calculateStudyPeriod()).isEqualTo(21),
() -> assertThat(study2.calculateStudyPeriod()).isEqualTo(3)
);
}

@Test
void 스터디를_다음_라운드로_넘기면_현재_라운드가_다음_라운드로_변한다() {
// given
Expand Down
Loading