[5기 - 이중원] SpringBoot Part3 Weekly Mission 제출합니다. #970
Conversation
model에 추가하여 html상에서 사용하기 위해 추가, 필요에 사용하는 곳에서 기존 getter 메서드 변환
기존 3306 포트는 mysql, 3000번 포트는 react default 포트라 사용하지 않는 3300포트로 변경
nav 바 변경, ui 개선
double brace 사용 시 내부적으로 inner class를 활용하여 발생하는 메모리 누수 문제 (외부 클래스 인스턴스를 지속적으로 참조해서 사라지지 않는 문제)
| return "customers"; | ||
| } | ||
|
|
||
| @GetMapping("/blackCustomers") |
There was a problem hiding this comment.
/customer path에 query string으로 표현해보는 건 어떨까요?
| this.voucherService = voucherService; | ||
| } | ||
|
|
||
| @GetMapping("/customers") |
There was a problem hiding this comment.
/customers를 prefix 형태로 설정하면 좋을 것 같습니다.
| @@ -41,7 +42,9 @@ public List<Customer> findAll() { | |||
| UUID customerId = UUID.fromString(data[0]); | |||
| String name = data[1]; | |||
| CustomerType customerType = CustomerType.valueOf(data[2]); | |||
| Customer customer = Customer.toCustomer(customerId, name, customerType); | |||
| LocalDateTime createdAt = LocalDateTime.parse(data[3]); | |||
There was a problem hiding this comment.
3이라는 인덱스 이용하여 parsing하는 이유를 상수로 표현하면 좋을 것 같습니다.
| Customer customer = customerRepository.findByID(customerId) | ||
| .orElseThrow( () -> new RuntimeException("해당 고객을 찾을 수 없습니다.")); | ||
| .orElseThrow(() -> new VoucherException(ErrorCode.CUSTOMER_NOT_FOUND)); |
There was a problem hiding this comment.
RuntimeException 대신에 전부 다 VoucherException으로 표현한다면 세부적인 Exception을 통한 가독성의 향상이나 exception을 세부적으로 handling하는 그런 역할을 하기 어려울 것 같습니다. exception을 통해서 어떤 예외상황인지를 알 수 있는 세부 exception을 이용하면 좋을 것 같습니다.
| import java.util.Objects; | ||
| import java.util.UUID; | ||
|
|
||
| public class Customer { | ||
| private final UUID customerId; | ||
| private final String name; | ||
| private final CustomerType customerType; | ||
| private final LocalDateTime createdAt; | ||
| private final LocalDateTime updatedAt; | ||
|
|
||
| private Customer(UUID customerId, String name, CustomerType customerType) { |
There was a problem hiding this comment.
this(...) 이용하면 중복 코드를 줄일 수 있을 것 같습니다.
| INVALID_DISCOUNT_VALUE(HttpStatus.BAD_REQUEST, "잘못된 고정 할인 금액입니다."), | ||
| INVALID_DISCOUNT_PERCENT(HttpStatus.BAD_REQUEST, "잘못된 할인율입니다."), | ||
|
|
||
| INVALID_INPUT(HttpStatus.BAD_REQUEST, "잘못된 입력입니다.") ; |
| this.voucherService = voucherService; | ||
| } | ||
|
|
||
| @GetMapping("api/v1/vouchers") |
There was a problem hiding this comment.
중복되는 path를 prefix로 빼는 것도 좋을 것 같습니다.
| @@ -6,6 +6,8 @@ CREATE TABLE customers ( | |||
| customer_id BINARY(16), | |||
| customer_name VARCHAR(30), | |||
| customer_type VARCHAR(30), | |||
| created_at datetime(6) not null , | |||
There was a problem hiding this comment.
| created_at datetime(6) not null , | |
| created_at datetime(6) NOT NULL, |
통일성을 맞춰서 예약어는 대문자로 쓰시는게 어떨까요?
| } | ||
|
|
||
| @GetMapping("/customers") | ||
| public String getCustomerListPage(Model model){ |
There was a problem hiding this comment.
query String으로 필터해서 blackcustomers까지 하나의 컨트롤러에서 처리하는게 훨씬 좋을거 같습니다.
| @GetMapping("/new-customer") | ||
| public String getNewCustomerPage(){ | ||
| return "new-customer"; | ||
| } |
There was a problem hiding this comment.
| @GetMapping("/new-customer") | |
| public String getNewCustomerPage(){ | |
| return "new-customer"; | |
| } | |
| @GetMapping("/customers/new") | |
| public String getNewCustomerPage(){ | |
| return "new-customer"; | |
| } |
| this.customerService = customerService; | ||
| } | ||
|
|
||
| @GetMapping("api/v1/customers") |
There was a problem hiding this comment.
@ReqeustMapping에 api/v1/customers를 동일하게 넣어두면 좋을거 같습니다.
| } | ||
|
|
||
| @GetMapping("api/v1/customers") | ||
| public Response<List<CustomerResponseDto>> getCustomerListPage(){ |
There was a problem hiding this comment.
blackcustomers를 쿼리스토링으로 필터링 하는게 더 좋을거 같아요.
| @PostMapping("api/v1/customers") | ||
| public Response<Void> enrollCustomer(CustomerRequestDto requestDto){ | ||
| customerService.newCustomer(requestDto); | ||
| return Response.success(); |
There was a problem hiding this comment.
status 204 no_content 을 쓰는게 더 좋을거 같습니다.
| package org.programmers.springorder.customer.dto; | ||
|
|
||
| import org.programmers.springorder.customer.model.CustomerType; | ||
| import org.springframework.lang.NonNull; |
There was a problem hiding this comment.
validatiton으로 하면 좋을거 같습니다.
| @NonNull String name, | ||
| @NonNull CustomerType customerType) { |
| @@ -1,23 +1,40 @@ | |||
| package org.programmers.springorder.customer.model; | |||
|
|
|||
| import java.time.LocalDateTime; | |||
| import java.util.Objects; | |||
| import java.util.UUID; | |||
|
|
|||
| public class Customer { | |||
There was a problem hiding this comment.
도메인도 validation이 필요 할거 같습니다.
| put("customerName", customer.getName()); | ||
| put("customerType", customer.getCustomerType().name()); | ||
| }}; | ||
| Map<String, Object> map = new HashMap<>(); |
There was a problem hiding this comment.
map이라는 단어보단 더 의미있는 단어
params
같은 단어요!
| .toList(); | ||
| } | ||
|
|
||
| public void newCustomer(CustomerRequestDto requestDto) { |
| @ExceptionHandler(VoucherException.class) | ||
| public String applicationHandler(VoucherException e){ | ||
| return "no-voucher"; | ||
| } | ||
|
|
||
| @ExceptionHandler(RuntimeException.class) | ||
| public String applicationHandler(RuntimeException e){ | ||
| return "no-voucher"; | ||
| } |
| try{ | ||
| model.addAttribute("voucherId", voucherId); | ||
| return "voucher-allocate"; | ||
| }catch (RuntimeException e){ |
There was a problem hiding this comment.
controllaAdivce에서 이미 다 하고 있지 않나요?
| @@ -29,12 +33,33 @@ public List<CustomerResponseDto> getBlackList() { | |||
|
|
|||
| public CustomerResponseDto findOwnerOfVoucher(UUID voucherId){ | |||
There was a problem hiding this comment.
@Tansactional 이 없어도 괜찮을까요?
| if (voucherType == VoucherType.FIXED && (discountValue < 100 || discountValue > 10000)) { | ||
| throw new VoucherException(ErrorCode.INVALID_DISCOUNT_VALUE); | ||
| } | ||
| if (voucherType == VoucherType.PERCENT && (discountValue < 3 || discountValue > 50)) { |
| 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) { |
|
|
||
| @Test | ||
| @DisplayName("올바른 input menu를 넣을 시에 선택한 menu를 반환한다.") | ||
| public void inputMenuCorrectTest(){ |
There was a problem hiding this comment.
@ParamterziedTest를 쓰면 좋을꺼 같습니다!
📌 과제 설명
기본 과제 구현 위주로 구성하였습니다.
2. 모든 voucher 목록을 볼 수 있다.
3. voucherId를 클릭하면 voucher 관련 상세페이지를 볼 수 있다.
2. allocate의 link를 클릭하면 해당 voucher를 할당할 수 있는 페이지로 이동
3. delete를 클릭하면 삭제된다.
4. voucherOwner의 경우 주인이 없으면 빈 칸으로 나온다.
2. voucher type을 지정한다.
3. discountValue를 넣고 버튼을 누르면 만들어진다.
4. redirect는 vouchers 목록 페이지로 간다.
2. id 부분을 클릭해 상세페이지로 갈 수 있다.
사용설명을 추가하였습니다.
✅ 피드백 반영사항
일부 테스트 코드를 추가하였으나 모두 반영하지 못하여 다시 확인해보겠습니다!
✅ PR 포인트 & 궁금한 점