[4기 김희빈] Weekely Mission 1, 2, 3 PR 제출합니다. #276

kmebin · 2023-07-27T13:03:44Z

📌 과제 설명 및 요구 사항

미션1 : JPA 소개 (단일 엔티티를 이용한 CRUD를 구현)
미션2 : 영속성 컨텍스트 (customer 엔티티를 이용하여 생명 주기 실습)
미션3 : 연관 관계 매핑 (order, order_item, item의 연관 관계 매핑 실습)

Shin-Jae-Yoon

희빈님 고생하셨습니다 !! 리뷰로 말하고 싶은 부분은 크게 3가지 인 것 같아요.

BaseEntity에서 키 값인 id를 가지고 있는게 맞는지 모르겠습니다.
@column을 사용하는 부분에 있어서 통일성을 가지면 좋을 것 같아요!
파라미터의 개수가 3개 미만인 경우에는 Builder보다 생성자를 통한 방법이 권장되는 것 같은데 이부분에 대한 의견이 궁금합니다 !

Shin-Jae-Yoon · 2023-07-27T14:25:42Z

src/main/java/com/programmers/jpabasic/global/common/BaseEntity.java

+
+    @CreatedDate
+    @Column(nullable = false, columnDefinition = "TIMESTAMP")
+    private LocalDateTime createdAt;


현재 updatable의 default 값이 true 일텐데, createdAt의 updatable이 true여도 괜찮을까요 ?

오 고려하지 못했던 부분이었는데 감사합니다! 바로 적용하겠습니다~

Shin-Jae-Yoon · 2023-07-27T14:36:18Z

src/main/java/com/programmers/jpabasic/global/common/BaseEntity.java

+
+    @Id
+    @GeneratedValue(strategy = GenerationType.AUTO)
+    private Long id;


BaseEntity의 역할이 단순히 엔티티가 사용하는 공통적인 정보를 모은다는 점에 있어서, 각 엔티티의 id를 가지고 있는 점이 어색한 것 같아요. Member의 id, Item의 id, Order의 id, OrderItem의 id가 공통적인 id라고 말할 수 있을까요 ?

BaseEntity 관련 게시글 같은 것을 참고해보면 좋을 것 같아요 !

id도 createdAt, updatedAt과 같이 모든 테이블에서 공통적으로 가지는 컬럼이라는 관점에서 작성했습니다..!
좀 더 고민해보겠습니다!!

Shin-Jae-Yoon · 2023-07-27T14:41:22Z

src/main/java/com/programmers/jpabasic/domain/member/entity/Member.java

