[5기 - 이중원] SpringBoot Part3 Weekly Mission 제출합니다. by shoeone96 · Pull Request #970 · prgrms-be-devcourse/springboot-basic

shoeone96 · 2023-11-05T14:55:36Z

📌 과제 설명

기본 과제 구현 위주로 구성하였습니다.

Home 화면 -> 기본 경로로 들어갈 경우 보이는 페이지
nav바로 페이지를 오갈 수 있음

1. Vouchers 페이지
2. 모든 voucher 목록을 볼 수 있다.
3. voucherId를 클릭하면 voucher 관련 상세페이지를 볼 수 있다.

1. voucher 상세페이지
2. allocate의 link를 클릭하면 해당 voucher를 할당할 수 있는 페이지로 이동
3. delete를 클릭하면 삭제된다.
4. voucherOwner의 경우 주인이 없으면 빈 칸으로 나온다.

1. customerId를 입력하면 그 고객에게 voucher가 할당된다.

1. voucher를 새로 할당하는 페이지
2. voucher type을 지정한다.
3. discountValue를 넣고 버튼을 누르면 만들어진다.
4. redirect는 vouchers 목록 페이지로 간다.

1. 회원 모두를 보여주는 페이지
2. id 부분을 클릭해 상세페이지로 갈 수 있다.

회원 정보 상세페이지
회원의 정보를 보여준다.
회원이 가진 voucher의 정보를 보여준다.

회원 가입시키는 페이지

1. black list를 보여주는 페이지

사용설명을 추가하였습니다.

✅ 피드백 반영사항

일부 테스트 코드를 추가하였으나 모두 반영하지 못하여 다시 확인해보겠습니다!

✅ PR 포인트 & 궁금한 점

model에 추가하여 html상에서 사용하기 위해 추가, 필요에 사용하는 곳에서 기존 getter 메서드 변환

기존 3306 포트는 mysql, 3000번 포트는 react default 포트라 사용하지 않는 3300포트로 변경

nav 바 변경, ui 개선

double brace 사용 시 내부적으로 inner class를 활용하여 발생하는 메모리 누수 문제 (외부 클래스 인스턴스를 지속적으로 참조해서 사라지지 않는 문제)

twotwobread · 2023-11-09T12:04:12Z

src/main/java/org/programmers/springorder/customer/controller/CustomerPageController.java

+        return "customers";
+    }
+
+    @GetMapping("/blackCustomers")


/customer path에 query string으로 표현해보는 건 어떨까요?

twotwobread · 2023-11-09T12:06:39Z

src/main/java/org/programmers/springorder/customer/controller/CustomerPageController.java

+        this.voucherService = voucherService;
+    }
+
+    @GetMapping("/customers")


/customers를 prefix 형태로 설정하면 좋을 것 같습니다.

twotwobread · 2023-11-09T12:09:47Z

src/main/java/org/programmers/springorder/customer/repository/FileCustomerRepository.java

                String name = data[1];
                CustomerType customerType = CustomerType.valueOf(data[2]);
-                Customer customer = Customer.toCustomer(customerId, name, customerType);
+                LocalDateTime createdAt = LocalDateTime.parse(data[3]);


3이라는 인덱스 이용하여 parsing하는 이유를 상수로 표현하면 좋을 것 같습니다.

twotwobread · 2023-11-09T12:26:22Z

src/main/java/org/programmers/springorder/voucher/service/VoucherService.java

