Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5기 - 정의진] SpringBoot Part3 Weekly Mission 제출합니다. #969

Open
wants to merge 134 commits into
base: main
Choose a base branch
from

Conversation

uijin-j
Copy link

@uijin-j uijin-j commented Nov 5, 2023

📌 과제 설명

바우처 관리 어플리케이션

👩‍💻 요구 사항과 구현 내용

(기본) 바우처 서비스 관리페이지 개발하기

  • Spring MVC를 적용해서 thymeleaf 템플릿을 설정해보세요.
  • 커맨드로 지원했던 기능을 thymeleaf를 이용해서 관리페이지를 만들고 다음 기능을 지원가능하게 해보세요
    • 조회페이 GET(”/vouchers”)
    • 상세페이지 GET(”/vouchers/{voucherId}”)
    • 입력페이지 GET(”/vouchers/create”) → POST(”/vouchers”)
    • 삭제기능 DELETE(”/vouchers/{voucherId}”)

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

  • Spring MVC를 적용해서 JSON과 XML을 지원하는 REST API를 개발해보세요

    • 전체 조회기능
    • 조건별 조회기능 (바우처 생성기간 및 특정 할인타입별)
    • 바우처 추가기능
    • 바우처 삭제기능
    • 바우처 아이디로 조회 기능
  • (보너스) 바우처 지갑용 관리페이지를 만들어보세요.

✅ 피드백 반영사항

  • 아직 부족한 점이 많지만.. 객체지향 체조 원칙을 최대한 고려해서 코드를 작성해 보았습니다! (메서드 1depth 유지, 메서드 분리, else 지양)
  • Service 테스트 시 MemoryRepository를 사용하던 부분을 JdbcRepository를 사용하는 것으로 변경했습니다!
  • console app 부분을 제외하고 테스트 커버리지를 최대한 높일 수 있도록 테스트 코드를 작성하였습니다. (IOExcepion과 LocalDateTime과 같이 외부 영향이 있는 부분은 테스트 코드에서 일단 제외했습니다..) 추후 console 부분도 테스트 코드 추가해보겠습니다!!
스크린샷 2023-11-05 22 54 46

(참고) 지갑 서비스(console app, web)는 개발 도중 일정이 촉박해서 아직 미완성 단계입니다! 그래서 console app 런처에 지갑 서비스가 표시되지만 선택하면 '지갑 서비스는 준비 중입니다.'라는 문구가 나오도록 해놨습니다.

✅ 실행 방법

Docker MySQL DB 연결 방법

  1. Docker mysql 이미지 다운 (관련링크)
docker pull mysql
  1. mysql 컨테이너 실행
docker run --name uijin-assignment-mysql -p 3305:3306 -e MYSQL_ROOT_PASSWORD=uijin-mysql-pw -d mysql:latest --character-set-server=utf8mb4 --collation-server=utf8mb4_unicode_ci

❗3305번 포트를 혹시 사용 중이시라면, 사용 가능한 포트 번호로 변경해주세요!

  1. 테이블 생성
    src/main/resources/schema.sql 스트립트를 실행해서 테이블을 생성할 수 있습니다:)
    src/main/resources/data.sql 스트립트를 실행해서 customers 테이블에 테스트 데이터를 삽입할 수 있습니다:)

