Skip to content
This repository was archived by the owner on Jan 28, 2024. It is now read-only.

refactor: hashSet을 위한 equals, hashCode 메소드 오버라이딩 #93

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

lyle0101
Copy link
Member

@lyle0101 lyle0101 commented Feb 7, 2021

No description provided.

Copy link
Member

@junhaesung junhaesung left a comment

Choose a reason for hiding this comment

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

equals, hashcode 에 대해서 좀 더 알아보고 다시 구현하면 좋을거같습니다!
entity 와 value object 의 차이에 대해서도 보시면 좋을거같아요

// then
Set<DayOfWeek> updatedAppearanceDays = updatedStore.getAppearanceDays().stream().map(AppearanceDay::getDay).collect(Collectors.toSet());
Set<PaymentMethodType> updatedPaymentMethods = updatedStore.getPaymentMethods().stream().map(PaymentMethod::getMethod).collect(Collectors.toSet());
assertThat(updatedAppearanceDays).isEqualTo(Set.of(DayOfWeek.MONDAY, DayOfWeek.THURSDAY));
Copy link
Member

Choose a reason for hiding this comment

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

월, 목요일에서 월, 금요일로 변경하는 경우, AppearanceDay 객체들이 어떻게 저장되는지도 확인해주세요.
현재 코드로 보면 월, 목 2개 삭제 후 다시 월, 금에 해당되는 2개의 AppearanceDay 를 생성할 것 같은데요
목요일만 삭제하고, 금요일만 추가할 수 있다면 그렇게 수정하면 좋을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

이건 고민을 해봤는데 updateStore 메소드 내부적으로 hash set 을 비운 후에 데이터를 넣는 식으로 구현되어 있어서 문제가 되지 않을 것 같아 그대로 두었습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

hash set 을 비우고 데이터를 넣는 식으로 구현된 이유는 그 때 아래 링크와 같은 상황이 발생해서였습니다!
https://blog.leocat.kr/notes/2016/04/26/hibernate-no-longer-reference

lyle0101 added 2 commits March 6, 2021 22:06
- private 메소드 아래로 이동
- update 후 다시 데이터 조회하여 검증하도록 수정
- 각 테스트의 이름을 좀 더 구체적으로 수정
@lyle0101 lyle0101 marked this pull request as draft March 8, 2021 17:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants