[2기-유민환] SpringBoot Part1 Weekly Mission 제출합니다. #186

yuminhwan · 2022-04-10T13:21:18Z

📌 과제 설명

바우처 관리 애플리케이션
- 4가지 커맨드(exit,create,list,blacklist)를 지원합니다.
- profile을 통해 기본적으로 Voucher가 파일로 관리되도록 하였습니다.
- YAML 프로퍼티를 사용하여 profile설정과 파일 경로 설정을 하였습니다.
- 로그를 기록하고 에러는 파일로 기록되도록 하였습니다.
- jar로도 실행가능하도록 하였습니다.

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

Console

입출력을 담당합니다.

AppConfiguration

YAML 프로퍼티를 통해 파일의 경로를 입력받아 Resource를 Bean으로 등록합니다. (블랙 리스트 파일만 해당합니다.)

VoucherProgram

전체적인 흐름을 제어합니다.
커맨드를 입력받습니다. (exit,create,list,blacklist)
만약 입력값이 잘못되었다면 다시 입력할 수 있습니다.
exit요청이 들어온 경우 프로그램을 종료합니다.
list요청이 들어온 경우 voucherService에게 저장되어 있는 voucher 정보를 요청합니다.
- 만약 파일이 잘못된 경우 예외처리를 통해 종료되게 됩니다.
create요청이 들어온 경우 voucher종류와 할인값을 입력받습니다.
- 이후 voucherService에게 voucher 생성을 요청합니다.
blacklist요청이 들어온 경우 userService에게 저장된 블랙 리스트 정보를 요청합니다.

📁 entity

User
Voucher - Interface
- FixedAmountVoucher, PercentDiscountVoucher - Voucher 구현체
  - 할인값이 잘못되었다면 예외를 발생합니다.
VoucherType
- voucher 종류 enum 입니다.
MenuType
- menu command 종류 enum입니다.

📁 exception

발생할 수 있는 예외를 재정의한 checked Exception입니다.

📁 repository

UserRepository - Interface
- FileUserRepository
  - Resource를 주입받습니다.
  - 주입받은 Resource를 통해 블랙리스트를 List형으로 가져옵니다.
VoucherRepository - Interface
- MemoryVoucherRepository
  - Map을 통해 Voucher를 저장하고 조회합니다.
- FileVoucherRepository
  - 파일을 통해 Voucher를 저장하고 조회합니다.
  - 직렬화를 통해 구현하였습니다.

📁 service

UserService
- UserRepository에게 블랙 리스트를 요청합니다.
VoucherService
- 입력받은 값에 맞게 Voucher를 생성하고 VoucherRepository에게 저장을 요청합니다.
- VoucherRepository에게 모든 Voucher정보를 요청합니다.

✅ PR 포인트 & 궁금한 점

이번에는 checked exception을 사용하기 위해 재정의하여 만들어 보았는 데 꼭 해주는 것이 좋을까요?
- unchecked exception을 사용할 때보다 exception을 처리해야한다는 것이 명시적으로 보였지만 어차피 한 곳에서 try-catch를 해주어서 굳이 사용해야 할까라는 생각이 듭니다.
- 상황에 맞게 사용하면 될까요?
FileVoucherRepository의 경우 Exception을 throws로 처리하게 되면 메서드 시그니처가 변해 try-catch를 통해 unchecked exception으로 변환해주었습니다.
- 이러한 방법이 잘못된 방법일까요?
logger를 처음 사용해봤는 데 제대로 사용한 지 모르겠습니다.
- 어느 형식으로 로그 메시지를 찍고 어느정도 선까지 로그를 생성해야 하는 지 기준이 안 잡힙니다.😂
현재는 블랙리스트 파일만 Resource로 가져왔습니다. Voucher파일 Resource로 가져왔을 때 IDE상에서는 정상적이였지만 jar 실행에서는 에러가 발생합니다.
- 이 경우 굳이 Resource를 통해 가져올 필요가 없을까요? 혹은 다른 방식이 있을까요?

테스트에서 아래와 같이 사용해보았는 데 저렇게 하는 것이 Mock객체를 사용하는 방법이 맞나요?

UserService userService = new UserService(() -> Arrays.asList(new User(UUID.randomUUID(), "jin"),
        new User(UUID.randomUUID(), "hwan"),
        new User(UUID.randomUUID(), "pobi")));

검증에 대한 기준이 아직 잡히지 않는 것 같습니다.
- InputView에게 숫자 검증 책임을 주고 각 숫자의 유효성 검증은 각 도메인에게 책임을 주고 예외 처리는 VoucherProgram에게 책임을 주었는 데 알맞은 방식일까요?

