Skip to content

Conversation

@parksey
Copy link

@parksey parksey commented Jul 15, 2023

📌 과제 설명
바우처 서비스 관리 페이지 및 API 개발

👩‍💻 요구 사항과 구현 내용
(기본) 바우처 서비스 관리페이지 개발하기

  • Spring MVC를 적용해서 thymeleaf 템플릿을 설정해보세요.
  • 커맨드로 지원했던 기능을 thymeleaf를 이용해서 관리페이지를 만들고 다음 기능을 지원가능하게 해보세요
    • 조회페이지
    • 상세페이지
    • 입력페이지
    • 삭제기능

(기본) 바우처 서비스의 API 개발하기

  • Spring MVC를 적용해서 JSON과 XML을 지원하는 REST API를 개발해보세요
    • 전체 조회기능
    • 조건별 조회기능 (바우처 생성기간 및 특정 할인타입별)
    • 바우처 추가기능
    • 바우처 삭제기능
    • 바우처 아이디로 조회 기능

추가

[ API 전용 예외 처리 추가 ]

  • 상태 코드와 메세지만 다시 반환할 수 있도록 변경

✅ 피드백 반영사항

  • whiel문의 not이 들어 가는 경우 가독성 저하 -> 변경하여 해결

  • 사용하지 않는 return값 리팩토링

  • Autowired에 대한 의미 설명

  • 오타 및 수정하지 않는 부분 리팩토링

  • 바우처 값 계산하는 메서드의 내용 설명 추가 (주석)

    • 바우처의 한도가 사용자의 금액보다 클 때, 바우처 사용시 남는 액수 계산 로직
    • 답변 완료
  • 테스트의 독립성을 유지하기 위해 Order삭제 및 테스트 코드 리팩토링

  • 생성자 내에서 validate를 할 수 있도록 리팩토링

  • 메서드 안의 많은 책임

    • 기존 많은 책임을 가지고 있는 문제 때문에 handle이라는 메서드명으로만 처리
    • [PR답변]
  • enum 클래스에 대한 if문을 switch로 변경하여 가독성을 증가

  • 모든 응답 객체를 Response라는 인터페이스로 동일한 동작을 하게 했지만, 오히려 요청시 혼란이 올 수 있기에 리팩토링

  • find transcation(readOnly=true)추가

  • 자신만의 컨벤션 지키면서 개발

  • ConditionalOnProperty 설명 추가 : [PR답변]

✅ PR 포인트 & 궁금한 점

[PR 포인트]

계속 지적 받던 클린코드 문제가 아직 많은 것 같습니다. 이 부분부터 고쳐보도록 노력하겠습니다.

Thymeleaf와 RequestDto

[질문]

  • Thymeleaf의 mapping만을 위한 기본 생성자를 생성하는 것이 괜찮은가요?

[현 상황]

// 패키지 구조
voucher/dto/request
   - VoucherCreationRequest.java
   - VoucherInfoRequest.java

여기서 VoucherCreationRequest클래스에서 내부적으로 DiscountType enum클래스VoucherInfoRequest 클래스를 가지고 있습니다.

// Thymeleaf form data

th:object="${voucherCreationRequest}"
 - th:field="*{voucherInfoRequest.amount}"
 - th:field="*{voucherInfoRequest.expiration}"
 - th:field="*{discountType}"

[상황 인식]
Thymeleaf에서 form-data를 통해 VoucherCreationRequest에 데이터를 바인딩을 해야 했습니다. 이때 VoucherInfoRequest에서는 전체 필드 값에 대한 생성자만 있었는데, 해당 생성자를 통한 바인딩이 안되어서
기본 생성자를 추가해 줘야 했습니다.

이렇게 사용하다 보니, 로직에서는 사용하지 않는 기본 생성자가 mapping을 위해 사용되게 되는데
이러면 로직에서 의도치 않은 접근이 허용돼서 잘못된 코드를 작성할 수 있겠다 라고 생각이 되었습니다.

따라서 위의 질문처럼 기본 생성자를 생성하는 것이 올바른 방향인지, 아님 설계를 변경해야 하는 것인지 궁금합니다!

*추가
docker폴더 하위에 docker-compose.yml을 넣었습니다!

parksey added 23 commits July 12, 2023 05:14
@parksey parksey changed the base branch from main to parksey July 15, 2023 04:36
@parksey parksey changed the title Parksey3 [4기 - 박세연] SpringBoot Part3 Weekly Mission 제출합니다. Jul 15, 2023
@parksey parksey requested a review from joseph-100 July 15, 2023 04:38
@parksey parksey self-assigned this Jul 15, 2023
Copy link

@joseph-100 joseph-100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

과제 하시느라 고생하셨습니다.

주차 별로 코드가 개선되는 느낌을 받아 좋습니다.
패키지 구성이 api, web, voucher, customer가 동등레벨로 구성되어 있는데 조금 애매한 느낌을 받습니다.
이 부분은 한번 더 고민 해주시고 PR 올려 주세요.

import java.util.List;

@Service
public class VoucherService {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TC 보강해 주세요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 보강하겠습니다

}

@PostMapping("/customer")
public ResponseEntity<CustomerResponse> create(@RequestBody CustomerCreationRequest creationRequest) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CustomerCreationRequest 유효성 체크는 필요 없을까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀듣고 Dto에 대해서도 유효성 검사가 필요할 것 같아 추가하였습니다.


private final String printMessage;

private final static Map<String, VoucherMenu> VOUCHER_MENU_MAP;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static을 final 키워드 전에 두세요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 3 to 4
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

미사용 import문은 꼭 지워주세요. 코드를 건든 곳은 습관적으로 최적화 해주면 좋습니다.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"10,1: 1"
}, delimiter = ':')
void 바우처가_이미_존재하면_예외발생(String userInput, String no) {
void 바우처가_이미_존재하면_예외발생(String userInput, String no) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

바우처가 이미 존재하면 예외발생에 대한 테스트가 맞나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

표현 자체가 적절하지 못했던 것 같습니다. 테스트 메서드 명을 변경하였습니다

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

실패하는 테스트가 있습니다.
사용하지 않는 코드는 정리해주세요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 테스트 전부 다시 확인해보겠습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants