6기 실전 연습 #1
6기 실전 연습 #1
Conversation
| .collect(Collectors.toUnmodifiableList()); | ||
| } | ||
|
|
||
| private <T> T repeat(Supplier<T> inputReader) { |
There was a problem hiding this comment.
제내릭을 사용하신게 인상깊네요! 덕분에 제내릭에 대해 한 번 알아볼 수 있는 경험이 되었습니다!
다만 메서드 이름을 repeat 보다는 getValidatedValue 나 getLegalValue 와 같은 식으로 하면 더 좋을 것 같다는 생각이 들었어요!
| throw new IllegalArgumentException("[ERROR] 각 메뉴는 중복될 수 없습니다."); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
코드의 복잡성 때문에 Coaches 클래스를 따로 안만드셨다고 하셨는데 저는 예외 처리의 책임을 분리하는 것만으로도 충분히 Coaches 클래스를 별로도 만들 이유가 있다고 생각해서 Coaches 라는 별도의 클래스를 만들었거든요. 일급 컬렉션을 사용해야하는 이유 링크를 참고해보시면 좋을것 같아요!
확실히 저도 고민했던 부분이여서 같이 얘기해보면 좋을 것 같아요!
There was a problem hiding this comment.
Coaches안에 로직이 오직 예외처리만 있다면 차라리 안 만들고 List 식으로 사용 하려는 생각 이었는데
@jhon3242님 말을 들어보니 예외처리의 책임을 분리하는 것만으로 충분히 이유가 있을수 있겠군요!
|
|
||
| public static Menu of(String menu) { | ||
| return new Menu(menu); | ||
| } |
There was a problem hiding this comment.
왜 생성자를 사용하지 않고 of 라는 메서드를 만들어서 활용하신지 궁금합니다🤔
There was a problem hiding this comment.
한번 정적 팩토리 메서드 연습겸 사용해봤습니다 ㅋㅋㅋㅋ 이유는 없습니다. 그냥 생성자 사용해도 될 거 같네요!!
| List<String> names = splitNames(inputNames); | ||
| validateMenuSize(names); | ||
| validateDuplicateMenu(names); | ||
| } |
There was a problem hiding this comment.
메뉴에 없는 음식이 입력된 경우에 대한 예외 처리가 추가되면 좋을 것 같아요!
| } | ||
|
|
||
| private boolean isValidCategory(List<Category> recommendedCategories, Category category) { | ||
| long count = recommendedCategories.stream().filter(c -> c.equals(category)).count(); |
There was a problem hiding this comment.
한 줄에 . 이 많아서 가독성을 조금 해치는 것 같아요 객체지향 생활체조 원칙 4번을 참고하시면 좋을 것 같아요!
There was a problem hiding this comment.
이 부분 놓쳤었네요 다 확인했다고 생각했는데 감사합니다!
| public static Map<Category, String> initMap() { | ||
| Map<Category, String> menuMap = new HashMap<>(); | ||
| menuMap.put(JAPAN, "규동, 우동, 미소시루, 스시, 가츠동, 오니기리, 하이라이스, 라멘, 오코노미야끼"); | ||
| menuMap.put(KOREAN, "김밥, 김치찌개, 쌈밥, 된장찌개, 비빔밥, 칼국수, 불고기, 떡볶이, 제육볶음"); | ||
| menuMap.put(CHINA, "깐풍기, 볶음면, 동파육, 짜장면, 짬뽕, 마파두부, 탕수육, 토마토 달걀볶음, 고추잡채"); | ||
| menuMap.put(ASIAN, "팟타이, 카오 팟, 나시고렝, 파인애플 볶음밥, 쌀국수, 똠얌꿍, 반미, 월남쌈, 분짜"); | ||
| menuMap.put(WESTERN, "라자냐, 그라탱, 뇨끼, 끼슈, 프렌치 토스트, 바게트, 스파게티, 피자, 파니니"); |
There was a problem hiding this comment.
메뉴을 하나의 String 이 아닌 여러 String으로 나누어서 넣어보면 어떨까요?
|
|
||
| public static void validateCoachName(String inputNames) { | ||
| List<String> names = splitNames(inputNames); | ||
| validateNames(names); | ||
| validateSize(names); | ||
| validateDuplicateCoachNames(names); | ||
| } |
There was a problem hiding this comment.
validateNames 는 Coach 내부적으로도 다시 한번 검증을 수행하고 있는데
InputValidator에서도 입력에 대한 유효성 검사를 하고
Coach의 Name에서도 유효성 검사를 총 2번 하는 이유가 궁금해요!
만약 Coach의 이름이 길이가 변경되는 요구사항이 생기면
그것에 대한 파급효과가 Coach 뿐만 아니라 InputValidator까지 영향을 가게 됩니다.
복잡한 프로그래밍일 수록 요구사항 변경에 의한 파급 효과가 많은 클래스에 영향을 주게 되고
그것은 곧 수정해야 하는 부분을 다 찾지 못하는 휴먼 에러가 발생하지 않을까? 라는 생각을 하게 되었어요
There was a problem hiding this comment.
@soochangoforit 님 말이 맞습니다!
제가 예외처리 위치에 대한 고민이 많아서 Coach 도메인에도 예외처리를 해놓고 InputValidator와 InputView 3가지 버전에 다 예외처리를 해놨는데 결정하는 과정에서 확인을 못한거 같아요!
지금 생각한 결론은 Coach 도메인에는 이름 같은 비즈니스 로직 예외만 처리하고 빈칸이나 타입같은 경우는 InputView에 할 거 같아요!
| private static void validateDuplicateMenu(List<String> names) { | ||
| Set<String> nameSet = new HashSet<>(); | ||
| for (String name : names) { | ||
| if (!nameSet.add(name)) { | ||
| throw new IllegalArgumentException("[ERROR] 각 메뉴는 중복될 수 없습니다."); | ||
| } | ||
| } |
| private void inputCoachCanNotEatMenu(List<Coach> coaches) { | ||
| for (Coach coach : coaches) { | ||
| List<Menu> menus = repeat(() -> inputCanNotEatMenu(coach)); | ||
| if (menus != null) { | ||
| coach.addCanNotEatMenu(menus); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
repeat의 파라미터가 Supplier 형식이라 어떻게 파라미터 있는 메서드를 재입력 로직을 적용할 수 있을까 고민했는데
그냥 파라미터 없고 return 값에 함수를 넣는 방식으로 해결할 수 있었군요
InitialMenu 위치 : 초기값 설정에 관한 요구사항이 나왔을 때, InitialMenu 클래스를 만들어서 컨트롤러에 생성자로 넣어두면 컨트롤러에서 사용하기 편할거 같아 이런식으로 구현을 했습니다. 이후 생각해보니 메뉴를 관리하는 주체가 Category 클래스라면 초기값 설정 역시 Category 내부에서 처리하는 것이 적합할것 같다는 생각이 들었습니다. 따라서 다음에는 Category 클래스 내부로 이 기능을 이동하는 것이 좋을 것 같습니다.
InputView와 예외 처리: 예외 처리를 위한 고민을 하면서 InputView 클래스 내에서 처리할 것인지, 아니면 InputValidator 클래스를 따로 만들어 처리할 것인지 고민했습니다. 예외 처리 중에 코치 인원이 2~5명이라는 특별한 예외 상황이 있었는데, 이를 처리하기 위해 Coaches 클래스를 만들었지만 실제로 Coaches 클래스에 추가로 로직을 구현하지 않아서 고민이 되었습니다. Coaches 클래스에서 로직이 없다면 불필요한 복잡성을 추가할 뿐이라고 생각했습니다. 지금 생각해보면 그냥 설계를 잘못한거 같기도 합니다... ㅋㅋㅋ 오직 예외 처리만 하는 InputValidator은 좋은 방법이지만 가독성 측면에서 불편할거 같다는 생각이 있습니다.
서비스 로직 위치: 처음에 추천 기능들을 모아놓는 Service를 사용했지만, MVC 아키텍처에 집중하고자 하여 Recommender 클래스를 도입하여 주요 로직을 분리를 하였습니다. 그러나 서비스 로직을 놓을 위치에 대한 고민은 여전히 남아있습니다. Controller에 로직을 모두 집중시키지 않고, 서비스 도입하여 비즈니스 로직을 분리하고 코드의 유지보수 측면에서도 좋기 때문입니다....