[4기 황창현, 이현호] JPA Mission 1 - 단일 Customer 엔티티를 이용한 CRUD by Hchanghyeon · Pull Request #269 · prgrms-be-devcourse/springboot-jpa

Hchanghyeon · 2023-07-26T18:30:46Z

🥇 요구 사항 및 구현 내용

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

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

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

This reverts commit 417d0e2.

kylekim2123

창현님, 현호님 두 분 다 너무 고생 많으셨습니다!
제가 감히 코드리뷰 할 실력이 되나 싶지만, 그래도 궁금한 점이나 의논해 볼만한 점 남겨놨으니
같이 논의 많이 해보면 재밌을 것 같아요!
그럼 화이팅~~

kylekim2123 · 2023-07-28T09:31:38Z

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

+    }
+
+    @GetMapping("/{id}")
+    public ResponseEntity<CustomerResponse> readCustomer(@PathVariable Long id) {


readCustomerById 와 같이 id를 통해 customer를 조회한다는 이름으로 명시하면 어떨까요?

말씀하신대로 어색한 느낌이어서 메소드명 수정해두었습니다!

적용 커밋: a1af086

kylekim2123 · 2023-07-28T09:51:56Z

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

+    public ResponseEntity<Void> createCustomer(@Valid @RequestBody CustomerCreateRequest customerCreateRequest) {
+        customerService.createCustomer(customerCreateRequest);
+
+        return ResponseEntity.status(HttpStatus.CREATED).body(null);


@PostMapping @ResponseStatus(HttpStatus.CREATED) public void createCustomer(@Valid @RequestBody CustomerCreateRequest customerCreateRequest) { customerService.createCustomer(customerCreateRequest); }

이런 방식으로 ResponseStatus 애너테이션을 이용해서 상태코드를 반환하는 건 어떨까요?
그리고 어차피 RestController 애너테이션이 있기 때문에, ResponseBody 애너테이션이 각 메서드마다 적용된 상태라서,
ResponseEntity 객체말고 커스텀 객체나 void로 반환하면 null 값을 사용하지 않아서 더 좋을 것 같아요!
이후 메서드 들도 마찬가지입니다!

확실히 큰 의미가 없다면 ResponseEntity를 이용하는 것보다 실제 객체를 반환하고 @ResponseStatus를 이용하는게 좋아보이네요 ㅎㅎ 감사합니다! 👍👍 수정했어요!

적용 커밋: 358544f

kylekim2123 · 2023-07-28T09:53:10Z

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

+public class Address {
+
+    @Column(name = "street_address", nullable = false)
+    private String streetAddress;


Column 애너테이션과 필드명 사이에 한줄 씩 띄어주시면 더 가독성에 좋을 것 같습니다!

말씀하신대로 가독성 측면에서 좋지 못한 것 같아서 전부 수정했습니다!

적용 커밋: f5c3dc7

kylekim2123 · 2023-07-28T10:00:30Z

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

+        return ResponseEntity.ok(customerResponses);
+    }
+
+    @PatchMapping


CustomerUpdateRequest DTO에 어떤 customer를 수정할 것인지에 대한 ID를 받는 것 같더라구요.
그런데 REST API는 http method를 이용해 행위를 규정하고, uri를 이용해 자원을 나타내는 것을 지향한다고 알고 있습니다.
(관련 참고 : https://meetup.nhncloud.com/posts/92)
하지만 현재 update 메서드에서는 PATCH /api/customers 이므로 어떤 자원을 수정한다는 것이 명시되어 있지 않은 것 같습니다.
이것과 관련해서 PATCH /api/customers/{id} 이렇게 하면 REST에서 지향하는 바를 더 지킬 수 있지 않을까 하는데 어떠신가요?

사실 이 부분에 대해서 굉장히 고민을 많이했었는데요. 피드백을 받기전에는 어차피 Post로 받을텐데 포함시켜서 받아도되지않을까? 라는 생각을 했던 것 같아요. 말씀하신대로 REST API를 따르려면 uri를 이용해 자원을 나타내는 것이 맞는 것 같아요! 수정해서 반영했습니다!

적용 커밋 : 737fa92

kylekim2123 · 2023-07-28T10:02:42Z

src/main/java/com/programmers/springbootjpa/dto/request/CustomerCreateRequest.java

+public class CustomerCreateRequest {
+
+    @NotBlank(message = "이름은 공백이거나 값이 없으면 안됩니다.")
+    private String name;


여기도 한 칸 공백 줄을 두면 더 가독성에 좋을 것 같아요!

말씀하신대로 가독성 측면에서 좋지 못한 것 같아서 전부 수정했습니다!

적용 커밋: f5c3dc7

kylekim2123 · 2023-07-28T10:06:00Z

src/main/java/com/programmers/springbootjpa/dto/request/CustomerCreateRequest.java

+    @NotBlank(message = "이름은 공백이거나 값이 없으면 안됩니다.")
+    private String name;
+    @Min(value = 1, message = "나이는 한살보다 많아야합니다.")
+    @Max(value = 100, message = "나이는 백살보다 적여야합니다.")


오타인 것 같습니다!
"적여야합니다." => "적어야합니다."

감사합니다! 적용 했습니다!

적용 커밋: 39c94fe

kylekim2123 · 2023-07-28T10:07:42Z

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

+    private final String nickName;
+    private final Address address;
+
+    public CustomerResponse(Customer customer) {


request DTO에서는 정적 팩토리 메서드를 통해 DTO 객체를 생성하고 있는데, response DTO에서는 그냥 생성자를 사용하신 이유가 있을까요?

현재 RequestDto에서는 DTO객체를 생성하지않고 도메인을 생성하고 있습니다! DTO에서 도메인을 생성해주는거와 도메인에서 DTO를 생성해주는 것이 다른 결이라고 생각해서 그렇게 했던 것 같아요! 이 부분은 다음 과제 때 적용해서 반영해보도록 하겠습니다!!! 아무래도 하나로 통일되게 작성하는 것이 옳다고 생각이 드네요!

jay-so · 2023-08-02T08:31:12Z

창현님, 현호님 수고하셨습니다! 😀😀
서로 같이 피드를 주고 받으면서 의논했으면 좋겠습니다! 화이팅!!

jay-so · 2023-08-02T08:40:53Z

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

+    @Column(name = "street_address", nullable = false)
+    private String streetAddress;
+    @Column(name = "detailed_address", nullable = false)
+    private String detailedAddress;


해당 필드명은 AddressDetail이 더 괜찮을 것 같습니다! @colunm(name = "address_detail, nullable = false) 이렇게 변경할 수 있을 것 같아요!

말씀하신 필드명도 카멜케이스를 따를 수 있지만 네이밍시 조금 더 자연스러운 영어구조는 detailedAddress가 자연스러운 것 같습니다! 영어에서 일반적으로 형용사는 명사 앞에 위치해서 명사 + 형용사(addressDetail)보다 형용사 + 명사 형태의 구조인 detailedAddress가 더 어울리지 않을까요?

jay-so · 2023-08-02T08:52:04Z

src/main/java/com/programmers/springbootjpa/dto/request/CustomerCreateRequest.java

+    @NotBlank(message = "이름은 공백이거나 값이 없으면 안됩니다.")
+    private String name;
+    @Min(value = 1, message = "나이는 한살보다 많아야합니다.")
+    @Max(value = 100, message = "나이는 백살보다 적여야합니다.")


@Min, @Max 어노테이션으로 범위를 지정해도 좋지만 @Size어노테이션으로 최소값, 최댓값에 대한 범위를 하는게 더 나아보입니다! @Size(min = 1, max = 100) 으로 바꿀 수 있을 것 같아요!

@Size 어노테이션은 문자열의 길이에 대한 제한으로 Integer에 대한 값의 범위를 지정할 수는 없는 것으로 알고있습니다!

@Max와 @Size의 차이

jay-so · 2023-08-02T08:53:53Z

src/main/java/com/programmers/springbootjpa/dto/request/CustomerUpdateRequest.java

+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class CustomerUpdateRequest {
+
+    @PositiveOrZero(message = "id값은 0이상 이어야합니다.")


@PositiveOrZero 어노테이션은 null 값일 때 검증을 통과시킵니다!! @NotNull을 붙여주어서 null 값일때도 고려해주세요!!

REST API 형식에 맞지 않는 필드였어서 삭제처리했습니다~!

jay-so · 2023-08-02T09:13:59Z

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

+        customerRepository.findById(id)
+                .orElseThrow(() -> new NoSuchElementException("삭제할 사용자가 없습니다."));
+
+        customerRepository.deleteById(id);


deleteCustomer 메소드에서 findById와 deleteById 2개의 쿼리가 발생됩니다! 여기서 다음과 같이 바꿀 수 있을 것 같아요!

Customer customer = customerRepository.findById(id).orElseThrow ~~~ customerRepository.delete(customer);

재훈님께서 말씀하신 내용이 어떤 의미인지 파악을 잘 못하겠습니다! 재훈님께서 작성해주신 코드도 쿼리는 2개의 쿼리가 나가지 않나요?(select와 delete)

우선 저희가 짠 로직은 삭제 전 customer에 대한 조회를 하여 있는 유저인지 파악하는 용도로 findById를 사용했기 때문에 굳이 Customer를 받을 필요가 없어서 위와 같은 로직으로 작성해둔 상태입니다!

[4기 황창현, 이현호] JPA Mission 2 - 영속성 컨텍스트 생명주기 실습

jay-so · 2023-08-02T12:30:10Z

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

+
+    public List<CustomerResponse> readAllCustomer() {
+        List<Customer> customers = customerRepository.findAll();
+


해당 부분에서 바로 return문을 이용해서 다음과 같이 사용할 수 있을 것 같아요!

return customerRepository.findAll().stream(). ~~

저는 customerRespository.findAll()은 하나의 행위이기 때문에 stream과 이어서 붙여쓰면 오히려 가독성이 더 안좋다라고 느껴지는 것 같아요. customers.stream()을 쓰면 말씀하신 내용보다 명확하게 어떤 것에 대해서 stream()을 사용하는건지 알 수 있지 않을까요? 재훈님은 이 부분에 대해서 어떻게 생각하시나요?

jay-so · 2023-08-02T12:32:41Z

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

+                .toList();
+    }
+
+    public CustomerResponse readCustomer(Long id) {


readCustomre 대신 readById로 바꿀 수 있을 것 같습니다!

말씀하신대로 Customer는 클래스명에 포함되기 때문에 삭제처리했습니다~!

적용 커밋: c64bb48

Hchanghyeon

영주님, 재훈님 리뷰 꼼꼼히 해주셔서 너무 감사드립니다! 👍👍

적어주신 리뷰 반영해야할 것들은 반영하였고 얘기를 나누어보고싶은 것들도 적어놓은 상태이니 확인 부탁드리겠습니다~! 감사합니다

Hchanghyeon · 2023-08-04T07:09:35Z

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

+    @Column(name = "street_address", nullable = false)
+    private String streetAddress;
+    @Column(name = "detailed_address", nullable = false)
+    private String detailedAddress;


말씀하신 필드명도 카멜케이스를 따를 수 있지만 네이밍시 조금 더 자연스러운 영어구조는 detailedAddress가 자연스러운 것 같습니다! 영어에서 일반적으로 형용사는 명사 앞에 위치해서 명사 + 형용사(addressDetail)보다 형용사 + 명사 형태의 구조인 detailedAddress가 더 어울리지 않을까요?

Hchanghyeon · 2023-08-04T07:15:06Z

src/main/java/com/programmers/springbootjpa/dto/request/CustomerUpdateRequest.java

+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class CustomerUpdateRequest {
+
+    @PositiveOrZero(message = "id값은 0이상 이어야합니다.")


REST API 형식에 맞지 않는 필드였어서 삭제처리했습니다~!

Hchanghyeon · 2023-08-04T08:09:52Z

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

+    public ResponseEntity<Void> createCustomer(@Valid @RequestBody CustomerCreateRequest customerCreateRequest) {
+        customerService.createCustomer(customerCreateRequest);
+
+        return ResponseEntity.status(HttpStatus.CREATED).body(null);


확실히 큰 의미가 없다면 ResponseEntity를 이용하는 것보다 실제 객체를 반환하고 @ResponseStatus를 이용하는게 좋아보이네요 ㅎㅎ 감사합니다! 👍👍 수정했어요!

적용 커밋: 358544f

Hchanghyeon · 2023-08-04T08:10:29Z

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

+    }
+
+    @GetMapping("/{id}")
+    public ResponseEntity<CustomerResponse> readCustomer(@PathVariable Long id) {


말씀하신대로 어색한 느낌이어서 메소드명 수정해두었습니다!

적용 커밋: a1af086

Hchanghyeon · 2023-08-04T08:12:01Z

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

+        return ResponseEntity.ok(customerResponses);
+    }
+
+    @PatchMapping


사실 이 부분에 대해서 굉장히 고민을 많이했었는데요. 피드백을 받기전에는 어차피 Post로 받을텐데 포함시켜서 받아도되지않을까? 라는 생각을 했던 것 같아요. 말씀하신대로 REST API를 따르려면 uri를 이용해 자원을 나타내는 것이 맞는 것 같아요! 수정해서 반영했습니다!

적용 커밋 : 737fa92

Hchanghyeon · 2023-08-04T08:15:17Z

src/main/java/com/programmers/springbootjpa/dto/request/CustomerCreateRequest.java

+    @NotBlank(message = "이름은 공백이거나 값이 없으면 안됩니다.")
+    private String name;
+    @Min(value = 1, message = "나이는 한살보다 많아야합니다.")
+    @Max(value = 100, message = "나이는 백살보다 적여야합니다.")


@Size 어노테이션은 문자열의 길이에 대한 제한으로 Integer에 대한 값의 범위를 지정할 수는 없는 것으로 알고있습니다!

@Max와 @Size의 차이

Hchanghyeon · 2023-08-04T08:21:27Z

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

+    private final String nickName;
+    private final Address address;
+
+    public CustomerResponse(Customer customer) {


현재 RequestDto에서는 DTO객체를 생성하지않고 도메인을 생성하고 있습니다! DTO에서 도메인을 생성해주는거와 도메인에서 DTO를 생성해주는 것이 다른 결이라고 생각해서 그렇게 했던 것 같아요! 이 부분은 다음 과제 때 적용해서 반영해보도록 하겠습니다!!! 아무래도 하나로 통일되게 작성하는 것이 옳다고 생각이 드네요!

Hchanghyeon · 2023-08-04T08:23:18Z

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

+
+    public List<CustomerResponse> readAllCustomer() {
+        List<Customer> customers = customerRepository.findAll();
+


저는 customerRespository.findAll()은 하나의 행위이기 때문에 stream과 이어서 붙여쓰면 오히려 가독성이 더 안좋다라고 느껴지는 것 같아요. customers.stream()을 쓰면 말씀하신 내용보다 명확하게 어떤 것에 대해서 stream()을 사용하는건지 알 수 있지 않을까요? 재훈님은 이 부분에 대해서 어떻게 생각하시나요?

Hchanghyeon · 2023-08-04T08:23:35Z

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

+                .toList();
+    }
+
+    public CustomerResponse readCustomer(Long id) {


말씀하신대로 Customer는 클래스명에 포함되기 때문에 삭제처리했습니다~!

적용 커밋: c64bb48

Hchanghyeon · 2023-08-04T08:27:57Z

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

+        customerRepository.findById(id)
+                .orElseThrow(() -> new NoSuchElementException("삭제할 사용자가 없습니다."));
+
+        customerRepository.deleteById(id);


재훈님께서 말씀하신 내용이 어떤 의미인지 파악을 잘 못하겠습니다! 재훈님께서 작성해주신 코드도 쿼리는 2개의 쿼리가 나가지 않나요?(select와 delete)

우선 저희가 짠 로직은 삭제 전 customer에 대한 조회를 하여 있는 유저인지 파악하는 용도로 findById를 사용했기 때문에 굳이 Customer를 받을 필요가 없어서 위와 같은 로직으로 작성해둔 상태입니다!

Hchanghyeon added 10 commits July 27, 2023 01:34

feat: customer 도메인 구현

e50a5b1

feat: CustomerController 구현

2ade203

feat: CustomerController관련 DTO 구현 및 Valid

e2ae899

feat: JPA로 CustomerRepository구현

5e48d52

feat: CustomerService 로직 구현

26fe500

refactor: 역직렬화 Getter추가

12ff6c0

feat: Customer 도메인 업데이트 관련 메소드 추가

0bee2e9

feat: Customer 도메인 업데이트 관련 메소드 추가

417d0e2

Revert "feat: Customer 도메인 업데이트 관련 메소드 추가"

e248791

This reverts commit 417d0e2.

feat: CustomerResponse dto객체 생성

effad61

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

Hchanghyeon requested review from jay-so and kylekim2123 July 26, 2023 18:30

Hchanghyeon assigned charlesuu and Hchanghyeon Jul 26, 2023

feat: 영속성 컨텍스트 생명주기 테스트 구현

209b83f

kylekim2123 reviewed Jul 28, 2023

View reviewed changes

jay-so reviewed Aug 2, 2023

View reviewed changes

Merge pull request #280 from prgrms-be-devcourse/창현,현호-mission2

acedb9b

[4기 황창현, 이현호] JPA Mission 2 - 영속성 컨텍스트 생명주기 실습

jay-so reviewed Aug 2, 2023

View reviewed changes

Hchanghyeon commented Aug 4, 2023

View reviewed changes


		public List<CustomerResponse> readAllCustomer() {
		List<Customer> customers = customerRepository.findAll();

Conversation

Hchanghyeon commented Jul 26, 2023

🥇 요구 사항 및 구현 내용

Uh oh!

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

jay-so commented Aug 2, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jay-so Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jay-so Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

jay-so Aug 2, 2023 •

edited

Loading

jay-so Aug 2, 2023 •

edited

Loading