-
Notifications
You must be signed in to change notification settings - Fork 169
[1기-엄진환] [3주차][W1D2~W1D5] Command-line Application #8
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
Conversation
|
진환님 멘토님 리뷰어를 안걸으셨네요!! |
기존 @bean 대신 @componentscan 사용하도록 변경
과제 커밋과 분리를 위해 강의 내용 완료 시점까지의 커밋 작성
📌 과제 설명과제 페이지(링크) 👩💻 요구 사항과 구현 내용create
✅ PR 포인트 & 궁금한 점feat:미션2 File 추가/조회 기능 구현 기능 구현내용에 필요한 내용을 수정했습니다. |
| static VoucherRepository voucherRepository; | ||
| public static void main(String[] args) { | ||
| var application = new AnnotationConfigApplicationContext(AppConfiguration.class); | ||
| voucherService = application.getBean(VoucherService.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
domain 로직에 spring 관련 코드가 있는 것은 조금 위험한 구현방식입니다.
그리고 voucherService가 commandLineApplication에
있는 것이 맞는지 한번 고민해보셔용
| { | ||
| Assert.isTrue(splitList.length == 3, "This command did not receive the required arguments."); | ||
| String voucherName = splitList[1]; | ||
| if (voucherName.equals("F")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
F 를 쓰는 것을 사용자가 알고 있을까요?
|
|
||
| while (!command.equals("exit")) { | ||
| String commandLine = scanner.nextLine(); | ||
| String[] splitList = commandLine.split(" "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용자가 입력한 값이 적절한지 check 하는 로직이 있으면
더 완벽해질 것 같습니다.
VictoryPark
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
질문 주신대로,
CommandLineApplication 클래스에 모든 기능을 넣어주셨네요!
기능을 분리하려면 어떻게 해야 할까도 한번 고민해보셔요!!
📌 W1D3 피드백 내용 구현
|
📌 코드 개선 진행팀원들이 작성한 코드들을 참고하여 구조를 개선했습니다.
|
📌 과제 설명(W1D4, W1D5)W1D4 : 블랙리스트 조회 기능 추가, yaml 설정하기(과제 링크) 👩💻 요구 사항과 구현 내용W1D4customer_blacklist.csv 구조type(normal/black), uuid, 이름 customerscustomer_blacklist.csv 파일의 모든 고객 정보 출력(uuid, 이름, 타입) blacks블랙리스트 타입의 고객만 출력(uuid, 이름, 타입) application.yaml
W1D5로그 파일(access-2021-08-27.log)
jar 파일 생성
✅ PR 포인트 & 궁금한 점블랙리스트의 출력을 위한 기능을 일반 고객과 블랙리스트 고객을 모두 출력해보기 위해서 |
| return voucher; | ||
| } | ||
|
|
||
| public void printAllVoucher() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
voucherService 에서 print 를 호출하는 것은
Input, Output 인터페이스를 구현하여 역할 분리를 한 것이 의미가 없어집니다.
다른 방법을 한번 생각해보셔요!!
|
|
||
| @Configuration | ||
| public class CommandLineApplication implements ApplicationRunner { | ||
| private final VoucherOperator voucherOperator; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 구현체로 타입을 선언하는 것이 아니라 다형성을 위해 인터페이스 타입으로 선언합니다.
| voucher.ifPresent(voucherService::saveVoucher); | ||
| } | ||
|
|
||
| public Optional<Voucher> createVoucher(String[] splitList) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validation check 로직이 조금 더 보강되었으면 좋겠네요..^.^
| public Optional<Voucher> createVoucher(String[] splitList) { | ||
| if (splitList.length != 2) return Optional.empty(); | ||
|
|
||
| Optional<Voucher> voucher = Optional.empty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사실, createVoucher 에서는 Optional일 필요가 없습니다.
Optional 은 select 로직에서 많이 사용됩니다.
왜냐하면 db에 데이터가 없을 경우 nullable하기 때문입니다.
| return Arrays.stream(VoucherType.values()) | ||
| .filter(voucherType -> voucherType.inputCommand.equals(command)) | ||
| .findAny() | ||
| .orElseThrow(IllegalArgumentException::new); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
잘못 입력했을때 예외를 던지는 것 말고 친절하게 다시 command를 작성할 수 있도록 바꿔볼까요?
| .orElseThrow(IllegalArgumentException::new); | ||
| } | ||
|
|
||
| public static Optional<Voucher> getVoucher(String inputCommand, long value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음..비즈니스 로직상 getVoucher 라는 naming 보다는 createVoucher 가 나을 것 같고
그러면 새로 new 하는 로직이니, Optional이지 않아도 됩니다.
VictoryPark
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommandLineApp 과 voucherService 간의
결합을 없애기 위해 많이 노력하셨군요!
진환님, 더 자세한 리뷰는 코멘트로 달아드렸습니다,.
확인해주세요😜
* On branch : develop
# 유저 생성 : UserFactory
# 유저 저장 : UserRepository
# 유저 타입 : UserType
* Changes to be committed:
Modified : User.java, UserFactory.java, UserFileRepository.java, UserType.java
* On branch : develop
# CSV 파일 읽기 : CSVReader
# CSV 파일 쓰기 : CSVWriter
# 샘플 CSV 파일 : user_allList, user_blackList
* Changes to be committed:
Modified : CSVReader.java, CSVWriter.java, sample.csv
feat : Customer can be added using jdbc template impl
refactor : 추상 클래스 수정 및 입력 검증 클래스 정규 표현식 수정
📌 과제 설명
과제 페이지(링크)
바우처 생성/조회 기능을 구현했습니다.
👩💻 요구 사항과 구현 내용
create
list
exit
✅ PR 포인트 & 궁금한 점
대부분의 기능을 CommandLineApplication.java에서 구현했습니다.
아쉬운 점 피드백 주시면 열심히 반영해보겠습니다!👨💻