4주차 : 고양이 장난감가게 만들기 #92

tmxhsk99 · 2023-07-24T22:43:00Z

안녕하세요 4주차 과제 리뷰요청 드립니다!
부족한 부분은 시간날때 조금씩 작성하겠습니다

hannut91 · 2023-07-25T02:09:02Z

app/src/main/java/com/codesoom/assignment/product/application/ProductService.java

+    public ProductResponse getProduct(Long id) {
+        Product product = productRepository.findById(id).orElseThrow(ProductNotFoundException::new);
+        return new ProductResponse(product);
+    }


도메인 객체를 바로 반환하지 않으려고 하신 것 같아요. ProductResponse를 Controller에서 응답을 결정하는 객체인 것 같네요. 서비스 레이어에서 웹 컨트롤러 레이어에서 사용되는 개념에 의존하고 있네요.

이러면 안쪽 레이어에서 바깥 쪽 레이어에 의존하게 돼요. 클린 아키텍처는 의존성의 방향이 자주 바뀌는 것에서 덜 바뀌는 쪽으로, 덜 중요한 곳에서 더 중요한 곳으로 향해야해요. 만약 응답을 다르게 해야된다고 했을 때, 서비스 레이어에서 영향을 받을 수 있을 것 같아요.

따라서 서비스 레이어에서 반환하는 것과 웹 컨트롤러에서 응답하는 것을 분리하는 것이 좋아 보입니다.

See also

https://codesoom.github.io/wiki/get-your-hands-dirty-on-clean-architecture/8/

hannut91 · 2023-07-25T02:09:25Z

app/src/main/java/com/codesoom/assignment/product/infra/api/ProductAdvice.java