bosuksh

안녕하세요 민환님
과제하느라 수고 많으셨어요.
코드도 굉장히 깔끔하고 리뷰할 점이 그렇게 많지는 않았던 것 같네요~!
몇개 코멘트 달았으니 확인해주세요

질문에 대해서 답변을 드리자면

checked Exception을 모두 만들어주면 좋겠지만, 사실 그럴 상황이 되지 못할 경우가 더 많습니다. 그렇기에 꼭 필요한 Exception 위주로 먼저 해주고, 한꺼번에 처리할 수 있는 Exception을 만들어서 한꺼번에 처리할 수 있는건 처리해주는것도 좋은 방법 일 것 같네요. 예를 들어 입력이 잘 못되었을 경우 사용하는 Exception을 만들어서 여기저기서 사용하는 거죠
Exception을 적절한 변환을 통해 처리하는 방법은 충분히 가능한 방법이라고 생각합니다~!
logger에 경우에는 아래에도 코멘트를 남겼지만, 에러 로그라고 해서 모두 찍어줄 필요는 없고, 에러가 발생했을 때 크리티컬한 부분 위주로 찍어주는게 좋을 듯 싶어요! 예를 들어, 레포지토리에서 데이터를 못가져온 경우같은 곳이요! 너무 많은 로깅은 성능 저하를 일으킬 수 있거든요~
Resource로 관리하지 않고 사용하는 FileUserRepository에서 @value 를 이용해서 주입받아도 저는 크게 상관이 없다고 생각하네요! 처음에는 최대한 단순하게 만드는걸 선호해서요
Mock하는 경우는 주로 mocking 프레임워크인 mockito 같은 것을 사용합니다. Mockig 프레임워크를 사용하면 간단하게 input과 output을 설정할 수 있어요. Mocking 프레임워크에 대한건 아래 링크를 걸어놨으니 확인해주세요
검증에 대한 책임은 본인이 설정하기 나름이지만 민환님이 설정하신 검증에 대한 책임은 좋은것 같아요

bosuksh · 2022-04-11T02:29:30Z

src/main/resources/logback-spring.xml

+    </appender>
+
+    <root level="info">
+        <appender-ref ref="CONSOLE"/>


log level info를 콘솔로 해두다보니 console프로그램에서 로그도 같이 찍히는 현상이 발생하네요~
이부분은 콘솔프로그램에서 영향이 가니 피하는게 좋겠죠?

요구사항만 생각하다보니 콘솔에 영향을 가는 것을 생각을 못했네요 😂

bosuksh · 2022-04-11T06:58:36Z

src/main/java/org/prgms/voucherProgram/repository/user/FileUserRepository.java

+    public List<User> findBlackUsers() {
+        List<User> users = new ArrayList<>();
+        try (BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(userResource.getInputStream()))) {
+            addBlackUsers(users, bufferedReader);
+        } catch (IOException e) {
+            throw new IllegalArgumentException(ERROR_WRONG_FILE);
+        }
+        logger.info("BlackUsers read at File => {}", users);
+        return users;
+    }


이부분은 블랙리스트에 추가하는 메소드가 있으면 이렇게 하는게 맞다고 생각하는데
블랙리스트에 추가하는 메소드가 없기 때문에 인스턴스 생성시 미리 가져오는걸 생각해봐도 좋을것 같습니다~!

어차피 정적인 데이터니깐 캐싱(?)을 사용해서 한번만 가져오도록 하면 좋겠네요!

bosuksh · 2022-04-11T07:00:23Z

src/main/java/org/prgms/voucherProgram/repository/voucher/FileVoucherRepository.java

+    private void saveVoucherAtFile(Voucher voucher) {
+        try (ObjectOutputStream objectOutputStream = new ObjectOutputStream(new FileOutputStream(FILE_NAME, true))) {
+            objectOutputStream.writeObject(voucher);
+        } catch (IOException e) {
+            throw new IllegalArgumentException(ERROR_WRONG_FILE);
+        }
+    }


save에서 같은 역할을 해주기 때문에 save안에서 구현해도 될 것 같네요~

클린 코드를 한다고 기계적으로 메서드 추출을 하다보니 중복되게 만든 것 같습니다 😂

bosuksh · 2022-04-11T07:24:21Z

src/test/java/org/prgms/voucherProgram/service/VoucherServiceTest.java