+@Builder
+@AllArgsConstructor(access = AccessLevel.PRIVATE)
+@NoArgsConstructor(access = AccessLevel.PROTECTED)
+public class Member extends BaseEntity {


혹시 Member와 Customer는 어떤 차이가 있을까요 !?

강의에 맞춰서 미션 1, 2를 Customer로 수행하고, 미션 3에서 Member 사용했던 것입니다! 큰 의미 없습니다 😂

Shin-Jae-Yoon · 2023-07-27T14:41:52Z

src/main/java/com/programmers/jpabasic/domain/member/entity/Member.java

+public class Member extends BaseEntity {
+
+    @Column(nullable = false, length = 50)
+    private String name;


Customer에는 Name 타입으로 받았었는데, Member에서는 다시 String으로 받은 이유가 있을까요 ?

Member 테이블은 컬럼으로 name만 존재하는 경우라고 가정했습니다! 사실 크게 의미를 두진 않았습니다 허허

Shin-Jae-Yoon · 2023-07-27T14:43:08Z

src/main/java/com/programmers/jpabasic/domain/order/entity/Item.java

+    private int price;
+
+    @Column
+    private int stockQuantity;


int 타입을 사용하는 부분에 있어서 위의 price와는 다르게 이 부분이나, Member의 age에는 nullable = false를 주지 않았는데, 통일성이 있으면 좋을 것 같아요 ~!

재고량은 null과 0이 다른 의미를 가질 수 있지 않을까 생각했던 것인데, 또 생각해보니 다른 의미를 가짐으로써 더 혼동을 줄 수 있을 것 같네요..! 감삼당

Shin-Jae-Yoon · 2023-07-27T14:44:43Z

src/main/java/com/programmers/jpabasic/domain/member/entity/Member.java

+    private String address;
+
+    @Column(length = 100)
+    private String description;


description에 대한 부분은 아래에서 사용했던 @lob 와는 다르게 설명이 길지는 않을 것 같아서 length를 100으로 하신걸까요 !?

넵 맞습니다! 보통 서비스에서 개인 프로필 설정할 때 소개 적는 상황을 생각하고 길이 제한을 걸었습니다 ㅎ.ㅎ

parksey

희빈님 코드 작성하시느라 고생하셨어요!

다른 분들과 공통된 부분 외의 리뷰했습니다!!
추가적으로

public static Customer create(String firstName, String lastName) {
        return Customer.builder()
            .name(Name.of(firstName, lastName))
            .build();
    }

Customer에서 Builder와 create 2개가 사용되었는데 그렇게 하신 이유가 있나요?

코드가 전반적으로 깔끔했던 점이 인상 깊었습니다!! nullable을 붙이는 부분도 여러 상황을 고려하시는 모습이 좋았어요!

parksey · 2023-07-27T15:56:15Z

src/main/java/com/programmers/jpabasic/domain/order/entity/OrderItem.java

+
+    @ManyToOne(fetch = FetchType.LAZY)
+    @JoinColumn(name = "order_id")
+    private Order order;


양방향 매핑 하신 이유가 궁금합니다!

하나의 주문(order)에 주문한 아이템 목록을 조회하는 것은 비지니스 로직 상 자주 사용될 수 있다고 판단해서 양방향을 적용했습니다!

위의 이유라면! 관계테이블(OrderItems)에서 이미 가능한 것 아닌가요??

예를 들어, List items = OrderItemRepository.findByOrder(...)

parksey · 2023-07-27T15:57:44Z

src/main/java/com/programmers/jpabasic/domain/order/entity/OrderItem.java

+
+    @ManyToOne(fetch = FetchType.LAZY)
+    @JoinColumn(name = "item_id")
+    private Item item;


item은 단방향으로 매핑 하셨는데, 단방향 매핑을 하신 이유가 있을까요?

반대로 item은 아이템의 정보로써 의미를 가지고, 해당 아이템이 포함된 주문 아이템 목록을 필요로하는 일은 많지 않다고 판단했습니다. 필요할 때 쿼리를 따로 날리면 되기 때문에 단방향으로도 충분하다고 생각합니다!

parksey · 2023-07-27T16:09:56Z

src/main/java/com/programmers/jpabasic/domain/order/entity/Order.java

+
+    @Column(nullable = false)
+    @Enumerated(value = EnumType.STRING)
+    private OrderStatus status;


nullable을 사용하신 용도가 값을 무조건 받아야 한다. 라는 의미로 사용하신 건가요??
사용 하신거면 나중에는 따로 초기값을 지정해 주실 건가요?

넵 해당 컬럼에 null이 올 수 없다는 의미로 사용하였습니다! 생성될 때의 status 초기값이 항상 같기 때문에 default를 적용해도 좋을 것 같네요 감사합니당 👍

kmebin · 2023-07-27T16:58:05Z

희빈님 고생하셨습니다 !! 리뷰로 말하고 싶은 부분은 크게 3가지 인 것 같아요.

BaseEntity에서 키 값인 id를 가지고 있는게 맞는지 모르겠습니다.

@column을 사용하는 부분에 있어서 통일성을 가지면 좋을 것 같아요!

파라미터의 개수가 3개 미만인 경우에는 Builder보다 생성자를 통한 방법이 권장되는 것 같은데 이부분에 대한 의견이 궁금합니다 !

재윤님 꼼꼼한 리뷰 감사합니다 🙌

이 부분 관련해서 좀 더 고민해보겠습니다! 감사합니다 👍
지금은 ddl-auto로 테이블을 생성하는 경우, 자동으로 적용되는 옵션 외에 필요한 컬럼 옵션들만 추가하였는데 통일성 면에서 공감하기 때문에 수정해보도록 하겠습니다!
저도 이 부분 고민했었는데, 빌더 패턴을 통해 생성하는 방법으로 팀 컨벤션을 정했다고 가정했습니다. 3개 미만일때는 빌더가 과한 느낌이 확실히 있어서 계속 고민되네요 😂

kmebin · 2023-07-27T17:30:49Z

희빈님 코드 작성하시느라 고생하셨어요!

다른 분들과 공통된 부분 외의 리뷰했습니다!! 추가적으로
public static Customer create(String firstName, String lastName) {
        return Customer.builder()
            .name(Name.of(firstName, lastName))
            .build();
    }
Customer에서 Builder와 create 2개가 사용되었는데 그렇게 하신 이유가 있나요?

코드가 전반적으로 깔끔했던 점이 인상 깊었습니다!! nullable을 붙이는 부분도 여러 상황을 고려하시는 모습이 좋았어요!

세연님 리뷰 감사합니다!! 🫡

create라는 팩토리 메서드를 따로 둬서 가독성을 높이고, 내부적인 생성 로직을 드러내지 않게 하고 싶어 적용했습니다 :)

ymkim97

희빈님 과제 하시느라 고생하셨습니다!
이미 다른 분들이 정말 꼼꼼하게 잘 봐주셔서 하나만 남기고 갑니닷!

ymkim97 · 2023-07-27T17:31:03Z

src/test/java/com/programmers/jpabasic/domain/customer/entity/CustomerPersistenceTest.java

@@ -0,0 +1,122 @@
+package com.programmers.jpabasic.domain.customer.entity;
+
+import static org.assertj.core.api.Assertions.*;


가급적이면 와일드 카드 import는 쓰지 않는 것이 좋다고 합니다!
인텔리제이에서 자동으로 바꿔주는 것을 방지하기 위해 세팅할 수 있습니다!!

아 헐 설정할 수 있군여! 꿀팁 감사합니다 ><<

hongdosan

과제 고생하셨습니다~

hongdosan · 2023-07-27T22:51:42Z

build.gradle

@@ -0,0 +1,38 @@
+plugins {
+    id 'java'
+    id 'org.springframework.boot' version '2.7.14'


3.0.0 이상이 아닌 2.7.14 버전을 쓴 이유가 있을까요?

hongdosan · 2023-07-27T22:52:19Z

src/main/java/com/programmers/jpabasic/JpaBasicApplication.java

+import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.data.jpa.repository.config.EnableJpaAuditing;
+
+@EnableJpaAuditing


따로 Config Class를 만들지 않고 main 에서 설정한 이유가 있을까요~?

hongdosan · 2023-07-27T22:55:22Z

src/main/java/com/programmers/jpabasic/domain/customer/entity/Customer.java

+    @Embedded
+    private Name name;
+
+    public static Customer create(String firstName, String lastName) {


이펙티브 자바 - 아이템 1 : 생성자 대신 정적 팩터리 메서드를 고려하라 를 잘 활용하네요!

hongdosan · 2023-07-27T22:57:41Z

src/main/java/com/programmers/jpabasic/domain/order/entity/OrderItem.java

+
+    @ManyToOne(fetch = FetchType.LAZY)
+    @JoinColumn(name = "order_id")
+    private Order order;


위의 이유라면! 관계테이블(OrderItems)에서 이미 가능한 것 아닌가요??

hongdosan · 2023-07-27T22:58:34Z

src/main/java/com/programmers/jpabasic/domain/order/entity/OrderItem.java

+
+    @ManyToOne(fetch = FetchType.LAZY)
+    @JoinColumn(name = "order_id")
+    private Order order;


예를 들어, List items = OrderItemRepository.findByOrder(...)

hongdosan · 2023-07-27T23:00:26Z

src/test/java/com/programmers/jpabasic/domain/customer/entity/CustomerPersistenceTest.java

+        transaction.commit();
+
+        // then
+        Customer result = entityManager.find(Customer.class, customer.getId());


then에서 값을 받아오는 것은 어색한 것 같은데 어떻게 생각하세요??

kmebin added 7 commits July 26, 2023 09:43

init: 프로젝트 생성

98c004d

feat: 고객 CRUD 구현

f2d7435

test: ID로 고객 조회 시 검증부 수정

5bfc3f8

test: Customer 엔티티 생명 주기 테스트

380985a

feat: createdAt, updatedAt 컬럼에 not null 조건 추가

cb3d240

feat: Order, OrderItem, Item 연관 관계 매핑

ce16b8d

chore: 사용하지 않는 파일 제거

5251dfa

kmebin added 🔥 매운맛 Team-Joseph labels Jul 27, 2023

kmebin requested review from hongdosan, parksey, ymkim97 and Shin-Jae-Yoon July 27, 2023 13:03

kmebin self-assigned this Jul 27, 2023

fix: id SEQUENCE 전략으로 수정

0abf21a

Shin-Jae-Yoon reviewed Jul 27, 2023

View reviewed changes

kmebin added 2 commits July 27, 2023 23:50

feat: 지연 로딩 적용

dc19392

feat: createdAt 속성에 updatable=false 적용

0872ed8

parksey reviewed Jul 27, 2023

View reviewed changes

ymkim97 reviewed Jul 27, 2023

View reviewed changes

hongdosan reviewed Jul 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4기 김희빈] Weekely Mission 1, 2, 3 PR 제출합니다. #276

[4기 김희빈] Weekely Mission 1, 2, 3 PR 제출합니다. #276

kmebin commented Jul 27, 2023

Shin-Jae-Yoon left a comment •

edited

Loading

Shin-Jae-Yoon Jul 27, 2023

kmebin Jul 27, 2023

Shin-Jae-Yoon Jul 27, 2023

kmebin Jul 27, 2023

Shin-Jae-Yoon Jul 27, 2023

kmebin Jul 27, 2023

Shin-Jae-Yoon Jul 27, 2023

kmebin Jul 27, 2023

Shin-Jae-Yoon Jul 27, 2023

kmebin Jul 27, 2023

Shin-Jae-Yoon Jul 27, 2023

kmebin Jul 27, 2023

parksey left a comment •

edited

Loading

parksey Jul 27, 2023

kmebin Jul 27, 2023

hongdosan Jul 27, 2023

hongdosan Jul 27, 2023

parksey Jul 27, 2023

kmebin Jul 27, 2023 •

edited

Loading

parksey Jul 27, 2023

kmebin Jul 27, 2023

kmebin commented Jul 27, 2023 •

edited

Loading

kmebin commented Jul 27, 2023

ymkim97 left a comment

ymkim97 Jul 27, 2023

kmebin Jul 27, 2023

hongdosan left a comment

hongdosan Jul 27, 2023

hongdosan Jul 27, 2023

hongdosan Jul 27, 2023

hongdosan Jul 27, 2023

hongdosan Jul 27, 2023

hongdosan Jul 27, 2023

		@@ -0,0 +1,122 @@
		package com.programmers.jpabasic.domain.customer.entity;

		import static org.assertj.core.api.Assertions.*;

[4기 김희빈] Weekely Mission 1, 2, 3 PR 제출합니다. #276

Are you sure you want to change the base?

[4기 김희빈] Weekely Mission 1, 2, 3 PR 제출합니다. #276

Conversation

kmebin commented Jul 27, 2023

📌 과제 설명 및 요구 사항

Shin-Jae-Yoon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parksey left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmebin Jul 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmebin commented Jul 27, 2023 • edited Loading

kmebin commented Jul 27, 2023

ymkim97 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongdosan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shin-Jae-Yoon left a comment •

edited

Loading

parksey left a comment •

edited

Loading

kmebin Jul 27, 2023 •

edited

Loading

kmebin commented Jul 27, 2023 •

edited

Loading