-
Notifications
You must be signed in to change notification settings - Fork 5
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
스크랩 엔티티 구현 (issue #489) #496
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package develup.domain.scrap; | ||
|
||
import develup.domain.IdentifiableEntity; | ||
import develup.domain.member.Member; | ||
import jakarta.persistence.Column; | ||
import jakarta.persistence.Embedded; | ||
import jakarta.persistence.Entity; | ||
import jakarta.persistence.FetchType; | ||
import jakarta.persistence.JoinColumn; | ||
import jakarta.persistence.ManyToOne; | ||
|
||
@Entity | ||
public class Scrap extends IdentifiableEntity { | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(nullable = false) | ||
private Member member; | ||
|
||
@Embedded | ||
private ScrappedItem item; | ||
|
||
@Column(nullable = false) | ||
private boolean isScrapped; | ||
|
||
protected Scrap() { | ||
} | ||
|
||
public Scrap(Member member, ScrappedItem item) { | ||
this(null, member, item, true); | ||
} | ||
|
||
public Scrap(Long id, Member member, ScrappedItem item, boolean isScrapped) { | ||
super(id); | ||
this.id = id; | ||
this.member = member; | ||
this.item = item; | ||
this.isScrapped = isScrapped; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package develup.domain.scrap; | ||
|
||
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
public interface ScrapRepository extends JpaRepository<Scrap, Long> { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package develup.domain.scrap; | ||
|
||
import jakarta.persistence.Column; | ||
import jakarta.persistence.Embeddable; | ||
import jakarta.persistence.EnumType; | ||
import jakarta.persistence.Enumerated; | ||
|
||
@Embeddable | ||
public class ScrappedItem { | ||
Comment on lines
+8
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 중복 코드를 제거하기 위해서 연관관계 매핑을 사용하지 않도록 했습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
해당 논의는 이곳에서 이어나갈게요! 리브가 말씀해주신 것처럼, 조회시 불편함은 있습니다. 다만, 제가 가장 염려되었던 부분은 중복 코드의 발생이었어요. 가령 domain.discussion.comment 패키나 domain.solution.comment 패키지과 같은 형태처럼 domain.discussion.scrap 요런식으로 생기게되면, 또 나중에는 domain.discussion.like 이런식으로 구현해야하는가 아닌가?라고 생각했어요. 저 혼자 판단하기 애매한 문제인것 같아요. 다른분들도 반례와 의견을 공유해주시면 설계가 좋아질 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Approve] 디스커션과 솔루션 댓글 기능에서도 서로 중복된 코드가 상당히 많았어요. 만약 이 부분도 비슷한 방식으로 구현한다면 엄청난 중복 코드가 발생할 것 같아요. 그런 점에서 지금 설계가 조금 더 나은 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 중복 코드 vs 복잡성 중 고른다면 저는 차라리 중복 코드를 고를 것 같긴합니다. 그리고 로직이 동일해서 중복되어 보이지만 사실상 하는 역할은 달라지기 때문에 중복이 아니라고 생각하기도 하구요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 좋은 의견 남겨주셔서 감사합니다. 중복 코드도 코드 베이스를 복잡하게 하지 않나요? |
||
|
||
@Column(name = "item_id", nullable = false) | ||
private Long id; | ||
Comment on lines
+11
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기에 솔루션이나 디스커션의 아이디가 들어가는거죠? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네 그러한 의도입니다. |
||
|
||
@Column(nullable = false) | ||
@Enumerated(EnumType.STRING) | ||
private ScrappedItemType itemType; | ||
|
||
protected ScrappedItem() { | ||
} | ||
|
||
public ScrappedItem(Long id, ScrappedItemType itemType) { | ||
this.id = id; | ||
this.itemType = itemType; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package develup.domain.scrap; | ||
|
||
public enum ScrappedItemType { | ||
|
||
DISCUSSION, | ||
SOLUTION, | ||
; | ||
} |
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.
해당 컬럼이 존재하는 이유는 사용자가 스크랩 버튼을 반복해서 클릭하는 경우 insert와 delete를 반복해서 수행하는 것보다 update 비용이 더욱 저렴하다고 생각했기 때문입니다.
다른 분들의 의견을 들어보고 싶습니다! 👀
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.
[Comment]
비용이 적을것이라는 것에는 동감합니다. 하지만 그 비용을 아끼는 대가로 나중에 구현될 스크랩 API의 명세가 복잡해지거나 내부 구현이 복잡해질 것 같네요.
물론, 코드가 얼마나 복잡해질지 모르고, 비용차이에 따른 서버의 성능이 얼마나 향상될지도 모르는 거긴 합니다. 이런 상황에서 어떤 방향의 시도가 먼저 되어야 할까요? 코드의 복잡도는 수치로 측정하기가 힘든데, 성능 향상은 비교적 명확하게 측정할 수 있을 것 같아요. 그러니 저는 복잡도를 먼저 감수하는 것 보단, 단순함을 먼저 선택하고 후에 필요에 의해 개선하는 방향이 좋을 것 같아요.
그런데, 한편으론 이런식이면 아무것도 못하고 매 기능마다 똑같은 수준으로 작성하니 성장이나 학습 관점에선 별로 좋지 않을 수 있을 것 같네요.