+class VoucherServiceTest {
+
+    VoucherService voucherService = new VoucherService(new VoucherRepository() {
+        @Override
+        public Voucher save(Voucher voucher) {
+            return voucher;
+        }
+
+        @Override
+        public List<Voucher> findAll() {
+            try {
+                return Arrays.asList(new FixedAmountVoucher(UUID.randomUUID(), 10L),
+                    new PercentDiscountVoucher(UUID.randomUUID(), 20L));
+            } catch (Exception ignored) {
+            }
+            return null;
+        }
+    });


이부분은 mock 프레임워크를 이용해서 구현해보면 좋을 것 같네요
그 예로 Mockito를 사용하면 클래스를 구현하지 않고 바로 값을 넣을수 있습니다.
https://lemontia.tistory.com/915

어제 강의를 통해 Mock에 대해 배울 수 있었습니다.
제가 사용한 방식은 Stub이더군요
Mockito에 대한 내용 감사합니다!

bosuksh · 2022-04-11T07:25:31Z

src/test/java/org/prgms/voucherProgram/repository/voucher/FileVoucherRepositoryTest.java

+import org.prgms.voucherProgram.exception.WrongDiscountAmountException;
+import org.prgms.voucherProgram.exception.WrongDiscountPercentException;
+
+class FileVoucherRepositoryTest {


바우처를 가져오지 못한 테스트케이스도 추가하면 좋겠네요

테스트 커버리지를 높이도록 노력해야겠습니다..

bosuksh · 2022-04-11T07:42:21Z

src/main/java/org/prgms/voucherProgram/VoucherProgram.java

+import org.springframework.stereotype.Component;
+
+@Component
+public class VoucherProgram {


logging을 찍는 이유는, 어떤 에러를 트레이싱하기 위해서 찍는 경우가 많은데요.
이 클래스에서 찍는 에러는 대부분 사용자가 잘못입력했을 때, 즉 시스템 말고 사용자의 문제로 인해서 exception이 발생한 경우이고 그 에러에 대해서는 화면으로 출력을 해주기 때문에 사용자가 인지하고 수정할 것입니다.
그렇기 때문에 이부분의 데이터를 모아서 어떤 통계정 사용이든, 다른 용도로 사용할게 아니라면 굳이 찍을 필요는 없을 것 같아요
로그도 결국 IO를 하는 과정이기 때문에 너무 많은 로깅은 성능의 저하를 일으킬 수 있습니다.

의도한 에러는 로그를 찍을 필요가 없고 의도치 않은 에러는 로그로 관리해야 한다는 뜻으로 받아드리면 될까요?

yuminhwan · 2022-04-12T04:49:55Z

안녕하세요 아만드님!
이번에도 좋은 리뷰 남겨주셔서 감사합니다!! 피드백 기반으로 리팩토링을 해보았습니다.
부족한 점이나 더 고쳐야할 점을 말씀해주시면 감사하겠습니다!!!
추가적으로 몇가지 질문이 있습니다.

질문

말씀하신대로 @value를 통해 파일 경로를 가져오도록 하였습니다. jar파일에서도 읽을 수 있게 ClassPathResource로 읽도록 하였는 데 올바른 방식인가요?
블랙 리스트를 인스턴스 생성시 미리 가져오게 하기 위해 생성자에서 읽어오도록 구현하였습니다. 이렇게 하는 방식이 올바른 방식인가요?
- static으로 선언하여 클래스 초기화를 통해 캐싱을 하고 싶은 데 파일 경로를 static으로 불러 올 수 없어 사용할 수 없습니다. 이럴 땐 생성자를 통해 캐싱을 하는 것밖에 없을까요?
그렇기에 꼭 필요한 Exception 위주로 먼저 해주고, 한꺼번에 처리할 수 있는 Exception을 만들어서 한꺼번에 처리할 수 있는건 처리해주는것도 좋은 방법 일 것 같네요. -> 꼭 필요한 Exception을 만들어 두고 그것들 중에 한꺼번에 처리할 수 있는 Exception을 만들 수 있다면 해당 Exception으로 대처하라는 말씀이실까요?
면담시간에 말한 MenuType과 VoucherType을 바꾸려고 했지만 적용하지 않았습니다.
- MenuType의 경우 VoucherProgram에 의존하게 되어 테스트도 어려워지고 작성한다 해도 통합테스트의 성격을 띄는 것 같아 적용하지 않았습니다.
- VoucherType의 경우 checked Exception을 던지게 되는 데 이를 다시 RuntimeException으로 바꾸는 것은 큰 의미가 없다고 생각하여 적용하지 않았습니다.
- 이러한 이유때문에 적용하지 않았는 데 타당한 지 멘토님의 의견이 궁금합니다!!
- 추가적으로 VoucherType에서 RuntimeException을 던지도록 수정해도 될까요? 굳이 checked Exception으로 모두 처리할 필요가 없을까요?

yuminhwan added 30 commits April 6, 2022 02:13

feat: 프로젝트 초기 설정

f76a61a

docs: docs/README.md 업데이트

8e8c35b

feat: 금액으로 할인하는 기능 구현

bbb343a

docs: docs/README.md 업데이트

fb53b16

feat: 퍼센트로 할인하는 기능 구현

6f85729

feat: 메모리 저장 기능 구현

b000b44

refactor: Percent, Amount 값 검증 변경

7827314

feat: 스테레오타입 선언

f4bede4

feat: VoucherService 구현

285e8cf

style: @DisplayName 어노테이션 추가

552437e

feat: Enum 타입 메뉴 구현

03810f4

refactor: VoucherType에서 입력값 검증하도록 변경

4cb88bb

feat: 커스텀 Exception 추가

670f5fd

feat: View 구현

0139c98

feat: Voucher toString() 구현

5b180e0

feat: 로직에 맞게 실행가능하도록 구현

309f187

docs: docs/README.md 업데이트

9c48220

style: 메시지 알맞게 출력하도록 변경

3022919

style: insert -> save 메서드 네임 변경

bb3eb4f

feat: VoucherType 바우처 이름으로 찾는 기능 구현

58793ef

style: 네이밍 변경

cd84843

style: Voucher toString 스타일 변경

3922f90

refactor: VoucherType 바우처 이름으로 찾는 기능 삭제

a8cd305

fix: toString 변경

5efb78b

feat: Voucher 파일로 저장하는 기능 구현

6763573

refactor: 재입력 재귀문 -> while문으로 변경

012eb00

refactor: VoucherType name 필드 삭제

821cb36

refactor: 프로젝트 폴더 구조 변경

a6cf82c

refactor: 프로젝트 폴더 구조로 인한 파일 경로 변경

2ae26ff

feat: 고객 블랙 리스트 명단 조회 기능 구현

83eb986

yuminhwan added 9 commits April 10, 2022 16:11

feat: logger 기록

7dd78a2

feat: logback 설정 적용

f189fa2

fix: 파일 경로가 올바르지 않을 시 프로그램 종료하지 못하는 버그 수정

3d2286a

refactor: 블랙 리스트 파일 resource로 주입하도록 변경

28e2ce6

refactor: try with resources로 리팩토링

b3249c2

style: 코드 정리

e64c01c

docs: docs/README.md 업데이트

aa53602

style: 메서드명 변경

18f2281

refactor: resource를 applicationContext가 가져오도록 변경

19732fe

yuminhwan added 🔥 매운맛 Team-Armand labels Apr 10, 2022

yuminhwan requested a review from bosuksh April 10, 2022 13:21

yuminhwan self-assigned this Apr 10, 2022

fix: voucher 파일 경로 수정

399d6aa

bosuksh reviewed Apr 11, 2022

View reviewed changes

yuminhwan added 12 commits April 11, 2022 22:51

refactor: log 콘솔에 안 찍히도록 변경

70d5d56

refactor: 불필요한 log 삭제 및 파일에서 데이터를 못 가져올 시 로그 생성

ca2b231

refactor: 불필요한 메서드 삭제

8efe433

refactor: 블랙리스트를 인스턴스 생성 시 가져오도록 변경

8937202

refactor: @value로 파일 위치 주입으로 변경

75959a5

refactor: 로그 메시지 수정

3c0abd8

refactor: 테스트 수정 및 테스트 케이스 추가

96ca2a3

refactor: Mockito 적용

5ed7953

refactor: 캐싱 방식 변경

4538552

refactor: @value 적용 및 파일 못찾을 시 예외 발생하도록 수정

c4d01c1

refactor: 테스트 케이스 추가

4755bab

style: 어노테이션 추가

f3cec86

yuminhwan requested a review from bosuksh April 12, 2022 04:49

bosuksh merged commit dfb4c4b into prgrms-be-devcourse:yuminhwan/w1 Apr 27, 2022

[2기-유민환] SpringBoot Part1 Weekly Mission 제출합니다. #186

[2기-유민환] SpringBoot Part1 Weekly Mission 제출합니다. #186

Uh oh!

Conversation

yuminhwan commented Apr 10, 2022

📌 과제 설명

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

Console

AppConfiguration

VoucherProgram

📁 entity

📁 exception

📁 repository

📁 service

✅ PR 포인트 & 궁금한 점

Uh oh!

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

yuminhwan Apr 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

yuminhwan commented Apr 12, 2022

질문

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yuminhwan Apr 12, 2022 •

edited

Loading