+public class ProductAdvice {
+
+    @ExceptionHandler(ProductNotFoundException.class)
+    @ResponseStatus(value = HttpStatus.NOT_FOUND)


Suggested change

@ResponseStatus(value = HttpStatus.NOT_FOUND)

@ResponseStatus(HttpStatus.NOT_FOUND)

hannut91 · 2023-07-25T02:11:44Z

app/src/main/java/com/codesoom/assignment/product/application/ProductService.java

+
+@Slf4j
+@Service
+public class ProductService {


상품에 대한 여러가지 일을 하다보니 서비스의 이름에 Service가 붙을 수 밖에 없었던 것 같아요. 여러가지 일을 할 수록 보편적인 이름을 붙일 수 밖에 없게 돼죠. 서비스가 하나의 일만 하도록 나눠보는 것은 어떨까요?

See also

https://johngrib.github.io/wiki/article/hierarchical-controller-package-structure/

hannut91 · 2023-07-25T02:12:58Z

app/src/test/java/com/codesoom/assignment/product/application/ProductServiceTest.java

+    @Test
+    @DisplayName("상품 목록을 조회하면, 상품 목록이 반환된다.")
+    public void testGetProductList() {


Given When Then에 충분히 익숙해지신 것 같아요. 이렇게 테스트를 작성할 때 좋은 프레임워크가 있는데 바로 BDD테스트에요. BDD를 적용해 보는 것은 어떠신가요?

See also

https://johngrib.github.io/wiki/junit5-nested/

https://www.betterspecs.org/

hannut91 · 2023-07-25T17:02:36Z

CodeSoom/spring-week3-assignment-1#100 (comment)

책이 집에 있어서 답변이 조금 늦었습니다. 위에 커멘트도 한 번 확인 부탁드립니다.

hannut91 · 2023-07-26T02:04:14Z

app/src/test/java/com/codesoom/assignment/product/application/ProductCreatorTest.java

+    @Nested
+    @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
+    class createProduct_메서드는 {
+        @Nested
+        @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
+        class 상품요청정보가_주어지면 {


테스트 이름이랑 설명 주석이 중복이라서, 중복을 제거하셨네요. 테스트를 깔끔하게 작성하려고 노력을 하신 것 같습니다.

코틀린과 kotest를 같이 쓰면 이런 형식으로 작성하는 것도 가능합니다. 자바에서는 한계가 있긴 하지만, 더 좋은 방향이 있다는 것을 아는 것도 중요합니다.

beforeEach { categoryCreator = CategoryCreator(repository) } describe("CategoryCreator") { describe("create") { beforeEach { every { repository.save(any()) } returns category } it("생성된 카테고리를 반환한다") { val result = categoryCreator.create(name) result.name shouldBe name } } }

hannut91 · 2023-07-27T01:48:38Z

app/src/main/java/com/codesoom/assignment/product/infra/api/web/v1/ProductAdvice.java

+    @ExceptionHandler(ProductNotFoundException.class)
+    @ResponseStatus(HttpStatus.NOT_FOUND)
+    @ResponseBody
+    public ErrorResponse ProductNotFoundExceptionHandler(ProductNotFoundException e) {


메서드 이름이 camelCase가 아니군요.

hannut91 · 2023-07-27T01:50:31Z

app/src/test/java/com/codesoom/assignment/product/application/ProductCreatorTest.java

+                Assertions.assertEquals(PRODUCT_REQUEST.getName(), product.getName());
+                Assertions.assertEquals(PRODUCT_REQUEST.getMaker(), product.getMaker());
+                Assertions.assertEquals(PRODUCT_REQUEST.getPrice(), product.getPrice());
+                Assertions.assertEquals(PRODUCT_REQUEST.getImageUrl(), product.getImageUrl());


단순 비교라서 AssertJ를 쓰는 것이 엄청 좋아보이지는 않지만, 다른 곳에서는 AssertJ를 사용하고 있으니 일관성을 위해서 여기서도 AssertJ를 쓰는게 좋겠습니다.

hannut91 · 2023-07-27T01:51:01Z

app/src/test/java/com/codesoom/assignment/product/application/ProductDeleterTest.java

+                Product product = createProduct();
+                productRepository.save(product);
+                productDeleter.deleteProduct(product.getId());
+                Assertions.assertThat(productRepository.findById(product.getId())).isEmpty();


코드와 코드사이에 빈 줄로 구분 없이 작성되어 있네요.

우리가 글쓰기를 할 때도 문단으로 나눈어서 글쓰기를 하잖아요. 그 이유는 문단에 하고 싶은 내용이 모두 각각 다르기 때문입니다. 코드도 마찬가지 입니다. 코드가 하는 일을 개행으로 적절히 구분해주면 코드를 읽는 사람이 의도를 파악하는데 도움이 됩니다. 따라서 서로 연관이 있고 의도에 따라서 묶어 구분해 주면 좋을 것 같아요

궁금한 점이 있습니다
저런 문맥상의 일의 구분을 두는 개행 이외에
혹시 개행 관련 해서 암묵적으로 이렇게 한다 이런게 있나요?
메서드 시작 시 한 줄 개행 한다던가...
뭔가 찾아보는데 이렇다 할만한 게 없어서 그냥 느낌으로만 하는 느낌입니다.

음 그런건 없는 것 같아요. 팀의 컨벤션에 따르는게 좋을 것 같습니다. 느낌으로 하는게 맞아요. 제가 이 코드들은 한 번에 읽어야 합니다. 라고 말하고 있는 것이니까요.

제가 레퍼런스를 남기지 않았었군요.

Grouping From the other side of the looking glass, white space is grouping, making

sure that related statements are grouped together.

In writing, thoughts are grouped into paragraphs. A well-written paragraph contains
only sentences that relate to a particular thought. It shouldn’t contain extraneous sen-
tences. Similarly, a paragraph of code should contain statements that accomplish a
single task and that are related to each other.

Blank lines Just as it’s important to group related statements, it’s important to sepa-
rate unrelated statements from each other. The start of a new paragraph in English is
identified with indentation or a blank line. The start of a new paragraph of code
should be identified with a blank line.

Using blank lines is a way to indicate how a program is organized. You can use them
to divide groups of related statements into paragraphs, to separate routines from one
another, and to highlight comments.

Although this particular statistic may be hard to put to work, a study by Gorla,
Benander, and Benander found that the optimal number of blank lines in a program
is about 8 to 16 percent. Above 16 percent, debug time increases dramatically (1990).

See also

Code Complete 2 - [7부] 소프트웨어 장인정신 > 31장: 레이아웃과 스타일 > 31.2 레이아웃 기법

hannut91 · 2023-07-27T01:51:50Z

app/src/test/java/com/codesoom/assignment/product/application/ProductReaderTest.java

+                productRepository.save(testProduct);
+
+                Assertions.assertThat(productReader.getProductList()).isNotEmpty();
+                Product product = productReader.getProductList().iterator().next();


interator를 직접 사용하고 있는 것이 익숙하진 않네요.

hannut91 · 2023-07-27T01:52:56Z

app/src/test/java/com/codesoom/assignment/product/application/ProductUpdaterTest.java

+        class 수정할_상품이_있다면 {
+            @BeforeEach
+            void setUp() {
+                productRepository.deleteAll();
+            }
+
+            @Test
+            @DisplayName("해당_상품정보를_수정한_후_해당_상품을_반환한다")
+            void 해당_상품정보를_수정한_후_해당_상품을_반환한다() {
+                Product product = createProduct();
+                productRepository.save(product);
+                ProductRequest productRequest = new ProductRequest("updateProduct", "updateMaker", 2000, "updateImage");
+                Product updatedProduct = productUpdater.updateProduct(product.getId(), productRequest);
+
+                Assertions.assertThat(updatedProduct.getName()).isEqualTo(productRequest.getName());
+                Assertions.assertThat(updatedProduct.getMaker()).isEqualTo(productRequest.getMaker());
+                Assertions.assertThat(updatedProduct.getPrice()).isEqualTo(productRequest.getPrice());
+                Assertions.assertThat(updatedProduct.getImageUrl()).isEqualTo(productRequest.getImageUrl());
+            }
+        }


class 수정할_상품이_있다면 { @BeforeEach void setUp() { productRepository.deleteAll(); Product product = createProduct(); productRepository.save(product); } @Test @DisplayName("해당_상품정보를_수정한_후_해당_상품을_반환한다") void 해당_상품정보를_수정한_후_해당_상품을_반환한다() { ProductRequest productRequest = new ProductRequest("updateProduct", "updateMaker", 2000, "updateImage"); Product updatedProduct = productUpdater.updateProduct(product.getId(), productRequest); Assertions.assertThat(updatedProduct.getName()).isEqualTo(productRequest.getName()); Assertions.assertThat(updatedProduct.getMaker()).isEqualTo(productRequest.getMaker()); Assertions.assertThat(updatedProduct.getPrice()).isEqualTo(productRequest.getPrice()); Assertions.assertThat(updatedProduct.getImageUrl()).isEqualTo(productRequest.getImageUrl()); } }

이렇게 하면 위의 BeforeEach와 차이점이 바로 보여서 무엇이 다른지 명확하게 보일 것 같아요

hannut91 · 2023-07-28T01:38:32Z

app/src/main/java/com/codesoom/assignment/product/application/ProductDeleter.java

+    }
+
+    public void deleteProduct(Long id) {
+        Product product = productRepository.findById(id).orElseThrow(ProductNotFoundException::new);


ProductReader.getProduct를 사용할 수도 있겠네요

hannut91 · 2023-07-28T01:39:46Z

app/src/main/java/com/codesoom/assignment/product/application/ProductReader.java

+        return StreamSupport.stream(products.spliterator(), false)
+                .collect(Collectors.toList());


stream을 사용하셨군요. 아마도 Iterable을 받아서 List로 변환할려고 하신 것 같습니다. productRepository.findAll()의 결과로 바로 List를 받을 수 있을까요?

hannut91 · 2023-07-28T01:40:42Z

app/src/main/java/com/codesoom/assignment/product/application/ProductUpdater.java

+    @Transactional
+    public Product updateProduct(Long id, ProductRequest productRequest) {
+        Product product = productRepository.findById(id).orElseThrow(ProductNotFoundException::new);
+        product.update(productRequest.toProductEntity());
+        return product;
+    }


Transaction때문에 Entity.save를 명시적으로 호출하지 않아도 자동으로 반영되겠네요.

Note that the call to save is not strictly necessary from a JPA point of view, but should still be there in order to stay consistent to the repository abstraction offered by Spring Data.

하지만 추상화와 일관성을 위해서 명시적으로 작성하는 것도 좋다는 의견도 있으니 참고하시면 좋을 것 같습니다.

참고

https://docs.spring.io/spring-data/jpa/docs/current/reference/html/#transactions

hannut91 · 2023-07-28T01:41:57Z

app/src/main/java/com/codesoom/assignment/product/infra/api/web/v1/ProductController.java

+public class ProductController {
+
+    private final ProductCreator productCreator;
+    private final ProductReader productReader;
+    private final ProductUpdater productUpdater;
+    private final ProductDeleter productDeleter;


각 서비스도 분리해주었으니, 컨트롤러도 분리할 수 있겠어요

hannut91 · 2023-07-29T05:54:44Z

app/src/main/java/com/codesoom/assignment/common/dto/ErrorResponse.java

+@Getter
+@Setter
+@ToString
+public class ErrorResponse {


에러 응답 형식을 규격화 하셨네요. 애플리케이션에서 일관성있는 에러 응답으로 인해 예측 가능성이 높아졌네요. 규격을 정한다면 다양한 상황에서도 적절히 사용할 수 있어야겠습니다. 그래서 여러개의 에러 응답이 있는 경우에도 대응이 가능하도록 만들면 더 좋을 것 같습니다.

예를들어서 세탁기가 문이 안닫혀 있다고 해서 문을 닫았더니, 물이 부족하다는 에러가 발생하고, 물을 채웠더니 세제가 부족하다고 하고, 세제를 채웠더니 옷이 너무 많다고 에러가 나면 짜증나겠죠? 여러 에러가 있을 경우 한 번에 에러를 반환할 수 있어야 합니다. 에를들어서 다음과 같이 할 수 있습니다.

{ "message": "Invalid Request", "errors": [ { "source": "Door", "type": "DOOR_IS_OPENED", "message": "Door must be closed" }, { "source": "Detergent", "type": "DETERGENT_MISSING", "message": "Detegent is mandatory" } ] }

이렇게 한 번에 피드백을 전달하면 상호작용을 단순화할 수 있고 리퀘스트 요청 횟수도 줄일 수 있습니다.

See also

일상 속 사물이 알려주는 웹 API 디자인 > 5장 직관적인 API 디자인하기 > 5.2.4 철저한 에러 피드백 반환하기

hannut91 · 2023-07-30T06:57:48Z

app/src/test/java/com/codesoom/assignment/product/infra/api/ProductControllerTest.java

+    @Test
+    @DisplayName("상품 생성시 올바르지 않은 요청인 경우 InvalidProductRequest 발생 .")
+    void createProductInvalidRequest() throws Exception {
+        // given
+        ProductRequest productRequest = new ProductRequest("", "", 0, "");


좀 과해보일 수 있지만 @ParameterizedTest를 이용해서 상품의 이름만 없는 경우, 상호만 없는 경우 등을 엄격하게 테스트할 수 있겠네요.

See also

https://junit.org/junit5/docs/current/user-guide/#writing-tests-parameterized-tests

hannut91 · 2023-07-30T06:58:25Z

app/src/test/java/com/codesoom/assignment/product/infra/api/ProductControllerTest.java

+    @DisplayName("상품 삭제 요청 시 없는 경우 ProductNotFound 예외 발생")
+    void deleteProductsNotFound() throws Exception {


예외가 발생하는 것은 시스템 관점인 것 같아요. 웹 컨트롤러 테스트를 작성할 때는 프론트엔드 사용자 입장에서 어떤 응답을 받는지를 테스트하는 것이 좋을 것 같습니다.

tmxhsk99 added 20 commits July 24, 2023 12:15

test : 상품 등록 테스트 작성

b6dc20d

feat : 상품 등록 기능 작성

b7fab18

refactor: getter 롬복으로 수정

7a652f1

refactor: 불필요한 import 삭제

7f83bbb

refactor: final 추가 및 사용하지 않는 생성자 삭제

ce8b6da

refactor: final 추가 및 사용하지 않는 생성자 삭제

f152427

test : 상품리스트 조회 테스트 작성

c89696c

feat : 상품 리스트 조회 기능 작성

a3e15d3

feat : 단일 상품 조회 테스트 작성

a8be822

feat : 단일 상품 조회 기능 작성

9fb095d

test : 단일 상품 조회 실패시 테스트 작성

6987f97

feat : product 관련 조회관련 에러처리 로직 추가 Error Advice 예외처리 추가

3fcdf27

test : 상품 수정 테스트 작성

2629f0c

feat : 상품 수정 기능 작성

6e5c2a9

test : 상품 삭제 테스트 추가

feb3230

feat : 상품 삭제 기능 추가

07895af

test : 상품 삭제 예외 발생 테스트 추가

a2ab229

feat: cors 관련 설정 적용

cf1fbd0

feat: Transactional 추가

e8cafd3

test : ProductService 테스트 작성

c9a9e37

hannut91 reviewed Jul 25, 2023

View reviewed changes

tmxhsk99 added 3 commits July 25, 2023 16:37

refactor : 서비스 레이어와 웹 레이어 간의 의존성 제거

921019e

chore : value 삭제

377cd7f

refactor : 계층형 패키지 스타일로 수정

b0f3227

test : 기존 ProductServiceTest 삭제 및 BDD 형식 ProductCreateTest 추가

c693974

hannut91 reviewed Jul 26, 2023

View reviewed changes

tmxhsk99 added 3 commits July 27, 2023 04:55

test : ProductReader 테스트 작성

1ba2012

test : getProduct 테스트 추가

f2c036d

test : ProductUpdater,ProductDeleter 테스트추가

d5e7b72

hannut91 reviewed Jul 27, 2023

View reviewed changes

tmxhsk99 added 5 commits July 28, 2023 08:07

chore : 메서드명 camelCase로 수정

d72292b

chore : 코드 통일성을위해 AssertJ 사용으로 수정

6134110

chore : 문맥상의 일의 구분을 위한 개행처리

908298d

test : 기본 셋업 객체 생성 로직 셋업 메서드로 이동

95e3bbd

refactor : Iterable -> List 로 반환하도록 수정

b2395a3

hannut91 reviewed Jul 28, 2023

View reviewed changes

tmxhsk99 added 4 commits July 29, 2023 08:39

refactor : repository 에서 productReader 사용하여 상품을 찾도록 수정

227d922

test : deleterTest 의존성 수정

d237689

feat : crudRepository -> JpaRepository로 수정

07dc77c

refactor : contorller 분리

22dbd7e

hannut91 reviewed Jul 29, 2023

View reviewed changes

tmxhsk99 added 4 commits July 30, 2023 07:16

chore : 클래스명 수정

7357651

chore : lombok getter에서 일반 getter로 수정

0e31033

feat : ProductRequest 검증 로직 추가 및 InvalidProductRequest 커스텀 에러 추가

2a2513b

test : InvalidProductRequest 에러 발생 테스트 추가

a95ec89

hannut91 approved these changes Jul 30, 2023

View reviewed changes

test : InvalidProductRequest 발생 테스트 케이스별 추가

5a564ea

hannut91 merged commit 7007f6a into CodeSoom:tmxhsk99 Jul 31, 2023

	@ResponseStatus(value = HttpStatus.NOT_FOUND)
	@ResponseStatus(HttpStatus.NOT_FOUND)

		return StreamSupport.stream(products.spliterator(), false)
		.collect(Collectors.toList());

		@DisplayName("상품 삭제 요청 시 없는 경우 ProductNotFound 예외 발생")
		void deleteProductsNotFound() throws Exception {

4주차 : 고양이 장난감가게 만들기 #92

4주차 : 고양이 장난감가게 만들기 #92

Uh oh!

Conversation

tmxhsk99 commented Jul 24, 2023

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!

hannut91 commented Jul 25, 2023

Uh oh!

hannut91 Jul 26, 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!

tmxhsk99 Jul 27, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hannut91 Jul 26, 2023 •

edited

Loading

tmxhsk99 Jul 27, 2023 •

edited

Loading