[1차 VER1.51] Java ToyProject upload by JisangLee #28

matrixpower1004 · 2023-05-10T15:29:51Z

refactoring : 멘토님 코드 리뷰 내용 반영

멘토님께 질문 사항 :

Enum의 name을 파라미터로 넘어온 String 값과 비교할 때 ==을 사용해야 할까요? 아니면 equals를 사용하는 게 맞을까요?
SummaryService에서 Comparator를 모듈화 해서 중복 코드를 줄였지만 이걸 더 추상화할 수 있는 방법이 있을까요?

Release Date : 2023-06-03

change list :

사용되지 않고 남아 있던 불필요한 메서드 제거
불필요한 주석 및 else keyword 제거
GroupService.java : searchByInput() 메서드 로직 변경
- GroupType NONE에 대한 필터링 처리
  - 변경 전 : .skip(1)
  - 변경 후 : .filter(type -> !type.isName("NONE"))
- GroupType명 비교를 GroupType에 메서드를 선언하여 비교
  - 변경 전 : .filter(type -> type.name().equals(input) || type.getShortName().equals(input))
  - 변경 후 : .filter(type -> type.isName(input))
Groups 인스턴스 생성시 default NONE 그룹을 add하는 로직을 생성자로 이동.
Groups.java - findGroup() 메서드 로직 변경
- null을 반환하던 로직을 예외를 던지도록 변경.
- GroupType 비교를 해당 객체에 메서드를 선언하여 비교.
  - 변경 전 : if (groups.get(i).getGroupType() == groupType
  - 변경 후 : if (*groups*.get(i).isGroupType(groupType))
SummaryService.java : showSummaryHeader() 로직 변경
- String + 로 연결하던 로직을 StringBuild로 변경
- 불필요한 if statement 제거
- arrayByGroupType() 메서드에서 GroupType 필터링 방법 변경
  - 변경 전 : .filter(customer -> group.getGroupType() == customer.getGroupType())
  - 변경 후 : .filter(customer -> group.isGroupType(customer.getGroupType()))
SummaryMenu.java
- manage() 메서드 중복 코드 삭제
- getSortOrder() 로직 변경

ToDo

하위 메서드에서 발생한 Exception을 상위 메서드로 전달하려면 어떤 방법이 더 좋을까?
break로 실행 제어를 하는게 좋은 방법일까?
SummaryService에서 Comparator를 모듈화 해서 중복을 줄였지만 이걸 더 추상화할 수 있는 방법은 없을까?

멘토님 코드 리뷰 감사합니다. ❤️

feat : 1차 기능 완성(미구현 기능 및 버그 존재함) 2. Customer Data 메뉴와 3. Classification Summary 메뉴 일부만 완성 되었습니다. ## 중점사항 - 최대한 메뉴가 Controller 역할을 할 수 있도록 서비스 로직을 분리하였습니다. - 초반에 디자인을 잘못해서 2번을 전체 구조를 다시 짰습니다. - 코드 작성 시간보다 버그 잡는 시간이 더 많이 걸렸네요. 요구사항 분석과 프로젝트 설계가 중요하다는 것을 배웠습니다. ## 해결해야 할 문제 1. 객체간 연관 관계를 잘못 이해하고 코드를 작성해서 Customer Data 입력 부분에 오류가 있습니다. 2. 미구현 기능등은 주말까지 구현할 예정입니다. 3. 싱글톤으로 생성된 객체는 한 번 생성 후 아래 코드와 같이 다른 객체에게 의존성을 주입하는 방법이 맞는지 궁금합니다. 😊 ```java private final Customers customers = Customers.getInstance(); private final Groups groups = Groups.getInstance(); if (choice == 1) summaryService.showDefault(customers, groups); if (choice == 2) summaryService.sortByName(customers, summaryMenu, groups); if (choice == 3) summaryService.sortByTime(customers, summaryMenu, groups); if (choice == 4) summaryService.sortByPayment(customers, summaryMenu, groups); if (choice == 5) break; ``` 항상 예리하고 배움이 되는 리뷰 남겨주셔서 감사합니다. ❤️

## feat : Summary 기능 완성 및 테스트 완료 ## Bug Fix - SummaryService 정렬이 적용되지 않던 문제가 해결되었습니다. ## Refactoring - SummaryService의 정렬 및 출력에 Stream 적극 활용 ## ToDo - SummaryService 정렬 파트 중복 코드 줄일 수 있는 방법은? 코드 리뷰 감사합니다. ❤️

### bugfix : Summary 기능 버그 수정 및 테스트 완료 --- Release Date : 2023-05-11 ## Bug Fix - SummaryService 정렬 방식 입력할 때 end 입력으로 종료할 수 있는 기능이 누락되어 추가 - SummaryMenu에서 하위 ↔ 상위간 입력 메뉴 전환이 엉뚱하게 동작하는 것을 수정 - 버그 수정을 하면서 throw를 이용해서 loop를 어떻게 제어해야 하는지 확실히 이해하게 되었다. - Menu interface의 Exception 처리 수정. thorw new로 Exception을 발생시키고 super로 전달하지 않았던 코드 수정. ## Refactoring - Customers.java : - Customer 객체를 배열로 반환하는 getCustomers() 메서드를 Stream을 이용하여 반환하는데 성공. ```java //기존코드 public Customer[] getCustomers() { Customer[] customerArr = new Customer[size]; for (int i = 0; i < size; i++) { customerArr[i] = customers.get(i); } return customerArr; } //변경된 코드 public Customer[] getCustomers() { return IntStream.range(0, customers.size()) .mapToObj(index -> customers.get(index)) .toArray(Customer[]::new); } ``` - SummaryService.java - 정렬 후 출력하는 메서드들의 코드 중복 제거. - 정렬 순서 입력 받는 메뉴의 위치 변경 → SummaryMenu로 이동. 메뉴에서 입력까지 받은 후 SummaryService에 매개변수로 전달하도록 로직 수정. ## ToDo - CustomerService의 메뉴 전환 버그 수정 - CustomerService의 메뉴 전환 시 입력 오류 메시지 출력하는 버그 수정 - Customer 등록 메뉴 구현 - Parameter 등록 메뉴 구현 코드 리뷰 감사합니다. ❤️

bugfix : Customer 등록 기능 버그 수정 - Customer GroupType 지정 루틴에 버그가 발견되어 수정 - Summary 그룹 분류 과정에 버그가 발견되어 수정 feat : Customer 등록시 ID 중복 체크 기능 추가 - TestCase에 NONE 타입 고객도 들어갈 수 있도록 코드 추가 refactoring : 고객 등급 판별 루틴에 가독성 향상을 위한 BiPredicate 적용 - 고객의 Parameter 값이 GENERAL Type의 기준보다 낮은지 판별 - 고객의 Parameter 값이 어느 GroupType에 해당하는지 판별 ```java BiPredicate<Group, Customer> lessThanGeneral = (group, customer) -> { if (customer.getCusTotalPay() < group.getParameter().getMinPay() || customer.getCusTotalTime() < group.getParameter().getMinTime()) { return true; } else return false; }; BiPredicate<Group, Customer> compareParameter = (group, customer) -> { if (customer.getCusTotalPay() >= group.getParameter().getMinPay() && customer.getCusTotalTime() >= group.getParameter().getMinTime()) { return true; } else return false; }; ``` ToDo - Parameter 등록 메뉴 구현 - 통합 테스트 및 남아 있는 버그 수정 Release Date : 2023-05-14 코드 리뷰 감사합니다. ❤️

feat : Parameter 메뉴 구현 완료 --- bugfix : Customer 메뉴 이동 버그 수정 - test.run() 에서 모든 기능 테스트 완료 ToDo - 데이터가 없는 초기 상태에서 테스트 및 예외 발생시 코드 수정 Release Date : 2023-05-15 코드 리뷰 감사합니다. ❤️

bugfix : Customer, Parameter 등록 과정에서 발생하는 버그 수정 및 예외 처리 Release Date : 2023-05-15 --- change list : - Groups 객체 생성 시 기본 그룹 NONE 자동으로 생성하도록 수정 - Customer, Parameter 등록 loop및 try-catch 처리 로직 수정 - 각 도메인별 Menu와 Service 객체를 private final로 선언 시 일정 개수 이상이 되면 시스템이 죽는 증상이 발생하여 private static final로 수정 프로젝트 후기 - 객체간 관계를 충분히 고려하지 않고 코딩부터 시작해서, 중간에 구조를 2번이나 갈아 엎어야 했다. 프로젝트를 결국 완성은 했지만 불필요한 시간 투입이 많았던 점은 고치도록 해야겠다. - Stream과 함수형 인터페이스를 최대한 적용해 프로젝트를 진행하면서 이해가 좀 더 깊어지는 효과가 있었다. - try-catch 와 while을 이용하여 맞는 값이 들어올 때까지 반복하는 루틴을 다양한 조합으로 많이 시도해 보면서 이해도 깊어지고 나만의 안정적인 조합을 찾아 적용한 것이 이 프로젝트의 최대 성과라 할 수 있다. - 프로젝트 제약 조건이 List, Set, Map 등 컬렉션의 사용을 금지하고 배열을 이용하는 프로젝트라 다양한 컬렉션으로 좀 더 간결하고 가독성이 좋은 코드를 작성하지 못한 점이 아쉽다. ToDo - 모든 기능을 Service 하나에 다 집어 넣었는데 디자인 패턴과 AOP에 대해서 더 학습하고 어떻게 분리를 하면 좋을지 연구를 해봐야겠다. - Customer와 Group간 관계가 조금 애매했는데 Group 분류 기준인 Group 객체 자체를 Customer에 포함시키는 게 맞는지 의문이 들어서 Customer 객체에는 GroupType만 포함되도록 수정했는 데 객체지향 관점에서 어떤 설계가 맞는지 좀 더 생각해 보고 학습을 해야겠다. 멘토님 코드 리뷰 감사합니다. ❤️

lalwr

소스 구현을 신경써써 구현한게 눈에 보이네요~

lalwr · 2023-06-01T13:44:12Z

me.smartstore/src/domain/customer/Customer.java

+    private String cusId;
+    private Integer cusTotalTime;
+    private Integer cusTotalPay;
+    //    private Group group;


주석은 제거해주세요.

lalwr · 2023-06-01T13:46:23Z

me.smartstore/src/domain/group/Groups.java

+        if (groups == null) {
+            groups = new Groups();
+            //Groups 인스턴스 생성시 기본 NONE 그룹을 같이 생성.
+            groups.add(new Group(new Parameter(0, 0), GroupType.NONE));


기본 셋팅이 필요하다면 생성자에서 셋팅하는것은 어떨까요?

lalwr · 2023-06-01T13:49:04Z

me.smartstore/src/domain/group/Groups.java

+        try {
+            int index = 0;
+            for (int i = 0; i < groups.size; i++) {
+                if (groups.get(i).getGroupType() == groupType) {


get해서 비교하기 보다는 해당 객체에 메소드를 만들어 구현하는것이 좋습니다.

리뷰 내용이 잘 이해가 되지 않습니다. Groups가 Group 객체들이 모여 있는 배열이라, 배열에 저장되어 있는 Group 오브젝트를 get() 으로 빼오지 않는다면 비교할 방법이 없다고 생각됩니다. get()으로 빼오지 않고 어떤 방법으로 파라미터로 넘어온 groupType을 비교할 수 있을까요? 힌트를 주세요. :)

groups.get(i).isGroupType(groupType) 로 Group 클래스 내부에서 groupType을 비교하는 방법을 만한거였습니다!

아하. 멘토님께서 리뷰해 주신 내용이 좀 더 책임과 역할이 명확한 방법인 것 같습니다. ❤️

lalwr · 2023-06-01T13:49:30Z

me.smartstore/src/domain/group/Groups.java

+            }
+            //검색 결과 index가 1보다 작다면 찾는 그룹이 없다는 뜻이므로 null return 후 그룹 등록을 유도
+            if (index < 1) return null;
+            else return groups.get(index);


else문은 없어도 되지않을까요?

lalwr · 2023-06-01T13:52:08Z

me.smartstore/src/domain/menu/SummaryMenu.java

+
+                if (choice == 1) summaryService.showDefaultSummary();
+                if (choice == 2) {
+                    boolean sortOrder = getSortOrder();


boolean sortOrder = getSortOrder(); 중복 코드는 1개만 선언하요 사용이 가능할꺼 같습니다.

넵. 연구해 보겠습니다.

lalwr · 2023-06-01T13:52:41Z

me.smartstore/src/domain/menu/SummaryMenu.java

+            String order = summaryMenu.nextLineUpper(END_MSG);
+            if (order.equals("A")) return false;
+            else if (order.equals("D")) return true;
+            else throw new InputFormatException();


else 문 제거해도 됩니다.

lalwr · 2023-06-01T13:58:20Z

me.smartstore/src/Service/GroupService.java

+        //GroupType ENUM을 배열 형태로 변환 후 0번 index NONE은 스킵하고
+        //사용자가 입력한 문자에 해당하는 GroupType을 찾아서 반환한다.
+        return Arrays.stream(GroupType.values())
+                .skip(1)    // NONE은 건너뜀


NONE 에 대한 필터링 처리를 하는것이 좋지 않을까요?

네. 필터 처리로 변경해 보겠습니다.

lalwr · 2023-06-01T13:59:15Z

me.smartstore/src/Service/GroupService.java

+        //사용자가 입력한 문자에 해당하는 GroupType을 찾아서 반환한다.
+        return Arrays.stream(GroupType.values())
+                .skip(1)    // NONE은 건너뜀
+                .filter(type -> type.name().equals(input) || type.getShortName().equals(input))


get이 아닌 해당 객체에 메소드를 선언하여 사용해보세요.

GroupType은 Enum 이고 NONE("N"), GENERAL("G"), VIP("V"), VVIP("VV") 이런 식으로 Enum 내부에 String type으로 ShortName이 들어가 있는 구조입니다. enum을 배열로 변환 후 사용자가 입력한 값이 enum의 name과 같은지 비교하도록 구현했는데 어떤 방법으로 get을 사용하지 않고 비교할 수 있을까요? 힌트를 주세요. :)

.filter(type -> type.isName(input)) 데이터를 가져오는 것이 아닌 해당 객체 내부에서 검증하는 것을 말한거였습니다!

lalwr · 2023-06-01T14:01:00Z

me.smartstore/src/Service/SummaryService.java

+    }
+
+    private String showSummaryHeader(Group group) {
+        String returnStr = "\n=============================="


stringbuilder 또는 stringbuffer 사용해서 문자열 계속 더 할때 사용하는 것이 좋습니다.

refactoring : 멘토님 코드 리뷰 내용 반영 멘토님께 질문 사항 : - [ ] Enum의 name을 파라미터로 넘어온 String 값과 비교할 때 ==을 사용해야 할까요? 아니면 equals를 사용하는 게 맞을까요? - [ ] SummaryService에서 Comparator를 모듈화 해서 중복 코드를 줄였지만 이걸 더 추상화할 수 있는 방법이 있을까요? Release Date : 2023-06-03 --- ### change list : - 사용되지 않고 남아 있던 불필요한 메서드 제거 - 불필요한 주석 및 else keyword 제거 - GroupService.java : searchByInput() 메서드 로직 변경 - GroupType NONE에 대한 필터링 처리 - 변경 전 : `.skip(1)` - 변경 후 : `.filter(type -> !type.isName("NONE"))` - GroupType명 비교를 GroupType에 메서드를 선언하여 비교 - 변경 전 : `.filter(type -> type.name().equals(input) || type.getShortName().equals(input))` - 변경 후 : `.filter(type -> type.isName(input))` - Groups 인스턴스 생성시 default NONE 그룹을 add하는 로직을 생성자로 이동. - Groups.java - findGroup() 메서드 로직 변경 - null을 반환하던 로직을 예외를 던지도록 변경. - GroupType 비교를 해당 객체에 메서드를 선언하여 비교. - 변경 전 : `if (groups.get(i).getGroupType() == groupType` - 변경 후 : `if (*groups*.get(i).isGroupType(groupType))` - SummaryService.java : showSummaryHeader() 로직 변경 - String + 로 연결하던 로직을 StringBuild로 변경 - 불필요한 if statement 제거 - arrayByGroupType() 메서드에서 GroupType 필터링 방법 변경 - 변경 전 : `.filter(customer -> group.getGroupType() == customer.getGroupType())` - 변경 후 : `.filter(customer -> group.isGroupType(customer.getGroupType()))` - SummaryMenu.java - manage() 메서드 중복 코드 삭제 - getSortOrder() 로직 변경 ### ToDo - 하위 메서드에서 발생한 Exception을 상위 메서드로 전달하려면 어떤 방법이 더 좋을까? - break로 실행 제어를 하는게 좋은 방법일까? - SummaryService에서 Comparator를 모듈화 해서 중복을 줄였지만 이걸 더 추상화할 수 있는 방법은 없을까? 멘토님 코드 리뷰 감사합니다. ❤️

matrixpower1004 · 2023-06-03T13:52:01Z

소스 구현을 신경써써 구현한게 눈에 보이네요~

멘토님께서 리뷰해 주신 내용들 모두 수정해서 다시 PR드렸습니다. :)

refactoring : 멘토님 코드 리뷰 내용 반영 멘토님께 질문 사항 : - [ ] Enum의 name을 파라미터로 넘어온 String 값과 비교할 때 ==을 사용해야 할까요? 아니면 equals를 사용하는 게 맞을까요? - [ ] SummaryService에서 Comparator를 모듈화 해서 중복 코드를 줄였지만 이걸 더 추상화할 수 있는 방법이 있을까요? Release Date : 2023-06-03 --- ### change list : - 사용되지 않고 남아 있던 불필요한 메서드 제거 - 불필요한 주석 및 else keyword 제거 - GroupService.java : searchByInput() 메서드 로직 변경 - GroupType NONE에 대한 필터링 처리 - 변경 전 : `.skip(1)` - 변경 후 : `.filter(type -> !type.isName("NONE"))` - GroupType명 비교를 GroupType에 메서드를 선언하여 비교 - 변경 전 : `.filter(type -> type.name().equals(input) || type.getShortName().equals(input))` - 변경 후 : `.filter(type -> type.isName(input))` - Groups 인스턴스 생성시 default NONE 그룹을 add하는 로직을 생성자로 이동. - Groups.java - findGroup() 메서드 로직 변경 - null을 반환하던 로직을 예외를 던지도록 변경. - GroupType 비교를 해당 객체에 메서드를 선언하여 비교. - 변경 전 : `if (groups.get(i).getGroupType() == groupType` - 변경 후 : `if (*groups*.get(i).isGroupType(groupType))` - SummaryService.java : showSummaryHeader() 로직 변경 - String + 로 연결하던 로직을 StringBuild로 변경 - 불필요한 if statement 제거 - arrayByGroupType() 메서드에서 GroupType 필터링 방법 변경 - 변경 전 : `.filter(customer -> group.getGroupType() == customer.getGroupType())` - 변경 후 : `.filter(customer -> group.isGroupType(customer.getGroupType()))` - SummaryMenu.java - manage() 메서드 중복 코드 삭제 - getSortOrder() 로직 변경 ### ToDo - 하위 메서드에서 발생한 Exception을 상위 메서드로 전달하려면 어떤 방법이 더 좋을까? - break로 실행 제어를 하는게 좋은 방법일까? - SummaryService에서 Comparator를 모듈화 해서 중복을 줄였지만 이걸 더 추상화할 수 있는 방법은 없을까? 멘토님 코드 리뷰 감사합니다. ❤️

matrixpower1004 added 6 commits May 10, 2023 23:32

matrixpower1004 changed the title ~~[1차 VER1.0] Java ToyProject upload by Jisang Lee~~ [1차 VER1.5] Java ToyProject upload by JisangLee Jun 1, 2023

lalwr reviewed Jun 1, 2023

View reviewed changes

matrixpower1004 changed the title ~~[1차 VER1.5] Java ToyProject upload by JisangLee~~ [1차 VER1.51] Java ToyProject upload by JisangLee Jun 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1차 VER1.51] Java ToyProject upload by JisangLee #28

[1차 VER1.51] Java ToyProject upload by JisangLee #28

matrixpower1004 commented May 10, 2023 •

edited

Loading

lalwr left a comment

lalwr Jun 1, 2023

matrixpower1004 Jun 1, 2023

lalwr Jun 1, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 1, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 3, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 1, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 1, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 1, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 1, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 1, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 3, 2023

lalwr Jun 1, 2023

matrixpower1004 Jun 1, 2023

matrixpower1004 commented Jun 3, 2023

[1차 VER1.51] Java ToyProject upload by JisangLee #28

Are you sure you want to change the base?

[1차 VER1.51] Java ToyProject upload by JisangLee #28

Conversation

matrixpower1004 commented May 10, 2023 • edited Loading

change list :

ToDo

lalwr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matrixpower1004 commented Jun 3, 2023

matrixpower1004 commented May 10, 2023 •

edited

Loading