✅ PR 포인트 & 궁금한 점

  • 현재 Controller DTO가 있고, Service DTO가 따로 있습니다! 이렇게 한 이유가, 컨트롤러가 기존 controller, view, api 이렇게 다양하게 있어서 인데요! 막상 만들고 보니, 현재로선 모든 Controller가 같은 형태로 Service DTO를 사용하고 있어서 Service DTO가 굳이 없어도 되겠다라는 생각이 드네요.. DTO를 레이어별로 따로 두는 것에 대해 의견을 듣고 싶습니다!
  • 이번에 VoucherType이나 CommandType과 같은 enum을 만들면서 VoucherType별 검증 로직이 다르므로 이걸 enum에 넣어야 할지, 또는 이런 유효성 검사는 enum에 있는 것보다 도메인에 있는 것이 나을지 / CommandType 별로 호출되는 메서드를 switch 문으로 호출했는데, 이걸 enum으로 넣는게 좋을지 근데 그러면 CommandType이 ConsoleVoucherApplication의 메서드를 public static으로 열어야 하는데 과연 그게 맞는건지..와 같은 고민이 항상 따라오더라구요! enum에 enum값과 관련된 기능을 enum에 넣는게 적합한지, 아닌지를 판단할 수 있는 팁이 있을까요?
  • voucherType 옵션을 주어 voucher 리스트를 조회하는 기능을 구현할 때, controller에 새로운 메서드를 추가하는 대신 기존 findAllVouchers() 메서드에 @nullable @RequestParam VoucherType를 받아 voucherType이 존재하지 않으면 service의 getVouchers()를 호출하고, 존재한다면 getVouchersByVoucherType()을 호출하도록 했는데요! 이 방법의 단점이 if문이 생기고, 나중에 필터 조건이 늘어나면 메서드가 너무 길어진다는 것이입니다. 이부분은 어떻게 처리하는 것이 좋을까요? 필터링 검색 같은 경우에는 동적 쿼리를 만들어야 해서 QueryDsl을 쓰는 방법이 있을 것 같지만, 지금 상태에서 QueryDsl은 조금 과한 느낌이 있는 것 같아서요..!
  • thymeleaf을 사용하던 중 제가 html, css의 정말 기초만 알고 있는 것 같다는 생각이 들었습니다.. 백엔드 개발자가 알아야 하는 프론트 지식은 어느 정도 인지 궁금합니다..

uijin-j and others added 30 commits October 13, 2023 16:30
################
# <타입> : <제목> 의 형식으로 제목을 아래 공백줄에 작성
# 제목은 50자 이내 / 변경사항이 "무엇"인지 명확히 작성 / 끝에 마침표 금지
# 예) feat : 로그인 기능 추가

# 바로 아래 공백은 지우지 마세요 (제목과 본문의 분리를 위함)

################
# 본문(구체적인 내용)을 아랫줄에 작성
# 여러 줄의 메시지를 작성할 땐 "-"로 구분 (한 줄은 72자 이내)

################
# 꼬릿말(footer)을 아랫줄에 작성 (현재 커밋과 관련된 이슈 번호 추가 등)
# 예) Close prgrms-be-devcourse#7

################
# feat : 새로운 기능 추가
# fix : 버그 수정
# docs : 문서 수정
# test : 테스트 코드 추가
# refact : 코드 리팩토링
# style : 코드 의미에 영향을 주지 않는 변경사항
# chore : 빌드 부분 혹은 패키지 매니저 수정사항
################
################
# <타입> : <제목> 의 형식으로 제목을 아래 공백줄에 작성
# 제목은 50자 이내 / 변경사항이 "무엇"인지 명확히 작성 / 끝에 마침표 금지
# 예) feat : 로그인 기능 추가

# 바로 아래 공백은 지우지 마세요 (제목과 본문의 분리를 위함)

################
# 본문(구체적인 내용)을 아랫줄에 작성
# 여러 줄의 메시지를 작성할 땐 "-"로 구분 (한 줄은 72자 이내)

################
# 꼬릿말(footer)을 아랫줄에 작성 (현재 커밋과 관련된 이슈 번호 추가 등)
# 예) Close prgrms-be-devcourse#7

