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

[ADD] Routine 엔티티 구체화 #3

Merged
merged 1 commit into from
Jan 2, 2024
Merged

[ADD] Routine 엔티티 구체화 #3

merged 1 commit into from
Jan 2, 2024

Conversation

thguss
Copy link
Member

@thguss thguss commented Jan 1, 2024

Issue

closed #2

Description

  • Routine 편의 메소드 생성 중 필요하여 Member 엔티티 내 메소드 추가했습니다.
  • DailyTheme를 ENUM에서 Entity 형식으로 변경함에 따라 Member.themes 칼럼을 일부 임시 수정했습니다.
  • DailyTheme에서 이름, 내용, 이미지 url을 가지고 있어 Entity 타입으로 변환했습니다.
  • 더미데이터를 제외한 Entity 내 생성자와 편의 메소드를 추가했습니다.
  • 전체 Entity의 칼럼에 속성을 추가했습니다.
  • HappinessTheme(행복루틴 테마)에 기획 문서를 참고하여 임의로 값을 추가해두었습니다.

@thguss thguss added add 작업 추가 sohyeon 소현 작업 labels Jan 1, 2024
@thguss thguss requested review from csb9427 and Chan531 January 1, 2024 16:13
@thguss thguss self-assigned this Jan 1, 2024
Copy link
Contributor

@csb9427 csb9427 left a comment

Choose a reason for hiding this comment

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

전반적으로 생각 많이 하시고 한 것 같아 보기 좋습니다~~ 그런데 저희 변동사항이 조금 있어서 1월 2일 오늘 최종 GUI나오고 한 번 더 회의 해봅시당~~

public enum HappinessTheme {
CLOSER_TO_ME("나와 더 가까워지기💖"),
Copy link
Contributor

Choose a reason for hiding this comment

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

미리 이렇게 값까지 넣는 거 너무 보기 좋습니다~~

@@ -42,7 +43,7 @@ public class Member extends BaseTime {

@Column(columnDefinition = "TEXT", nullable = false)
@Convert(converter = StringListConverter.class)
private List<DailyTheme> themes;
private List<String> themes;
Copy link
Contributor

Choose a reason for hiding this comment

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

제가 잘 몰라서 그런데 원래 있던 거에서 String으로 바꾼 이유 한 번만 설명 부탁드려도 될까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

DailyTheme이 Enum에서 Entity로 바껴서 String으로 바뀐 거 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

@csb9427 찬님 말씀대로 Entity 값을 고대로 넣을 수 없기 때문에 Converter 가능한 String 형식으로 우선 변경해두었어요! 테마 관련 기능 확정되고 수정해도 좋을 듯 해요 :)

@@ -42,7 +43,7 @@ public class Member extends BaseTime {

@Column(columnDefinition = "TEXT", nullable = false)
@Convert(converter = StringListConverter.class)
private List<DailyTheme> themes;
private List<String> themes;
Copy link
Contributor

Choose a reason for hiding this comment

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

DailyTheme이 Enum에서 Entity로 바껴서 String으로 바뀐 거 같습니다!

ENVIRONMENT("환경🍃"),
EMPTY("비워내기🗑️"),
IMMERSION("몰입을 위한 준비🔥"),
OVERCOME_HELPLESSNESS("무기력 극복☔️"),
Copy link
Contributor

Choose a reason for hiding this comment

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

P4 : 여기 뒤에 , 붙어도 돌아가나요??? 궁금해서 여쭤봅니다 ㅎㅎ!!

Copy link
Member Author

@thguss thguss Jan 2, 2024

Choose a reason for hiding this comment

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

@Chan531 네! ,로 구분하고 ;로 맺음 짓는 것만 지키면 실행되는 것으로 알고 있어요. 나중에 값을 추가할 때 편리하도록 지정한 것이고, 코드 스타일을 맞추는 과정에서 끝에는 , 빼도 상관은 없습니다 ~!

Copy link
Contributor

Choose a reason for hiding this comment

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

좋은 거 알아갑니다~!

this.member.initHappinessRoutine();
}
this.member = member;
member.addHappinessRoutine(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

P4 : init을 통해 Member의 HappinessRoutine을 null로 만들어준 다음, add를 통해 HappinessRoutine를 this로 설정해주고 있는데 null로 만들어주는 과정이 중간에 있는 이유가 궁금합니다!! 그냥 this로 바로 설정해도 될 거 같아서요!!

Copy link
Member Author

Choose a reason for hiding this comment

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

@csb9427 해당 메소드는 member의 값을 선언해주는 것인데, 이미 선언된 member 값이 있다면 member의 happinessRoutine 멤벼 변수도 업데이트 해주어야 양방향 관계가 깨지는 것에 대비할 수 있어 추가한 로직이에요!
사실 현재 서비스 기획 특성 상 위와 같은 상황은 없을 것 같긴 해서 this로 바로 설정해도 큰 문제는 없을 것 같아요. 이 부분은 정기모임 때 함께 의논해서 합의점 찾아가면 좋을 것 같아요 :)

Copy link
Contributor

Choose a reason for hiding this comment

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

옙 정성담긴 답변 감사합니다!

}

private void setMember(Member member) {
if (Objects.nonNull(this.member)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nonNull 같은 것도 있군요! 배워갑니다!

@thguss thguss merged commit a49e00a into all_#1 Jan 2, 2024
@thguss thguss deleted the sohyeon_#2 branch January 2, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add 작업 추가 sohyeon 소현 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants