[Team07-BE] 메인, 상세보기 구현 #36

ghost · 2021-04-26T08:14:17Z

메인, 상세보기 구현

안녕하세요! team07백엔드 히로, 어거스트 입니다. 현재까지 구현한 부분에 대한 PR입니다.
상세한 피드백 부탁 드립니다. 감사합니다!

구현 내용

메인 페이지 출력 데이터 api 구현
- 3개의 각 카테고리에 대해서 아이템 목록을 ItemDto를 통하여 가져오도록 구현했습니다.
- Aggregate root를 Category로 하여 Item목록을 가져오도록 구현했습니다.
- ItemDto에 List필드를 가지며, 변환해주는 메소드toList를 별도로 구현했습니다.
상세보기 페이지 출력 데이터 api 구현
- categoryId와 Item의 hash값을 받아서 상품 상세보기에 대한 데이터를 응답 메시지로 보냅니다.
- DetailItemDto를 통해서 json 데이터를 보냅니다.
- ItemDto와 마찬가지로 List필드에 대해서는 변환해주는 메소드 toList를 사용하여 생성합니다.

TODO

주문과 OAuth에 대한 부분은 추후 추가할 예정입니다.

카테고리의 Set<Item>를 Map<String, Item>으로 변경 Category::findItem에서 O(1)만에 찾을수 있도록 함

DataTypeUtils::toList를 추가 Entity로 부터 String 필드 받아서 List로 변환 후 DTO에 저장하도록

ItemDto에서 배열 형태로 저장할 필요가 있는 필드에 대해 List로 변경 DataTypeUtils::toList를 활용하여 변경

DetailItemDto에서 배열 형태로 저장할 필요가 있는 필드에 대해 List로 변경 DataTypeUtils::toList를 활용하여 변경

DataTypeUtils::toList에서 "[]"가 제거되지 않아 replaceAll로 변경 불필요한 코드 제거 및 코드 정돈

wheejuni

안녕하세요, 수고 많으셨어요. 👍
아무래도 due date는 다가오고 요구사항은 복잡하고... 힘드시죠.
그래도 가끔은 코드를 작성하신 후, 돌아보는 시간을 가지시기를 권합니다.
간단하게 피드백 몇개 남겨 보았습니다. 설계에 대해 다시 고민하셔야 하는 부분도 있습니다.
확인하시고 재질문이나 반영을 부탁드릴게요. 감사합니다. 🥇

wheejuni · 2021-04-27T14:15:54Z

backend/sidedish/src/main/java/com/codesquad/sidedish/controller/CategoryController.java

