-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/becooq81 step2 #6
base: becooq81
Are you sure you want to change the base?
Conversation
jpa: | ||
database-platform: org.hibernate.dialect.H2Dialect | ||
hibernate: | ||
ddl-auto: create |
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.
개발단계에서는 큰 상관이 없겠지만 실제 서비스 환경에서는 ddl-auto create 가 금기시 됩니다. 혹시 왜일까요?
|
||
@Entity(name = "Friendship") | ||
@Getter | ||
@SequenceGenerator(name = "FRIENDS_SEQ_GEN", sequenceName = "FRIENDS_SEQ", allocationSize = 1) |
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.
SequenceGenerator
를 사용할 때 allocationSize
를 1로 설정하게 될 경우 성능 문제가 발생할 수도 있는데 그 이유가 뭘까요?
Page<Friendship> findAllByFrom(Member from, Pageable pageable); | ||
Page<Friendship> findAllByTo(Member to, Pageable pageable); | ||
Optional<Friendship> findByFromAndTo(Member from, Member to); | ||
Optional<Friendship> findByFrom(Member from); |
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.
from_member
도 ManyToOne 필드인데 findByFrom
으로 한개의 결과값이 나올 수 있나요?
@CreationTimestamp | ||
private LocalDateTime createdAt; | ||
|
||
@UpdateTimestamp | ||
private LocalDateTime updatedAt; |
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.
요런 거의 대부분의 엔티티에 들어가는 필드들은 따로 빼서 BaseEntity
등과 같이 정의해두고 상속 받아서 사용하는 것도 좋습니다
public void addRequested(Friendship friendship) { | ||
this.requested.add(friendship); | ||
} | ||
|
||
public void addReceived(Friendship friendship) { | ||
this.received.add(friendship); | ||
} |
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.
한 엔티티에서 다른 엔티티를 다루는 멤버 메소드를 가지고 있는건 엔티티의 책임의 범위가 너무 넓어진 것 같아요. 따로 Service 로 빼는 것도 방법이 될 것 같아요~
friendshipRepository.save(friendship); | ||
member.addRequested(friendship); | ||
to.addReceived(friendship); | ||
checkIfFriendshipExists(friendship); |
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.
이건 from to 가 바뀐 friendship 이 존재하는지 확인하는 메소드인 것 같은데 그렇다면 함수명의 변경이 필요해 보여요. Friendship 이 존재하는지 여부를 확인해서 이미 존재하면 만들지 않는 듯한 메소드 같아서요.
"역방향 친구 관계가 존재하는지 확인하고 존재한다면 ~한다" 같은 의미가 이름에 포함되어야 할 것 같아요.
public List<Friendship> findFriendshipsByMember(String username, int page) { | ||
Member member = memberService.findMemberByUsername(username); | ||
PageRequest pageRequest = PageRequest.of(page, PAGE_SIZE); | ||
return friendshipRepository.findAllByFrom(member, pageRequest) |
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.
만약 상대가 나를 추가한 상황이면 어떻게 되나요? 상대가 나를 추가했어도 나의 친구 목록에는 보여야 할 것 같은데 findAllByFrom
이면 내가 추가한 사람들만 보이지 않나요?
No description provided.