+    public List<VoucherResponseDto> getCustomerOwnedVouchers(UUID customerId) {
        Customer customer = customerRepository.findByID(customerId)
-                .orElseThrow( () -> new RuntimeException("해당 고객을 찾을 수 없습니다."));
+                .orElseThrow(() -> new VoucherException(ErrorCode.CUSTOMER_NOT_FOUND));


RuntimeException 대신에 전부 다 VoucherException으로 표현한다면 세부적인 Exception을 통한 가독성의 향상이나 exception을 세부적으로 handling하는 그런 역할을 하기 어려울 것 같습니다. exception을 통해서 어떤 예외상황인지를 알 수 있는 세부 exception을 이용하면 좋을 것 같습니다.

twotwobread · 2023-11-09T12:30:06Z

src/main/java/org/programmers/springorder/customer/model/Customer.java

+    private final LocalDateTime createdAt;
+    private final LocalDateTime updatedAt;

    private Customer(UUID customerId, String name, CustomerType customerType) {


this(...) 이용하면 중복 코드를 줄일 수 있을 것 같습니다.

twotwobread · 2023-11-09T12:33:19Z

src/main/java/org/programmers/springorder/exception/ErrorCode.java

+    INVALID_DISCOUNT_VALUE(HttpStatus.BAD_REQUEST, "잘못된 고정 할인 금액입니다."),
+    INVALID_DISCOUNT_PERCENT(HttpStatus.BAD_REQUEST, "잘못된 할인율입니다."),
+
+    INVALID_INPUT(HttpStatus.BAD_REQUEST, "잘못된 입력입니다.") ;


개행이 요기 아래에 있어야 할 것 같습니다.

twotwobread · 2023-11-09T12:37:07Z

src/main/java/org/programmers/springorder/voucher/controller/VoucherRestController.java

+        this.voucherService = voucherService;
+    }
+
+    @GetMapping("api/v1/vouchers")


중복되는 path를 prefix로 빼는 것도 좋을 것 같습니다.

ksy90101 · 2023-11-19T04:02:00Z

mysql/init.sql

                           customer_id BINARY(16),
                           customer_name VARCHAR(30),
                           customer_type VARCHAR(30),
+                           created_at datetime(6) not null ,


Suggested change

created_at datetime(6) not null ,

created_at datetime(6) NOT NULL,

통일성을 맞춰서 예약어는 대문자로 쓰시는게 어떨까요?

ksy90101 · 2023-11-19T04:02:53Z

src/main/java/org/programmers/springorder/customer/controller/CustomerPageController.java

+    }
+
+    @GetMapping("/customers")
+    public String getCustomerListPage(Model model){


query String으로 필터해서 blackcustomers까지 하나의 컨트롤러에서 처리하는게 훨씬 좋을거 같습니다.

ksy90101 · 2023-11-19T04:03:52Z

src/main/java/org/programmers/springorder/customer/controller/CustomerPageController.java

+    @GetMapping("/new-customer")
+    public String getNewCustomerPage(){
+        return "new-customer";
+    }


Suggested change

@GetMapping("/new-customer")

public String getNewCustomerPage(){

return "new-customer";

}

@GetMapping("/customers/new")

public String getNewCustomerPage(){

return "new-customer";

}

ksy90101 · 2023-11-19T04:04:52Z

src/main/java/org/programmers/springorder/customer/controller/CustomerRestController.java

+        this.customerService = customerService;
+    }
+
+    @GetMapping("api/v1/customers")


@ReqeustMapping에 api/v1/customers를 동일하게 넣어두면 좋을거 같습니다.

ksy90101 · 2023-11-19T04:05:18Z

src/main/java/org/programmers/springorder/customer/controller/CustomerRestController.java

