[4기 박세연] Weekely Mission 1,3 PR 제출합니다 #275

parksey · 2023-07-27T11:35:13Z

📌 과제 설명

고객(Customer) 엔티티를 생성하고, 관련 CRUD기능 테스트 코드 추가

[연관관계 매핑]

고객 - 주문 : 매핑 X, 주문에만 customer id값 존재
- 고객에 주문에 대한 정보를 가지고 있을 필요가 없다고 생각했습니다.
- 따라서 Id값만 가지게 하는 방식으로 구성했습니다.
주문 - OrderItems : 양방향 매핑
- 주문에는 항상 주문 정보가 함께 와야 한다고 생각해서 강한 결합을 원했습니다. 따라서 두 관계를 양방향 매핑을 해주었습니다.
OrderItem - Item : 매핑 X, OrderItem에만 item에 대한 id값 존재
- 주문 정보에 대한 item이기 때문에, Item이 주문 정보를 가지고 있을 필요가 없다고 생각했습니다.
- 따라서 OrderItem에만 id값이 존재하도록 했습니다.

hongdosan · 2023-07-27T12:34:39Z

order/src/main/java/com/example/prog/orderingsystem/customer/Customer.java

+
+    @Id
+    @GeneratedValue(strategy = GenerationType.SEQUENCE)
+    protected Long customerId;


기본키 접근제어를 protected로 한 이유가 있나요?

아까 질문 드렸을 때 mock객체 만들 때 사용하고 못바꿨네요.. 원래는 private 사용합니다!

hongdosan · 2023-07-27T12:35:16Z

order/src/main/java/com/example/prog/orderingsystem/customer/Customer.java

+    @Id
+    @GeneratedValue(strategy = GenerationType.SEQUENCE)
+    protected Long customerId;
+    private String firstName;


다른 컬럼들은 @column 명시적으로 나타내지 않은 이유가 있나요?
그리고 이럴 경우 length = 255 로 설정될텐데 괜찮은가요?

나타내지 않은 이유는 없어요. 일관성을 위해서 고치는 게 좋을 거 같네요
네 length는 varchar를 기준으로 생각하고 있었어요!
따로 예외를 추가해야 겠네요

hongdosan · 2023-07-27T12:35:40Z

order/src/main/java/com/example/prog/orderingsystem/customer/Customer.java