################
# feat : 새로운 기능 추가
# fix : 버그 수정
# docs : 문서 수정
# test : 테스트 코드 추가
# refact : 코드 리팩토링
# style : 코드 의미에 영향을 주지 않는 변경사항
# chore : 빌드 부분 혹은 패키지 매니저 수정사항
################
Voucher 필드 게터 추가
메서드 분리, 상수 분리
메서드명 리네임, 메서드 분리
기능 구현이 완료되면, 다시 서비스
Comment on lines +18 to +21
public JdbcTemplateConfig(@Value("${spring.datasource.username}") String username,
@Value("${spring.datasource.password}") String password,
@Value("${spring.datasource.driver-class-name}") String driverClassName,
@Value("${spring.datasource.url}") String url) {
Copy link

Choose a reason for hiding this comment

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

https://rutgo-letsgo.tistory.com/93
@ConfigurationProperties를 한번 알아보면 어떨까요?

Comment on lines +25 to +37
if(voucherType != null) {
List<VoucherResponse> vouchers = voucherService.getVouchersByVoucherType(voucherType).stream()
.map(VoucherResponse::of)
.toList();

return ApiResponse.ok(vouchers);
}

List<VoucherResponse> vouchers = voucherService.getVouchers().stream()
.map(VoucherResponse::of)
.toList();

return ApiResponse.ok(vouchers);
Copy link

Choose a reason for hiding this comment

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

https://medium.com/@miguel.duque7/how-to-use-spring-data-jpa-specifications-to-filter-sql-queries-with-join-tables-1e821178c76b

이글을 참고해서 한번 fitlering builder패턴? 느낌을 한번 알아보면 어떨까요?

public ApiResponse deleteVoucher(@PathVariable Long voucherId) {
voucherService.deleteVoucher(voucherId);

return ApiResponse.ok();
Copy link

Choose a reason for hiding this comment

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

204 no_content를 한번 알아보면 어떨까요?

}

@PostMapping
public String createVoucher(@RequestParam("voucherType") String voucherType,
Copy link

Choose a reason for hiding this comment

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

Suggested change
public String createVoucher(@RequestParam("voucherType") String voucherType,
public String createVoucher(@RequestParam String voucherType,

이거 생략해도 무관해요~

Comment on lines +80 to +86
private static boolean isNotPositive(int discountValue) {
return discountValue <= 0;
}

private static void validateDiscountValue(int discountValue) {
if (isNotPositive(discountValue)) throw new IllegalArgumentException("할인 금액 또는 할인율은 양수입니다.");
}
Copy link

Choose a reason for hiding this comment

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

validate의 역할이 controller 심히 고민 해보세요.

Comment on lines +92 to +98
private void getMyVouchers(UUID customerId) {
// TODO
}

private void registerVoucher(UUID customerId) {
// TODO
}
Copy link

Choose a reason for hiding this comment

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

구현 하시죠.

import java.util.NoSuchElementException;
import java.util.UUID;

@Controller
Copy link

Choose a reason for hiding this comment

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

@ControllerAdivce를 적용해보면 어떨까요?

<link rel='stylesheet' href='https://fonts.googleapis.com/css?family=Roboto'>
<script src='https://kit.fontawesome.com/a076d05399.js' crossorigin='anonymous'></script>
<style>
body {
Copy link

Choose a reason for hiding this comment

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

파일 분리 해보면 어떨까요?

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;

@SpringBootTest
Copy link

Choose a reason for hiding this comment

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

JdbcTest를 한번 알아보시면 좋을거 같습니다.

voucherRepository.save(voucher3);

// when
Optional<Long> latestId = voucherRepository.findLatestVoucherId();
Copy link

Choose a reason for hiding this comment

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

autoIncrement를 적용해보면 좋겠어요.

if (isNotPositive(discountValue)) throw new IllegalArgumentException("할인 금액 또는 할인율은 양수입니다.");
}

private static boolean isNotPositive(int discountValue) {

Choose a reason for hiding this comment

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

과도한 분리 같습니다. 그리고 0같은 magic number를 상수로 표현하면 더 좋을 것 같습니다.


import java.util.UUID;

public class CustomerIdAndName {

Choose a reason for hiding this comment

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

dto에서 반복되는 보일러 플레이트 코드들을 record를 이용하면 줄일 수 있을 것 같습니다.

Voucher voucher = voucherService.createVoucher(request.toServiceRequest());
VoucherResponse response = VoucherResponse.of(voucher);

return ApiResponse.ok(response);

Choose a reason for hiding this comment

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

201 created도 찾아보면 좋을 것 같습니다.

customer.getEmail(),
Timestamp.valueOf(customer.getCreatedAt()));

if(update != 1) {

Choose a reason for hiding this comment

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

1의 의미를 잘 표현할 수 있는 상수를 만들면 좋을 것 같습니다.

public Optional<Voucher> findById(Long voucherId) {
if(voucherMap.containsKey(voucherId)) {
return Optional.of(voucherMap.get(voucherId));
} else return Optional.empty();

Choose a reason for hiding this comment

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

else 없이 아래에서 return 하는게 더 이해하기 좋을 것 같습니다.


Customer customer = customerController.findCustomerByEmail(customerEmail);

if(customer == null) {

Choose a reason for hiding this comment

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

null인 경우가 있다면 Optional로 인지할 수 있게 구성하면 좋을 것 같습니다.

}

private void getMyVouchers(UUID customerId) {
// TODO

Choose a reason for hiding this comment

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

아직 구현되지 않았다면 exception으로 표현해두는 것도 좋을 것 같습니다.

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.

4 participants