-
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
[Feat] product entity 작성 #14
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.
고생하셨습니당~!
import lombok.extern.slf4j.Slf4j; | ||
import org.hibernate.annotations.DynamicInsert; | ||
|
||
@Slf4j |
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.
entity에 @slf4j를 사용하신 이유가 무엇인가?
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.
제가 알기론 컬럼이 너무 길거나 혹은 많은게 아니면 JPA에서 기본적으로 모든 필드에 대해 쿼리를 작성하는 것이 "쿼리 재사용"의 이유로 성능이 더 좋다고 알고 있는데 @DynamicInsert를 사용하신 이유가 궁금합니다!
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 Long id; | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "PRODUCT_ID", nullable = false) |
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.
저희 테이블에서 사실상 null이 가능한 경우를 가지는 컬럼은 없는 것 같은데 모든 컬럼에 "nullable = false" 사용해서 통일성 주는 것은 어떠신가요?
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.
좋아요~ 디자이너 분한테 여쭤봐서 null 없으면 안되도록 하는것도 말씀드렸어요!
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.
나 어제 새벽3시에 코드리뷰 달고 잤는데, 리뷰만 달고 Review changes를 안눌렀더니 안달렸네,,,,,,,,
import lombok.extern.slf4j.Slf4j; | ||
import org.hibernate.annotations.DynamicInsert; | ||
|
||
@Slf4j |
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.
엔티티 클래스에 로깅라이브러리가 있는 이유는 뭔가용?
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.
위에 코멘트 달려있어용
|
||
@Slf4j | ||
@Entity | ||
@Table(name = "AGE") |
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.
https://wakestand.tistory.com/689
아까 했던 이야긴데, @Entity
만 사용 시 테이블 명을 클래스 명으로 해 준대요. 없어도 될 것 같아요!
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.
근데 range같은건 예약어라서 테이블을 클래스명으로 하면 안돼서 일단은 달아놓을게요!
@Getter | ||
@DynamicInsert | ||
@AllArgsConstructor | ||
@NoArgsConstructor |
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.
빌더를 쓰면, 기본 생성자를 함부로 호출 할 수 없도록 accessLevel을 protected로 하는 것이 좋대요
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(name = "BRAND", length = 100) | ||
private String brand; | ||
|
||
@OneToMany(mappedBy = "PRODUCT") |
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.
이거 일대다인경우, 외래키를 가지는 쪽(BrandImage)이 컬럼을 가지고 있는것이 좋지 않을까요?
Product는 brandImages라는 리스트를 알 필요가 없고, 가지고 있을 필요도 없다.
오히려 brandImages에서 Product를 안다면(외래키로), findByProductId를 통해 가져오는것이낫다.
라는걸 얼핏 본 것 같은데, , , 어떻게 생각하세요?
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.
위에 커밋 바뀌어 있어용
🗃 Issue
🔥 Task
📄 Reference