+    }
+
+    @GetMapping("api/v1/customers")
+    public Response<List<CustomerResponseDto>> getCustomerListPage(){


blackcustomers를 쿼리스토링으로 필터링 하는게 더 좋을거 같아요.

ksy90101 · 2023-11-19T04:05:44Z

src/main/java/org/programmers/springorder/customer/controller/CustomerRestController.java

+    @PostMapping("api/v1/customers")
+    public Response<Void> enrollCustomer(CustomerRequestDto requestDto){
+        customerService.newCustomer(requestDto);
+        return Response.success();


status 204 no_content 을 쓰는게 더 좋을거 같습니다.

ksy90101 · 2023-11-19T04:06:33Z

src/main/java/org/programmers/springorder/customer/dto/CustomerRequestDto.java

+package org.programmers.springorder.customer.dto;
+
+import org.programmers.springorder.customer.model.CustomerType;
+import org.springframework.lang.NonNull;


validatiton으로 하면 좋을거 같습니다.

ksy90101 · 2023-11-19T04:06:40Z

src/main/java/org/programmers/springorder/customer/dto/CustomerRequestDto.java

+        @NonNull String name,
+        @NonNull CustomerType customerType) {


빈값이 들어와도 괜찮을까요?

ksy90101 · 2023-11-19T04:07:49Z

src/main/java/org/programmers/springorder/customer/model/Customer.java

 import java.util.Objects;
 import java.util.UUID;

 public class Customer {


도메인도 validation이 필요 할거 같습니다.

ksy90101 · 2023-11-19T04:08:24Z

src/main/java/org/programmers/springorder/customer/repository/JdbcCustomerRepository.java

-            put("customerName", customer.getName());
-            put("customerType", customer.getCustomerType().name());
-        }};
+        Map<String, Object> map = new HashMap<>();


map이라는 단어보단 더 의미있는 단어
params
같은 단어요!

ksy90101 · 2023-11-19T04:09:20Z

src/main/java/org/programmers/springorder/customer/service/CustomerService.java

+                .toList();
+    }
+
+    public void newCustomer(CustomerRequestDto requestDto) {


create와 같은 동사를 쓰면 어떨까요?

ksy90101 · 2023-11-19T04:10:32Z

src/main/java/org/programmers/springorder/exception/PageExceptionHandler.java

+    @ExceptionHandler(VoucherException.class)
+    public String applicationHandler(VoucherException e){
+        return "no-voucher";
+    }
+
+    @ExceptionHandler(RuntimeException.class)
+    public String applicationHandler(RuntimeException e){
+        return "no-voucher";
+    }


status를 의미있게 내랴보면 어떨까요?

ksy90101 · 2023-11-19T04:18:51Z

src/main/java/org/programmers/springorder/voucher/controller/VoucherPageController.java

+        try{
+            model.addAttribute("voucherId", voucherId);
+            return "voucher-allocate";
+        }catch (RuntimeException e){


controllaAdivce에서 이미 다 하고 있지 않나요?

ksy90101 · 2023-11-19T04:22:15Z

src/main/java/org/programmers/springorder/customer/service/CustomerService.java

@@ -29,12 +33,33 @@ public List<CustomerResponseDto> getBlackList() {

    public CustomerResponseDto findOwnerOfVoucher(UUID voucherId){


@Tansactional 이 없어도 괜찮을까요?

ksy90101 · 2023-11-19T04:24:42Z

src/main/java/org/programmers/springorder/voucher/dto/VoucherRequestDto.java

+        if (voucherType == VoucherType.FIXED && (discountValue < 100 || discountValue > 10000)) {
+            throw new VoucherException(ErrorCode.INVALID_DISCOUNT_VALUE);
+        }
+        if (voucherType == VoucherType.PERCENT && (discountValue < 3 || discountValue > 50)) {


상수화를 해보면 어떨까요?

ksy90101 · 2023-11-19T04:25:03Z

src/main/java/org/programmers/springorder/voucher/repository/FileVoucherRepository.java

+                LocalDateTime createdAt  = LocalDateTime.parse(data[3]);
+                LocalDateTime updatedAt  = LocalDateTime.parse(data[4]);
+                Voucher voucher = Voucher.getFromDbVoucherNoOwner(voucherId, discountValue, voucherType, createdAt, updatedAt);
+                if (data.length == 6) {


6도 상수화 해야 할거 같아요!

ksy90101 · 2023-11-19T04:30:26Z

src/test/java/org/programmers/springorder/utils/MenuTypeTest.java

+
+    @Test
+    @DisplayName("올바른 input menu를 넣을 시에 선택한 menu를 반환한다.")
+    public void inputMenuCorrectTest(){


@ParamterziedTest를 쓰면 좋을꺼 같습니다!

shoeone96 added 30 commits November 1, 2023 20:16

chore: 로그설정 변경

2cfd990

build: thymeleaf 의존성 추가

ab537b1

feat: voucher 받는 페이지 전송 컨트롤러 추가

143ea86

refactor: voucherResponseDto record로 변환

f54ae10

model에 추가하여 html상에서 사용하기 위해 추가, 필요에 사용하는 곳에서 기존 getter 메서드 변환

feat: 모든 voucher를 불러오는 기본 페이지 작성

50e516c

chore: docker 포트 변경

f84a2ee

기존 3306 포트는 mysql, 3000번 포트는 react default 포트라 사용하지 않는 3300포트로 변경

feat: voucherconsoler과 web 버전 분리

90d64ae

refactor: requestDto를 class -> record로 변경

2f5e047

chore: console로 진행하는 class 명 변경

888f096

chore: 스프링 구동 로그 제거

b106396

feat: 새로운 voucher 페이지로 넘어가는 url 생성

656ac05

feat: 새로운 voucher를 등록하는 api 생성

e3f8c4d

feat: 새로운 voucher를 생성하는 페이지 작성

fdab3d6

refactor: ui refactoring

a06c20a

nav 바 변경, ui 개선

chore: 사용하지 않는 import 문 제거

a903890

chore: voucherService 메서드 명 변경

06b430c

feat: voucher 상세페이지 조회 api 추가

f0661d5

refactor: voucherResponseDto로 변환하여 model 전달

0fa0927

feat: voucher 상세 조회 서비스 로직 작성

9a3745e

refactor: voucher id를 누르면 상세페이지 갈 수 있게 변경

8c9d938

feat: voucher 상세페이지 작성

c1b4fc7

feat: voucher 삭제 api 추가

acab01c

feat: voucher 상세페이지 삭제 기능

574c0b9

feat: voucher 삭제 api 추가

a690cd9

refactor: double brace를 이용한 HashMap 초기화 부분 변경

c47a0fd

double brace 사용 시 내부적으로 inner class를 활용하여 발생하는 메모리 누수 문제 (외부 클래스 인스턴스를 지속적으로 참조해서 사라지지 않는 문제)

feat: 이미 삭제되었거나 존재하지 않는 id의 voucher 검색 시 보여주는 페이지 작성

351a4b2

feat: customer 목록을 불러오는 페이지 api 작성

6289fe6

feat: customer 목록을 불러오는 서비스 로직 작성

5aa2bf2

feat: customer 목록을 불러오는 페이지 작성, Customer 정보 페이지에 넣기 위한 getter 추가

d48a364

feat: 새로운 고객 생성 페이지 이동 항목 추가

b3223fd

shoeone96 added 12 commits November 8, 2023 16:03

chore: csv 파일 gitignore에 추가

f15bc48

feat: requestParam 이 있는 경우 그 시간에 맞춰서 조회하는 rest API 메서드 추가

20dd088

feat: 조회 시작 시간과 종료 시간을 포함해서 voucher를 조회하는 로직 추가

719fb6f

feat: 조회 시작 시간과 종료 시간을 포함해서 voucher를 조회하는 jdbc로직 추가

81c405f

feat: fileRepository와 MemoryRepository에서 시간기한 별로 조회하는 메서드 추가

0cee509

chore: naming convention 지키기

b46dce4

feat: voucher 등록 시 필요한 validation 작성

efe9095

feat: voucher 등록 시 발생할 수 있는 enum 작성

a335184

feat: voucher 등록 시 타입이 잘못 기록된 경우 처리해줄 수 있는 예외 처리

ecdea71

test: voucher 등록 시 발생하는 예외 테스트

9b2e795

refactor: VouchergiveRequestDto -> record변경 및 notnull 유효성 검증 추가

a232c8a

feat: customer 등록 시 nonnull 조건 추가

1e27ba4

twotwobread reviewed Nov 9, 2023

View reviewed changes

ksy90101 reviewed Nov 19, 2023

View reviewed changes

	created_at datetime(6) not null ,
	created_at datetime(6) NOT NULL,

		@@ -29,12 +33,33 @@ public List<CustomerResponseDto> getBlackList() {

		public CustomerResponseDto findOwnerOfVoucher(UUID voucherId){

Conversation

shoeone96 commented Nov 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 과제 설명

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

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!

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!

3 participants

shoeone96 commented Nov 5, 2023 •

edited

Loading