[4기 김영주, 소재훈] JPA Mission 1 - 단일 Customer 엔티티를 이용한 CRUD #267

jay-so · 2023-07-26T10:40:31Z

🥇 요구 사항 및 구현 내용

미션1 : JPA 소개(단일 엔티티를 이용한 CRUD를 구현)

JPA 프로젝트 세팅
단일 엔티티를 이용한 CRUD를 구현

고객(Customer) 엔티티는 ID, 이름, 나이, 닉네임, 주소 정보를 가진다.
단일 고객 엔티티를 이용한 CRUD를 구현한다.

hanjo8813 · 2023-07-26T10:49:29Z

src/main/java/com/programmers/springbootjpa/domain/Customer.java

+    private Customer(String name, Integer age, String nickName, String address) {
+        this.name = name;
+        this.age = age;
+        this.nickName = nickName;
+        this.address = address;
+    }
+
+    public static Customer of(CustomerCreateRequest request) {
+        return new Customer(
+                request.getName(),
+                request.getAge(),
+                request.getNickName(),
+                request.getAddress()
+        );
+    }


롬복 @AllArgsConstructor(staticName = "of") 쓰면 바로 정적팩토리 메소드를 만들수 있습니다.
(딱 지금 코딩하신대로 롬복이 private 생성자를 써서 만들어줍니다)

반영: 76cbbdf

매개변수로 request DTO를 받고 있어서, 롬복에서 자동으로 팩토리 메서드를 생성해 주는 기능을 적용하기 어려울 것 같습니다.

그래서 빌더 패턴 이용해서, DTO 클래스 내에 Entity로 변환하는 toEntity() 메서드를 만들어서 사용하는 방향으로 했습니다.

Hchanghyeon

영주님, 재훈님~ 코드가 너무 깔끔한 것 같아요! 개인적으로 궁금한 사항들에 대해서 의견 남겨두었으니 확인 부탁드리겠습니다! 👍👍 1차 미션 고생많으셨습니다~!

Hchanghyeon · 2023-07-26T11:17:37Z

src/main/java/com/programmers/springbootjpa/domain/Customer.java

+import lombok.NoArgsConstructor;
+
+@Entity
+@Table(name = "customers")


혹시 테이블 명을 복수로 지으신 이유가 있으실까요?!

저는 개인적으로 아래에 링크에 있는 이유들 때문에 단수로 지으려고 합니다!

테이블 명명 딜레마 단수 대 복수 이름

반영: 0b68d4b

복수로 지은 이유가 따로 있는 것은 아니었습니다. 찾아보니 단수로 짓는 것이 더 권장되는 분위기네요! 그에 따라 변경했습니다.

Hchanghyeon · 2023-07-26T11:27:27Z

src/main/java/com/programmers/springbootjpa/domain/Customer.java

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


기본 키 생성 전략을 GenerationType.AUTO로 가져간 이유가 궁금합니다!

AUTO를 쓸 경우 MySQL이 꼭 IDENTITY전략을 가져가지 않고 TABLE로 가져간다고 합니다.
(저도 MySQL은 IDENTITY인 줄 알았지만,, 그렇다고 하네요..!!)
Hibernate 버전에 따라 AUTO의 자동 전략이 바뀔 수도 있는 위험이 있어서 그 부분에 대해서는 어떻게 생각하시는지 궁금합니다!

JPA 기본키 생성 전략, @GeneratedValue 사용시 주의점

반영 : 0b68d4b

AUTO 전략이 Dialect에 맞게 하이버네이트가 알아서 전략을 선택해주기 때문에 해당 전략으로 했었습니다.

또한 강의에서도 실무에서는 AUTO 전략을 많이 쓴다고 해서 선택한 점도 있었습니다.

그런데 하이버네이트 5버전 부터는 AUTO를 선택하면 Mysql의 경우에는 TABLE이 선택된다는 걸 이번에 창현님 덕분에 알게되었네요. 그래서 IDENTITY 전략을 사용할 수 있도록 명시했습니다. 감사합니다.

Hchanghyeon · 2023-07-26T11:37:34Z

src/main/java/com/programmers/springbootjpa/domain/Customer.java

+@Table(name = "customers")
+@Getter
+@NoArgsConstructor
+@EqualsAndHashCode


현재 equals나 hashcode를 사용하는 곳이 없는 것 같은데 선언한 이유가 따로 있으실까요?!

반영: 0b68d4b

테스트 코드 용으로 하나 만들었다가, 결국 관련된 코드를 쓰지 않게 되었는데 미처 해당 애너테이션을 지우지 못했었네요.

Hchanghyeon · 2023-07-26T11:51:21Z

