-
Notifications
You must be signed in to change notification settings - Fork 3
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
refactor: 스터디 엔티티 수정 #466
Conversation
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.
에밀 고생하셨습니다~ 궁금한 부분이 좀 있어서 질문 몇 가지 남겼습니다. 답변 주시면 감사하겠습니다~
} | ||
|
||
private void validateStudyProcessingStatus() { | ||
protected void validateStudyProcessingStatus() { |
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.
p2: V1,V2를 없앴으니 접근제어자가 private이어도 좋을 거 같습니다!
this.currentRoundNumber = currentRoundNumber == null ? 1 : currentRoundNumber; | ||
this.rounds = rounds == null ? new ArrayList<>() : rounds; | ||
} | ||
|
||
public static Study initializeStudyOf( |
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.
p3: 빌더, update 메서드를 비롯하여 minimumWeeks
의 값이 할당되는 부분이 없는 이유는 너무 변경지점이 많아서 일단 보류한 것인가요??
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.
민트의 코멘트를 보고 빌더가 있는데 왜 이걸 이렇게 썼지? 생각해보니 Period와 Round를 문자열만 받아 생성하기 어려워서였군요ㅋㅋㅋ.. 두 객체에 문자열을 받아 생성할 수 있는 책임을 넘겨주고 빌더에 .period(new Period(PeriodOfRound)) 처럼 스트링을 넘겨주는 편이 좋아보이는데 이때는 왜 이렇게 했을까요??
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.
우린 확실히 뉴비였네요
@Column(nullable = false) | ||
@Enumerated(value = EnumType.STRING) | ||
private PeriodUnit periodUnit; | ||
private int minimumWeeks; |
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.
p5: 뭔가 속이 시원한데요 ㅋㅋ
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.
객체도 지워버리시죠
@@ -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) { |
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.
p2: 이 메서드도 private이어도 될 거 같습니다!
@Entity | ||
public class ProgressDayOfWeek { | ||
|
||
@EmbeddedId |
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.
p3: 이렇게 복합키를 사용할 수 있는 줄은 처음 알았네요 👍
엔티티의 id로 쓰인다는 점에서 클래스명을 ProgressDayOfWeekId
로 바꾸는 게 더 직관적일 거 같은데 어떻게 생각하시나요??
private Long studyId; | ||
|
||
@Column(name = "progress_day_of_week") | ||
private int progressDayOfWeek; |
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.
p3: 이게 스터디를 주 몇 일 하는지 나타내는 값인가요?? 왜 복합키로 사용한 건지 궁금합니다!
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.
고생많으셨습니다 에밀
이 녀석을 dev에 봉합시키는 순간 말도안되는 문제들이 생기겠군요. git 이 없었다면 정말 걱정됐을 것 같아요ㅋㅋㅋㅋㅋㅋ.
api 만들면서 하나씩 해결해보죠! 컴파일 에러 처리반 TEAM 201 화이팅입니다
this.currentRoundNumber = currentRoundNumber == null ? 1 : currentRoundNumber; | ||
this.rounds = rounds == null ? new ArrayList<>() : rounds; | ||
} | ||
|
||
public static Study initializeStudyOf( |
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.
민트의 코멘트를 보고 빌더가 있는데 왜 이걸 이렇게 썼지? 생각해보니 Period와 Round를 문자열만 받아 생성하기 어려워서였군요ㅋㅋㅋ.. 두 객체에 문자열을 받아 생성할 수 있는 책임을 넘겨주고 빌더에 .period(new Period(PeriodOfRound)) 처럼 스트링을 넘겨주는 편이 좋아보이는데 이때는 왜 이렇게 했을까요??
validateName(name); | ||
validateNumberOfMaximumMembers(numberOfMaximumMembers); | ||
validateMaster(member); | ||
validateStudyProcessingStatus(); | ||
this.name = name; | ||
this.numberOfMaximumMembers = numberOfMaximumMembers; | ||
this.startAt = startAt; | ||
this.introduction = introduction; |
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.
update도 이렇게 말고 변수마다 구현해줄 걸 그랬네요ㅋㅋㅋㅋㅋ.. 단일 책임 원칙을 경험에서 배워갑니다.
.periodOfRound(PeriodUnit.getPeriodNumber(periodOfRound)) | ||
.periodUnit(PeriodUnit.getPeriodUnit(periodOfRound)) | ||
.introduction(introduction) |
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.
이친구는 삭제가 필요하겠군요
|
||
@Column(nullable = false) | ||
@Enumerated(value = EnumType.STRING) | ||
private PeriodUnit periodUnit; | ||
private Integer progressDaysPerWeek; |
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.
Enum set으로 두는 건 어떨까요?
* fix: totalRoundCount, periodOfRound 삭제에 따른 컴파일 에러 해결 * fix: 엔티티 수정에 따른 컴파일 에러 임시 해결
* refactor: swagger 설정 변경 * refactor: swagger url 설정 파일로 관리
* feat: 동적 쿼리를 통해 스터디 목록 필터링/정렬 * refactor: config v1 prefix 제거 * refactor: 패키지 수정 * refactor: v1 prefix 제거 * refactor: status null 처리 * refactor: response 이름 수정 * refactor: param을 enum으로 컨버팅 * refactor: enum 순서 수정 * refactor: 메서드 추출
* refactor: Study.startAt 제거 및 minimumWeeks와 meetingDaysCountPerWeek 추가 * fix: 스터디 목록 조회 API url 수정 * fix: 실패하는 스터디 일부 복구 및 주석 작성 * test: 실패하는 테스트 복구
* refactor: 상세 조회 수정 * feat: 멤버 인증 정보 조회 응답 수정
* feat: 지원한 스터디 목록 조회 기능 * refactor: querydsl로 마이그래이션 * chore: 개행 * chore: 정렬 수정
* feat: 주차별 회차 조회 기능 추가 * refactor: 투두 -> 머스트두 변경, 생성 / 수정 통합 * Update backend/src/test/java/com/yigongil/backend/domain/round/RoundTest.java * Update backend/src/test/java/com/yigongil/backend/domain/round/RoundTest.java
👍 관련 이슈
기획의 어떤 부분을 구현 / 수정했는가 (굉장히 상세하게 적어주세요, 해당 커밋의 링크, 코드의 위치를 남겨주면 더욱 좋습니다.)
밑은 예시입니다.
이슈 링크