+    private String lastName;
+
+    private Customer(Long customerId, String firstName, String lastName) {
+        this.customerId = customerId;


기본키도 값을 받도록한 이유가 있나요?

기본키 값 자체를 받도록 한 이유는 null값을 명시적으로 넣는 것을 보여주거나 나중에 기본 값을 주게 된다면
해당 값을 넣어주는 방식으로 사용할 예정이었습니다.
그렇기 때문에 전체 필드를 가지는 생성자는 접근을 못하도록 private을 사용했어요

hongdosan · 2023-07-27T12:36:07Z

order/src/main/java/com/example/prog/orderingsystem/order/domain/Item.java

+    private String name;
+
+    @Column(name = "price", nullable = false)
+    private Long price;


필드 타입을 Long로 한 이유가 있나요?

@builder와 값을 nullable한 시점에 기본 초기 값은 무조건 넣어야 한다고 생각했습니다.

물론 validation자체를 아직 넣어준건 아니였지만, depency나 valid메서드를 통해 값을 사용하여 처리를 할 수 있겠지만 Long을 통해 데이터가 들어오지 않았다는 것을 null로 바로 잡을 수 있다고 생각했습니다.

만약, 입력 데이터가 들어온지 검증할 필요가 없거나, 현재 상황처럼 기본 값을 0으로 줄 수 있다고 하면, long 으로 해도 상관없습니다!

hongdosan · 2023-07-27T12:36:36Z

order/src/main/java/com/example/prog/orderingsystem/order/domain/Item.java

+        this.description = description;
+    }
+
+    protected Item() {


다른 엔티티는 Lombok을 사용했는데 여기서 따로 생성해준 이유가 있나요?

단축키로 넣다 보니 저걸로 들어갔습니다.
이것도 일관성을 위해 변경해야 겠네요

hongdosan · 2023-07-27T12:38:29Z

order/src/main/java/com/example/prog/orderingsystem/order/domain/Order.java

+
+    @OneToMany(cascade = CascadeType.ALL)
+    @JoinColumn(name="order_id")
+    private List<OrderItem> orderItems = new ArrayList<>();


관계테이블을 둔 이유가 다대다 혹은 일대다를 다대일로 승격시키기 위함으로 알고 있는데,
굳이 관계테이블과의 관계도 양방향을 한 이유가 있나요??

먼저 과제의 요구사항에 있던 모든 테이블을 처음부터 사용하려 하지는 않았습니다. 관계를 구성하다 보니 이렇게 되었으며 설명 드리겠습니다.

Order와 Item은 서로 직접적인 연관 관계는 없습니다. 하지만 order에는 메뉴에 대한 기본 정보는 있었어야 했기 때문에 메뉴id를 가지고 있어야 했습니다.

원래는
Order에 menuId값을 가지고 있고, 관리를 하게 할 생각이었습니다.
이렇게 되면, order는 menuId를 가지고 있지만, menu의 경우 가지고 있을 필요가 없으므로
List
에 대해 가지고 있고 ,lazy를 적용하고 반대는 아무 값도 가지지 않기 때문에

@OneToMany(fetch = FetchType.LAZY) private List<Item> items= new ArrayList<>();

이런 식으로 구성했을 것 같습니다.

하지만 2가지 문제점이 생겼는데,

결국 @jointable이 적용될테니 order와 item간의 외래키가 조인테이블에 의해 한번 더 저장한다는 걸로 알고 있어서 구성을 변경해야 했습니다.

주문과 아이템간에 주문 아이템에 대한 상세 정보가 더 필요하다는 것을 느꼈습니다.

예를들어, 아이템을 주문한 갯수, 추가 옵션 등

따라서
1번만 있었다면, Item에 order_id가 있는 것은 말이 안되기 때문에 두 연관성을 낮춰 단방향으로 진행했을 것 같습니다.

다만 2번의 경우도 추가 되었기 때문에,
주문 갯수, 옵션등을 추가할 수 있는 orderItem이라는 테이블을 넣었습니다.
(이렇게 결과적으로 과제와 동일한 테이블들이 생기게 되었습니다.)

다시 관계를 보았는데, Order의 경우 OrderItem과 항상 함께 오기 때문에 강한 결합이 필요하다 생각했고
두 관계를 양방향으로 가지게 했습니다.
다만 lazy를 통해 해당 테이블을 필요하면 가져올 수 있도록 변경했고, Order의 경우 삭제가 된다면 OrderItem들도 전부 삭제가 되어야 하기 때문에 cascade를 넣어주었습니다.

마지막으로 OrderItem의 경우 item에 대한 상세 정보를 항상 가져올 필요가 있을까?에 대해 생각해보았고
해당 부분은 필요 없기도 하며, 양방향으로 인해 Order에서 값을 가져올 때의 관계도 끊어 줄 수 있다는 생각으로 id값만 가지게 했습니다.

설명이 좀 길긴 했지만, 제가 어떤 flow로 이러한 관계를 구성하게 되었는지 이해가 되셨으면 좋겠네요
성능이나 구성 자체가 이상하거나 문제가 될 수 있을 것 같은 부분은 말씀해주세요!

hongdosan · 2023-07-27T12:39:24Z

order/src/main/java/com/example/prog/orderingsystem/order/domain/OrderItem.java

+public class OrderItem {
+
+    @Id
+    @GeneratedValue(strategy = GenerationType.AUTO)


지금 기본키 전략이 어느 곳은 SEQUENCE고 여기는 AUTO인데 다르게 설정한 이유가 있나요?

H2로 SEQUENCE를 사용하는 것이 동일한 것은 알고 있었고, 여기서 테스트 코드로 서로 다른 가를 확인해 보려고 변경했다가 커밋에 적용을 못했네요!
변경해야겠네요

hongdosan · 2023-07-27T12:41:18Z

order/src/main/resources/application.yaml

+spring:
+  jpa:
+    generate-ddl: true
+    open-in-view: false


osiv 설정을 false로 한 이유가 있나요?

해당 부분은 강의 설정 그대로 가져왔고, 해당 osiv는 제가 자세히 모르는 관계로 설명은 못 드리겠네요. 추가적으로 공부하고 답변 드리겠습니다!

hongdosan · 2023-07-27T12:41:41Z

order/src/main/java/com/example/prog/orderingsystem/order/domain/Order.java

+    }
+
+    @Builder
+    private Order(Long customerId, String memo, List<OrderItem> orderItems) {


어떤 역할을 하는 건가요?

현재 생성자도 있고 Builder도 있어서 조금 헷갈리는 것 같아요

주문이 한번에 여러 아이템들이 추가 될 것을 생각해봤습니다. 예를 들어 패캐지 메뉴가 있다고 하면
해당 메뉴를 주문하면 프런트에서 그 값들을 파싱해서 줄 수 있고 여러 item들이 한 번에 들어오겠죠
그렇게 되면 Order에 초기 값을 전달을 해야 하고 그걸 표현하기 위해 추가했습니다.

hongdosan · 2023-07-27T12:48:51Z

고객에 주문에 대한 정보를 가지고 있을 필요가 없다고 생각했습니다.

이 말에 동의합니다. 고객은 주문을 알필요가 없습니다. 하지만 주문은 고객의 정보를 알아야하지 않을까요? 굳이 필드를 id로만 한 이유가 있을까요?

주문 - OrderItems : 양방향 매핑

OrderItems Table을 주문 정보라고 생각하신건가요? 제가 이해한 바로는 OrderItems는 Order - Item 간의 다대다를 다대일 관계로 승격시키기 위한 관계(매핑) 테이블이라고 생각했습니다. 주문 정보 테이블 이라고 하면 테이블 명을 바꾸거나 해야할 것 같습니다.
만약 관계 테이블이 맞다면! 굳이 Order와 Item이 OrderItems를 몰라도 될 것 같습니다. 관계 테이블 하나로 둘 모두를 관리할 수 있으니까요!

hongdosan · 2023-07-27T12:52:03Z

세연님 과제하느라 고생하셨습니다~

Shin-Jae-Yoon

안녕하세요 세연님. 고생하셨습니다!
전반적으로 다른분들과 겹치는 리뷰 내용도 있었습니다.

생성자와 Builder가 사용된 부분에서 개인적으로 어색함을 느꼈습니다.
사용하는 입장에서 어떨때는 이렇게, 어떨때는 저렇게 만드는 부분에서 혼란을 느낄 것 같아요!
테스트코드 작성법에 있어서 저와 스타일이 다른 것 같아서 어떻게 남겨야할 지 모르겠는데, 개인적인 생각으로는 테스트코드는 그 자체로 간단하고, 독립적이며, 빠르게 작성할 수 있어야 한다고 생각합니다. 그런면에 있어서 조금 보기 힘들었던게 있는 것 같아요. (지극히 주관적인 생각입니다!)

Shin-Jae-Yoon · 2023-07-27T14:10:07Z

order/src/main/java/com/example/prog/orderingsystem/order/domain/Order.java

+    }
+
+    @Builder
+    private Order(Long customerId, String memo, List<OrderItem> orderItems) {


현재 생성자도 있고 Builder도 있어서 조금 헷갈리는 것 같아요

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

order/src/main/java/com/example/prog/orderingsystem/order/domain/OrderItem.java

@@ -0,0 +1,25 @@
+package com.example.prog.orderingsystem.order.domain;
+
+import jakarta.persistence.*;


*와 같이 애스터리스크로 임포트하는 것보다 사용하는 것만 임포트하는 것은 어떨까요?

넵! 어떤 메서드를 사용하는지 보여주는 것이 더 좋은 것 같네요. 변경해야겠네요

parksey · 2023-07-27T14:36:59Z

은 주문을 알필요가 없습니다. 하지만 주문은 고객의 정보를 알아야하지 않을까요? 굳이

저도 처음에는 고객의 객체를 가져올 수 있도록 했었습니다 . 하지만 이런 생각이 들었어요 주문을 적용하는 시점이 회원을 가지고 있는 시점이 동일할까?

왜냐하면, 아마존이나 다른 이커머스에서 주문을 하게되면, 프런트에서 요청을 보내게 되겠죠, 그걸 처리를 하기 위해 서비스 단에서 로직 처리를 할 것으로 생각이 됩니다.

이때 주문이 들어가게 되면, 필요한 값들을 Db에서 가져와서 처리를 하게 되는데, Order에 고객까지 가지고 있게 된다면 고객의 테이블도 접근해야하지 않을까요? 그러면 그 여러 검증 로직을 거치는 동안에 검증을 진행하지 않아도 되는 customer를 조회하느라 시간이 소요될 것 같다 생각했습니다.

[결론]

결국 유저는 결국 결제를 할 때 해당 정보를 조회하면 되기 때문에 이러한 조회 시간과, 주문을 적용하는 시점에서의 간격이 있다고 생각했습니다. 따라서 객체를 넣지 않고 값을 넣었습니다.

parksey · 2023-07-27T14:39:02Z

주문 - OrderItems : 양방향 매핑

OrderItems Table을 주문 정보라고 생각하신건가요? 제가 이해한 바로는 OrderItems는 Order - Item 간의 다대다를 다대일 관계로 승격시키기 위한 관계(매핑) 테이블이라고 생각했습니다. 주문 정보 테이블 이라고 하면 테이블 명을 바꾸거나 해야할 것 같습니다.
만약 관계 테이블이 맞다면! 굳이 Order와 Item이 OrderItems를 몰라도 될 것 같습니다. 관계 테이블 하나로 둘 모두를 관리할 수 있으니까요!

그렇네요. 생각하신 방향이 맞습니다!! 확실히 클래스 및 테이블 명이 올바르지 못하다는 생각이 드네요 고쳐야할 것 같아요.

parksey · 2023-07-27T14:40:14Z

세연님 과제하느라 고생하셨습니다~

넵 혁준님도 리뷰해 주셔서 감사합니다!!

parksey · 2023-07-27T14:48:16Z

안녕하세요 세연님. 고생하셨습니다! 전반적으로 다른분들과 겹치는 리뷰 내용도 있었습니다.

생성자와 Builder가 사용된 부분에서 개인적으로 어색함을 느꼈습니다.
사용하는 입장에서 어떨때는 이렇게, 어떨때는 저렇게 만드는 부분에서 혼란을 느낄 것 같아요!

테스트코드 작성법에 있어서 저와 스타일이 다른 것 같아서 어떻게 남겨야할 지 모르겠는데, 개인적인 생각으로는 테스트코드는 그 자체로 간단하고, 독립적이며, 빠르게 작성할 수 있어야 한다고 생각합니다. 그런면에 있어서 조금 보기 힘들었던게 있는 것 같아요. (지극히 주관적인 생각입니다!)

사실 생성자와 빌더 말고도 일관성이 없는 부분은 많았다고 생각합니다. 그것 포함해서 변경해 볼게요!

생성자와 Builder가 사용된 부분의 경우 생성자는 private를 사용했는데, 그 이유로는 Builder에서 데이터의 검증을 해줘야할 수 있다고 생각했기 때문입니다. 또한 이렇게 하다보니 Builder와 생성자를 통해 초기 값을 따로 지정 이 가능했었는데 그러면 혹시 데이터 검증 또한 builder에서 하는게 나을까요?
어떻게 생각하시는지 궁금합니다!!

parksey · 2023-07-27T14:49:11Z

안녕하세요 세연님. 고생하셨습니다! 전반적으로 다른분들과 겹치는 리뷰 내용도 있었습니다.

생성자와 Builder가 사용된 부분에서 개인적으로 어색함을 느꼈습니다.
사용하는 입장에서 어떨때는 이렇게, 어떨때는 저렇게 만드는 부분에서 혼란을 느낄 것 같아요!

테스트코드 작성법에 있어서 저와 스타일이 다른 것 같아서 어떻게 남겨야할 지 모르겠는데, 개인적인 생각으로는 테스트코드는 그 자체로 간단하고, 독립적이며, 빠르게 작성할 수 있어야 한다고 생각합니다. 그런면에 있어서 조금 보기 힘들었던게 있는 것 같아요. (지극히 주관적인 생각입니다!)

넵 재윤님도 고생하셨습니다! 리뷰 남겨주셔서 감사해요!!

ymkim97

세연님 과제하시느라 고생하셨습니다!
저도 좀 늦게 달아서 이미 다른 분들이 잘 해주셔서 할게 딱히 없었습니다 ㅎㅎ

ymkim97 · 2023-07-27T16:57:22Z

order/src/main/java/com/example/prog/orderingsystem/order/domain/Item.java

@@ -0,0 +1,40 @@
+package com.example.prog.orderingsystem.order.domain;
+
+import jakarta.persistence.*;


가급적 와일드 카드 import는 쓰지 않는 것이 좋다고 합니다!
기본값으로 같은 import가 5개 이상이면 인텔리제이에서 자동으로 바꿔줘서 이를 예방하기 위해서 세팅 할 수 있습니다.

ymkim97 · 2023-07-27T17:02:35Z

order/src/test/java/com/example/prog/orderingsystem/OrderApplicationTests.java

+	@Test
+	void contextLoads() {
+	}
+


사용하지 않는 테스트 클래스는 없애는 것이 좋을 것 같습니다!
test/java 파일을 우클릭하고 Run All Tests를 할때 이것도 같이 돌아갑니다!

kmebin

다른 분들이 리뷰 잘 해주셔서 하나만 남깁니다! 세연님 과제 고생많으셨어요 ~ .~

kmebin · 2023-07-27T17:38:40Z

order/src/test/java/com/example/prog/orderingsystem/customer/CustomerPersistentTest.java

+@DisplayName("Customer 테스트")
+@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)


메서드에 @DisplayName을 적용하면 불필요한 부분이라고 생각하는데, 추가하신 이유가 있으실까요?!

feat: persistent테스트추가

885be9b

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

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

parksey self-assigned this Jul 27, 2023

hongdosan reviewed Jul 27, 2023

View reviewed changes

Shin-Jae-Yoon reviewed Jul 27, 2023

View reviewed changes

ymkim97 reviewed Jul 27, 2023

View reviewed changes

kmebin reviewed Jul 27, 2023

View reviewed changes

		@@ -0,0 +1,25 @@
		package com.example.prog.orderingsystem.order.domain;

		import jakarta.persistence.*;

		@@ -0,0 +1,40 @@
		package com.example.prog.orderingsystem.order.domain;

		import jakarta.persistence.*;

		@DisplayName("Customer 테스트")
		@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)

[4기 박세연] Weekely Mission 1,3 PR 제출합니다 #275

Are you sure you want to change the base?

[4기 박세연] Weekely Mission 1,3 PR 제출합니다 #275

Conversation

parksey commented Jul 27, 2023

📌 과제 설명

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 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongdosan commented Jul 27, 2023

hongdosan commented Jul 27, 2023

Shin-Jae-Yoon 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

parksey commented Jul 27, 2023

[결론]

parksey commented Jul 27, 2023 • edited Loading

parksey commented Jul 27, 2023

parksey commented Jul 27, 2023

parksey 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

kmebin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parksey Jul 27, 2023 •

edited

Loading

parksey commented Jul 27, 2023 •

edited

Loading

kmebin left a comment •

edited

Loading