src/main/java/com/programmers/springbootjpa/domain/Customer.java

+    private String name;
+
+    @Column(name = "age")
+    private Integer age;


age를 Primitive 타입이 아닌 Wrapper 클래스를 사용한 이유가 궁금합니다!

물론 @Id로 선언한 id의 경우 long으로 했을 때 값이 0이 있을 수 있어 실제 값이 없는건지 아니면 키가 0으로 설정되었는지 알 수 없어서 Long으로 선언하기도하지만 다른 필드의 경우 이러한 문제랑 상관이 없다고 생각이 들어서요!

그리고 int의 경우 4byte를 사용하지만 Integer의 경우 클래스 포인터, 플래그, 락, int 값까지 총 16byte가 사용된다고 합니다!
유저의 수가 증가하면 증가할 수록 부담이 될 수도 있지 않을까? 라는 생각이 듭니다!

여러 이유가 있을 것 같은데요. 일단 Id가 Long 이기 때문에 Wrapper 클래스로 모두 통일하기 위함도 있었습니다.

추가적으로 age 필드의 경우 nullable이 true로 되어 있어서 null이 될 수 있는데요. (나이가 입력되지 않은 경우)

따라서 int보다는 Integer가 더 적합할 것이라 생각했습니다!

Hchanghyeon · 2023-07-26T11:53:02Z

src/main/java/com/programmers/springbootjpa/controller/CustomerController.java

+    private final CustomerService customerService;
+
+    @PostMapping
+    @ResponseStatus(HttpStatus.CREATED)


ResponseEntity를 사용하지 않고 Status 설정으로 처리하는 것도 보기 좋네요 👍👍

Hchanghyeon · 2023-07-26T11:55:04Z

src/main/java/com/programmers/springbootjpa/controller/CustomerController.java

+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+@RequestMapping("/api/v1/customers")


v1은 어떤 의미로 적으신건지 궁금합니다!

알아보니 api uri에 버전을 명시하는 이유는, 여러 api 기능 중 일부의 기능이 업데이트 되었을 때 클라이언트 단에서는 업데이트 된 api를 사용하게 되는데, 어떤 게 구버전이고 어떤 게 신버전인지 명시하기 위함의 목적도 있더라구요.

즉, 일종의 관례로 굳어진 네이밍인 것 같은데, 그런 차원에서 사용했습니다. 지금은 규모가 작기 때문에 딱히 필요는 없겠네요!

Hchanghyeon · 2023-07-26T12:00:03Z

src/main/java/com/programmers/springbootjpa/service/CustomerService.java