+@RestController
+public class CategoryController {
+
+    CategoryService categoryService;


네, 뭔가가 빠졌죠

넵, 추가했습니다!

wheejuni · 2021-04-27T14:16:23Z

backend/sidedish/src/main/java/com/codesquad/sidedish/controller/CategoryController.java

+    }
+
+    @GetMapping("/main")
+    public List<CategoryDto> getMain() {


List 를 바로 리턴하기보단 상위 리스폰스 객체로 한번 감싸는 것을 추천합니다만,
클라이언트 개발자들과 협의된 형식인가요?

ResponseCategory라는 클래스로 List<Category>를 감싸주는 형식으로 변경했습니다.
(현재 시점에는 @GetMapping(/{categoryId})로 받아서 CategoryDto를 리턴하는 형태로 변경되었습니다.)

wheejuni · 2021-04-27T14:17:35Z

backend/sidedish/src/main/java/com/codesquad/sidedish/controller/ItemController.java

+    public DetailItemDto order(@PathVariable Long categoryId, @PathVariable String hash, int orderCount) {
+        categoryService.order(categoryId, hash, orderCount);
+
+        return getDetailItem(categoryId, hash);


컨트롤러의 각 메소드는 서로의 존재도 모를 정도로 독립적으로 작성해주세요.
컨트롤러 메소드를 내부에서 재활용하는 구현은 극히 피해야 할 방법입니다. 하나의 메소드만 떼내어 다른 클래스로 이동시켜도 컴파일 에러가 전혀 발생하지 않도록 코드를 작성해주세요.

wheejuni · 2021-04-27T14:18:12Z

backend/sidedish/src/main/java/com/codesquad/sidedish/controller/OrderController.java

+@RestController
+@RequestMapping("/")
+public class OrderController {
+}


사용하지 않는 클래스를 미리 작성할 필욘 없습니다. 심지어 @RequestMapping 도 되어있는지라, 실제 요청이 여기로 매치될텐데요, 핸들러 메소드는 아무것도 없네요.

네, 삭제했습니다!

wheejuni · 2021-04-27T14:20:30Z

backend/sidedish/src/main/java/com/codesquad/sidedish/domain/Item.java

+    private String detailHash;
+    private String image;
+    private String alt;
+    private String deliveryType;
+    private String title;
+    private String description;
+    private Integer normalPrice;
+    private Integer salePrice;
+    private String badge;
+    private String topImage;
+    private String thumbImages;
+    private String productDescription;
+    private Integer point;
+    private String deliveryInfo;
+    private String deliveryFee;
+    private String prices;
+    private String detailSection;
+    private int stock;


딱봐도 뭐가 너무 많고, 정리되지 않은듯 보이죠?
DB에 드나들기 위한 역할의 클래스라 하더라도, DB 관점이 아닌 객체지향 관점에서의 설계와 객체 역할 분리는 여전히 유효합니다.
우리는 Java 개발자지 SQL 개발자가 아니잖아요.

묶이고, 묶인 채로 분리되어야 할 정보들이 보일 겁니다.
더운 여름날 엄마 말씀처럼 가만~히 앉아서 들여다보면 보이실 겁니다.
마음 가는대로 클래스 분리를 얼마든지 시도해 보세요.

https://javabom.tistory.com/57
Spring Data JDBC도 @Embedded 를 지원합니다. 사용하시면 됩니다.

wheejuni · 2021-04-27T14:28:26Z

backend/sidedish/src/main/java/com/codesquad/sidedish/domain/Item.java

+        if(!checkStock(orderCount)) {
+            //custom exception 추가 예정
+        }
+        this.stock -= orderCount;


좋은 방법입니다만, 사이드 이펙트와 휴먼 에러 발생을 막는 차원에서 개인적으론 아래와 같은 구현을 선호합니다.

Suggested change

this.stock -= orderCount;

this.stock = this.stock - orderCount;

wheejuni · 2021-04-27T14:28:56Z

backend/sidedish/src/main/java/com/codesquad/sidedish/domain/User.java

+    String email;
+
+    String name;


이 두 필드에도 있어야 할 게 없는 모습이죠.
조금만 신경쓰시면, 이런 피드백은 받지 않으실 수 있습니다.

wheejuni · 2021-04-27T14:30:06Z

backend/sidedish/src/main/java/com/codesquad/sidedish/dto/DetailItemDto.java

+
+import java.util.List;
+
+public class DetailItemDto {


모든 필드에 @JsonProperty 를 붙일 필욘 없습니다.

wheejuni · 2021-04-27T14:31:05Z

backend/sidedish/src/main/java/com/codesquad/sidedish/dto/DetailItemDto.java

+    @JsonProperty
+    private List<String> detailSection;
+
+    public DetailItemDto(String detailHash, String topImage, List<String> thumbImages, String productDescription, Integer point, String deliveryInfo, String deliveryFee, List<String> prices, List<String> detailSection) {


가장 보고싶지 않은 유형의 생성자입니다. (2)
어떻게 개선해야 할까요? 이 생성자에 넘겨질 정보들은, 다 어디서 오는 정보들인거죠?
그 정보들을 담고 있는 클래스가 있었던 것 같은데요, 클래스 외부엔 그 클래스를 받는 메소드나 생성자만 노출하고, 이렇게 복잡하고 보고 싶지 않은 생성자는 가급적 숨기면 안될까요?

wheejuni · 2021-04-27T14:36:41Z

backend/sidedish/src/main/java/com/codesquad/sidedish/util/DataTypeUtils.java

+    public static List<String> toList(String s) {
+        s = s.replaceAll("\\[", "");
+        s = s.replaceAll("\\]", "");
+        s = s.replace("\"", "");
+
+        String[] strings = s.split(",");
+
+        return Arrays.asList(strings);
+    }


아.... 그다지 좋은 방법이 아닙니다.
만약 원소 중에 [ ] 문자가 있으면 저장을 못 하겠네요. 저장을 못하는 정도가 아니라 데이터 정합성이 꼬이겠네요.
괜찮을까요?

DB 연동 확인 테스트 코드 closes: #36

refactor : API관련 내용을 수정하라.

* Chore. Category 컴포넌트 생성 * Feat. Category UI #4 * Style. Add Header Styled Component #18 * Style. Head style and react icon 추가 #18 * Feat. delivery 작업 중 #9 * Style: Category style #4 * Feat. hover & badge 완료 * Style: Category UI #4 * Feat. 초기데이터 fetch 요청 -Fetch custom Hook 작성 -초기데이터: 메인메뉴, 카테고리1번 * Feat. tabClick 효과 구현 #33, MainMenu Fetch #3 * Style. categorySlide 1줄 나열. #4 * Chore. 더보기 버튼 구현시작 #5 * Feat. 상세페이지 폼 구현 - 배경색설정/상세페이지 위치 가운데로 설정/x버튼구현 - x버튼 클릭 시 기존 페이지로 보여줌 #36 * Chore. 불필요한 코드, 파일 제외하여 pr하기 * Feat: toggle button #5 * Feat. 상세데이터 fetch 받아옴 #38 * Feat: loading 중에 loading 이미지 삽입 #5 * Fix: 호버 이미지에 margin 적용 제거 #4 * Style: cursor and category title * Feat. 배포API주소로 변경 (CATEGORY제외) * Fix. REACT WARNING 수정: HEADER styled-components 선언 위치 수정 * Feat: env 파일에 url 넣기 * Chore: fetch 대신 axios 이용 * Fix: fetch error 임시로 고침 - detail 컴포넌트에서 처음에 fetch 받는 데이터를 유효한 url로 주어서 에러가 나오지 않게 했다. * Fix: fetch error fix - fetch hook 에 null 이 들어오면 fetch 를 하지 않게 해서 처음랜더링 될때 fetch 하지 않게 했음 - 더보기 클릭 연속으로하면 나타나는 warning Warning: Can't perform a React state update on an unmounted component. -> useEffect에 클린함수로 상태값 초기화 시켜줘서 해결 * Fix: loading 화면 띄우게 하기 * Feat: Error handling - useFetch 안에서 error 처리 * Fix: FeedBack 반영 - component 명칭 통일 * Fix: Feedback 반영 - ModalMode => modalMode * Feat: add Modal component * Feat: loading gif MainMenu * Feat: error component * Chore. 복구 * Chore. error 수정 * Feat. portal사용-> modalError 수정 * Fix: header hover 에러 고침 * Feat: stock 0 일시 버튼 disable * Refactor: OrderBtn * Feat. 수량 onChange 작업 중 * Chore. PRICEE + STOCK 관리 추가 MERGING * Feat. 상세페이지 이미지클릭시 대표이미지 변경 * Feat. error 컴포넌트생성 * Feat. errorPage에서 메인홈으로 이동 * Chore. rebase FE * Chore. rebase 2/13 * Chore : .gitignore 파일 생성 * Chore : 프로젝트 폴더인 BE 폴더 안에 .gitignore 파일 생성 * Chore : side dish 스프링 부트를 이용한 gradle 프로젝트 생성 * Feat : jdbc를 이용한 mysql 연결을 위해 application.properties 설정 * Feat : side dish 프로젝트의 schema.sql 파일 생성 * Chore: .gitignore 추가 * Feat: dish 관련 클래스와 레포지토리 생성 기본 프로젝트 골격을 잡기 위해 Dish, DishController, DishRepository, DishService, BestDishResponseDTO의 틀 생성 * Feat: badge 관련 클래스와 레포지토리 생성 기본 프로젝트 골격을 잡기 위해 Badge, BadgeRepository 생성 * Delete BE directory * Delete .gitignore * Chore. merging(error+oath) * Feat. useFetch method전달방식 변경 - Ooath 인증코드 8080 포트로 post요청 -cors blocked * Feat. 캐러셀 상세페이지 적용 * Feat. DetailPage 미니캐러셀 구현 중 * Feat. 재고주문api요청보냄, but실패 * Style : detailpage * - * Feat. LOGIN 요청중 * Chore. 로그인 작업구현중 * Feat. 재고수량 DB 업뎃가능, (새로고침시만) * Fix: error * Feat. Login구현 -atomic component화 * Feat. 받은 토큰으로 다시 get요청 -> 로그인정보 가져오기 구현 * Refactor . 파라미터 분해할당 * Chore. 불필요한 constant파일 삭제 * Feat.로그아웃 api 요청 * Update README.md * Chore. igonore설정 * Chore. * Chore. 끝나지않는 merge * Feat. 범용성있는 Carousel은 전역 폴더링. -기타 중복 파일 삭제 Co-authored-by: eamon3481 <68339352+eamon3481@users.noreply.github.com> Co-authored-by: jeong-inho <derosatam76@gmail.com> Co-authored-by: Jeong InHo <63284310+eNoLJ@users.noreply.github.com>

hiro032 added 21 commits April 19, 2021 16:16

build: Initialize gradle project setting

c157707

chore: Update .gitignore

1db1213

chore: Move .gitignore

35edbb2

chore: Delete .DS_Store

b282daa

feat: Add classes for Spring Data JDBC (#2)

07ab3a6

feat: Add script for schema & DDL (#2)

8bba605

feat: Add category findAll logic (#2)

d455f67

feat: Add GET request detail item logic (#2)

1665331

feat: Add Item DTO for main page (#6)

5b6bdcc

feat: Add Order & User Entity (#6)

368ba1e

refactor: Add dto factory methods in Item class (#6)

a68fd9d

feat: Add @JsonProperty to all DTO classes (#6)

84e95a1

feat: Add DetailItemDto transformation for JSON logic (#6)

880fa27

feat: Add order logic with problem (#8)

03bfe22

feat: Add Item order (#8)

67b387d

refactor: Change Set to Map (#8)

08ab5b5

카테고리의 Set<Item>를 Map<String, Item>으로 변경 Category::findItem에서 O(1)만에 찾을수 있도록 함

build: Update to connect MySQL in AWS (#8)

8f08c57

feat: Add DataTypeUtils (#8)

9a75175

DataTypeUtils::toList를 추가 Entity로 부터 String 필드 받아서 List로 변환 후 DTO에 저장하도록

refactor: Change ItemDTO field type String to List (#8)

6aff54e

ItemDto에서 배열 형태로 저장할 필요가 있는 필드에 대해 List로 변경 DataTypeUtils::toList를 활용하여 변경

refactor: Change DetailItemDto field type String to List (#8)

4a1748d

DetailItemDto에서 배열 형태로 저장할 필요가 있는 필드에 대해 List로 변경 DataTypeUtils::toList를 활용하여 변경

fix: Modify DataTypeUtils::toList (#8)

92e6e7d

DataTypeUtils::toList에서 "[]"가 제거되지 않아 replaceAll로 변경 불필요한 코드 제거 및 코드 정돈

ksundong added the review-BE BE 리뷰 label Apr 26, 2021

wheejuni self-requested a review April 27, 2021 12:36

wheejuni self-assigned this Apr 27, 2021

wheejuni requested changes Apr 27, 2021

View reviewed changes

sallyjellyy added a commit that referenced this pull request Apr 29, 2021

feat: 어플리케이션 테스트 코드 작성

1179c42

DB 연동 확인 테스트 코드 closes: #36

wheejuni pushed a commit that referenced this pull request Apr 29, 2021

Merge pull request #36 from swing-park/refactor-response-json-api

b0daa2e

refactor : API관련 내용을 수정하라.

	this.stock -= orderCount;
	this.stock = this.stock - orderCount;

[Team07-BE] 메인, 상세보기 구현 #36

Are you sure you want to change the base?

[Team07-BE] 메인, 상세보기 구현 #36

Uh oh!

Conversation

ghost commented Apr 26, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

메인, 상세보기 구현

구현 내용

TODO

Uh oh!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghost commented Apr 26, 2021 •

edited by ghost

Loading