+    }
+
+    public List<CustomerResponse> findAllCustomers() {
+        return customerRepository.findAll().stream().map(CustomerResponse::of).toList();


저는 한줄에 점을 하나 찍어서 다음줄로 넘기면 가독성이 더 개선된다고 생각하여 길어질 경우 그렇게 적용하고 있는데 이 부분에 대해서는 어떻게 생각하시나요?!

Suggested change

return customerRepository.findAll().stream().map(CustomerResponse::of).toList();

return customerRepository.findAll()

.stream()

.map(CustomerResponse::of)

.toList();

약간 이런식으로여!

반영 : 0b68d4b

해당 부분은 프로젝트 패키지 전체를 대상으로 Reformat Code를 진행했는데, 자동으로 바뀐 부분이더라구요 ㅠㅠ

다시 원래대로 바꿔놨습니다!

Hchanghyeon · 2023-07-26T12:05:36Z

src/main/java/com/programmers/springbootjpa/service/CustomerService.java

+
+    @Transactional
+    public CustomerResponse updateCustomerById(Long id, CustomerUpdateRequest request) {
+        Customer customer = customerRepository.findById(id).orElseThrow(() -> new IllegalArgumentException("해당 ID에 해당하는 구매자가 없습니다."));


저는 IllegalArgumentException의 경우 사용자가 입력한 입력값 자체에 문제가 있을 때 사용하는 편이고 데이터베이스에서 조회를 했는데 없는 경우 NoSuchElementException을 사용하는 편인 것 같습니다! 이 부분에 대해서는 어떻게 생각하는지 궁금합니다!

반영 : 0b68d4b

사실 NoSuchElementException 의 존재에 대해 잘 모르고 있었어요!

찾아보니 IllegalArgumentException는 인수 자체에 문제가 있을 때이고, NoSuchElementException 는 해당 원소가 없는 경우에 조금 더 적합하더군요. 그래서 후자로 변경했습니다.

charlesuu

영주님, 재훈님 모두 수고하셨습니다~! 리뷰 남겨보았는데 확인해주시면 감사하겠습니다!

charlesuu · 2023-08-03T07:02:53Z

src/main/java/com/programmers/springbootjpa/controller/CustomerController.java

+
+    @PostMapping
+    @ResponseStatus(HttpStatus.CREATED)
+    public CustomerResponse createCustomer(@RequestBody CustomerCreateRequest request) {


CustomerResponse대신 유저 id만을 반환하거나, 아무 것도 반환하지 않는 방식에 대해서는 어떻게 생각하시나요? 저는 구체적인 요구사항이 주어지지 않은 상황에서는 최소한의 정보만을 반환하고 있기에, CustomerResponse를 반환하신 이유가 궁금합니다.

말씀하신대로 구체적인 요구사항이 주어지지 않았기 때문에, 굳이 고객 정보 전체를 반환할 이유가 없다는 것에 동감합니다.

어떤 서비스에서는 고객 정보를 생성하고, 해당 고객의 프로필과 같은 고객 정보를 볼 수 있는 페이지로 바로 이동하는 경우도 있을 것 같은데요. 그런 경우에는 생성과 동시에 해당 고객 정보를 받아와 화면에 출력할 수도 있겠습니다. 만약 createCustomer가 void를 반환한다면 고객 프로필 화면을 띄워주기 위해 findCustomerById 요청을 다시 보내야 할 것입니다. 물론 클라이언트 단에서도 고객 정보를 가지고 있기 때문에 굳이 다시 조회를 하지 않아도 될 거라 생각할 수도 있지만, 백엔드의 어떤 비즈니스 로직에 의해 프론트에서는 가지고 있지 않는 정보가 DB에는 들어있을 수도 있을겁니다. (기간을 넣었는데 날짜로 변환해서 DB에 넣는다거나 등등)

결국 요청을 두 번 보내지 않고, create와 동시에 고객 정보를 반환해준다면 해당 서비스에서만큼은 이 방식이 더 효율적일 수 있겠습니다. 말씀해주신대로 요구사항이 구체적이지 않았기 때문에, 저희끼리 자유롭게 상황을 설정해서 반환했다고 보시면 좋을 것 같습니다! 감사합니다.

charlesuu · 2023-08-03T07:05:25Z

src/main/java/com/programmers/springbootjpa/controller/CustomerController.java

+        return customerService.findAllCustomers();
+    }
+
+    @PatchMapping("/{id}")


엔티티 정보 중 일부만 수정하고 있기에 PatchMapping 해주신 점 좋네요!

charlesuu · 2023-08-03T07:12:56Z

src/main/java/com/programmers/springbootjpa/domain/Customer.java

+    private String nickName;
+
+    @Column(name = "address", nullable = false, length = 100)
+    private String address;


주소에는 '도로명 주소', '지번 주소', '우편 번호' 등이 정보가 포함될 수 있는데요. @Embedded, @embeddable 어노테이션을 경험해보기 위해 Address 객체를 따로 분리하여 3가지 필드를 포함시켜보는 것은 어떨까요? 참고

오 정말 좋은 기능이네요! 감사합니다. 덕분에 몰랐던 것을 알게된 계기가 된 것 같습니다. 다만 이번 미션에서는 최소 요구사항의 만족을 통한 기능 연습에 초점을 두고 있어서, 이번에는 따로 학습만 진행하고 실제 적용은 다음 기회에 한번 해보도록 하겠습니다!

charlesuu · 2023-08-03T07:16:05Z

src/main/java/com/programmers/springbootjpa/dto/CustomerResponse.java

+    private String nickName;
+    private String address;
+
+    public static CustomerResponse of(Customer customer) {


정적 팩토리 메서드가 하나의 매개 변수를 받는 경우 of 보다는 from 네이밍이 선호되는 것으로 알고 있습니다. 참고. from으로 바꿔주시면 좋을 것 같아요!

반영 : a2357b7

말씀하신대로 of 보다는 from이 더 어울려서 바꿨습니다. 감사합니다.

charlesuu · 2023-08-03T07:27:13Z

src/main/java/com/programmers/springbootjpa/service/CustomerService.java

+    private final CustomerRepository customerRepository;
+
+    @Transactional
+    public CustomerResponse createCustomer(CustomerCreateRequest request) {


Service레이어의 모든 기능 메서드에 'Customer' 라는 단어가 포함되고 있는데요. 정형화 된 CRUD 메서드만 존재하는 상황이고, 클래스명에 포함 된 단어이니 생략하는건 어떨까 싶습니다. 예를 들어 CustomerService.createCustomer() -> CustomerService.create()

저도 사실 해당 부분에 대해서는 많은 고민을 했었는데, 비슷한 고민을 하신 것 같아서 너무 재밌는 것 같습니다.
일단 저는 create 보다는 createCustomer가 더 낫지 않을까라는 의견을 가지고 있어요.

여러 상황을 생각해 볼 수 있는데, 만약 도메인이 조금 복잡해서 customer 말고 어떤 다른 것도 생성하는 기능도 추가된다면 어떨까요? 그렇다면 customerService.create()만으로는 정확히 고객을 생성하는 것인지, 아니면 고객과 관련된 다른 걸 생성하는 것인지 의미가 명확하지 않게 된다고 생각해요.

그리고 만약 의존성을 주입 받을 때, 권장되진 않지만 private CustomerService service;와 같은 형식으로 추가했다면 어떨까요?
그러면 service.create() 는 어떤 행위를 하는지 이것만 봐서는 절대 모를 것 같습니다. service.createCustomer() 는 그래도 대충 고객을 생성하는 기능이구나라는 걸 알 수 있지 않을까요? 물론 가장 중심적인 원인은 service 라고 모호한 변수명을 지었다는 것이지만, 이런 말도 안되는 상황까지 생각해봤을 때 createCustomer() 라고 메서드명을 짓는 것 자체가 다양한 상황에 대비할 수 있게 해준다고 생각합니다.

물론 현재 미션에서는 이런 상황까지 고려할 필요가 없을 정도로 단순하긴 해서 create()로 바꿔도 문제는 없을 것 같네요!

charlesuu · 2023-08-03T07:29:57Z

src/test/java/com/programmers/springbootjpa/domain/CustomerTest.java

+class CustomerTest {
+
+    @Test
+    void of() {


@DisplayName 어노테이션을 통해 무엇을 테스트하는 것인지 적어주시면 좋을 것 같습니다! 메서드 네이밍도 조금 바꿔주시면 좋을 것 같아여~

반영 : 36b08a5

너무 좋습니다! 바로 바꿨어요!

charlesuu · 2023-08-03T07:31:08Z

src/test/java/com/programmers/springbootjpa/domain/CustomerTest.java

+
+        Customer customer = request.toEntity();
+
+        assertThat(customer.getName()).isEqualTo("Kim Jae Won");


내부 필드의 동등성을 확인하기 위해 assertThat을 여러 번 사용하고 계신데요. 개인적으로 이런 코드도 가독성이 좋다고 생각하지만! 경험을 위해 usingRecursiveComparison() 메서드를 통해 자동으로 내부 동등성을 비교하는 방식으로 바꿔보는 것은 어떨까요?

반영: 36b08a5
좋은 피드백 감사합니다! usingRecursiveComparison() 메소드를 알지 못했었는데 덕분에 알게되었습니다! 감사합니다!!

jay-so added 6 commits July 26, 2023 17:25

feat: Customer엔티티 추가

e130466

feat: Customer CREATE 기능 추가

44cb5f8

feat: Custer READ 기능 추가

e1fcb3d

chore: application.yaml h2 DB 설정

67487b6

feat: Customer UPDATE 기능 추가

bf761fd

feat: Customer DELETE 기능 추가

81792d2

jay-so added Team-Tyler 🔥 매운맛 labels Jul 26, 2023

jay-so requested review from charlesuu and Hchanghyeon July 26, 2023 10:40

jay-so assigned kylekim2123 and jay-so Jul 26, 2023

jay-so marked this pull request as ready for review July 26, 2023 10:40

hanjo8813 reviewed Jul 26, 2023

View reviewed changes

Hchanghyeon reviewed Jul 26, 2023

View reviewed changes

jay-so added 2 commits July 31, 2023 22:23

refactor: DTO to Entity 메소드 추가

76cbbdf

style: Customer 관련 문구 수정

0b68d4b

charlesuu reviewed Aug 3, 2023

View reviewed changes

jay-so added 3 commits August 4, 2023 00:02

style: 정적 팩토리 메소드명 리팩토링

a2357b7

test: usingRecursiveComparison() 메소드를 통한 Customer entity 동등성 비교

36b08a5

style: DeleteMapping Url /추가

ccbed5f


		Customer customer = request.toEntity();

		assertThat(customer.getName()).isEqualTo("Kim Jae Won");

[4기 김영주, 소재훈] JPA Mission 1 - 단일 Customer 엔티티를 이용한 CRUD #267

Are you sure you want to change the base?

[4기 김영주, 소재훈] JPA Mission 1 - 단일 Customer 엔티티를 이용한 CRUD #267

Uh oh!

Conversation

jay-so commented Jul 26, 2023

🥇 요구 사항 및 구현 내용

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hchanghyeon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlesuu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!