-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Code Review #1916
base: main
Are you sure you want to change the base?
Code Review #1916
Changes from 38 commits
513b727
bacfd04
a8871e1
3a66279
2955897
9eae8f1
65e1541
7ac7534
d72d8c6
91a82e0
d0efb04
4d795d9
dfa499f
27607b6
2bcf2ea
21afeba
e3b97f4
1af8196
d541208
8fa22cd
560de3f
90a1a4a
a3e310d
fa305ed
711a0e0
30d730e
dcd707c
29cbbbe
8fd2115
418b3a4
d1164b8
d28dadf
13c4863
e31a7e0
1336d82
36ed8c2
bdab76b
4cf7998
dab2848
a8d33b1
f5c568c
f748023
4443095
6839f41
98fb823
68d6133
670f786
a43a353
4499059
7cf8f46
932c7e5
d15aa50
6e53252
6a58b65
6f8a72d
f70356f
ef51dec
a956788
9f47ef8
27f289f
f4e3318
a3bcb0b
f2b59f6
c54eaed
9599abd
72acbca
59d8a96
f8d3c75
eabf2b4
6d86583
2f73cc5
068fda3
c85bccb
5506941
647fc1e
c365732
9b8a075
d76546f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,17 @@ | ||
# java-calculator-precourse | ||
# java-calculator-precourse | ||
|
||
구현 할 기능 목록 | ||
1. 문자열을 입력받는 기능 | ||
2. 문자열을 구분자를 기준으로 분리하는 기능 | ||
3. 커스텀 구분자를 지정하는 기능 | ||
4. 잘못된 값을 입력할 경우 예외처리하는 기능 | ||
5. 숫자를 더하는 기능 | ||
6. 결과를 출력하는 기능 | ||
|
||
|
||
초기 구현 후 리팩토링 할 목록 | ||
1. 문자열을 구분자를 기준으로 분리하는 기능 -> 기능들을 더 메소드 단위로 나누기 | ||
|
||
|
||
예외처리 | ||
1. 잘못된 입력 | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
plugins { | ||
id 'org.gradle.toolchains.foojay-resolver-convention' version '0.8.0' | ||
} | ||
rootProject.name = 'java-calculator' | ||
rootProject.name = 'java-menu' |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package menu; | ||
|
||
import menu.controller.RecommendController; | ||
import menu.service.RecommendService; | ||
import menu.view.InputView; | ||
import menu.view.OutputView; | ||
|
||
public class Application { | ||
public static void main(String[] args) { | ||
RecommendService recommendService = new RecommendService(); | ||
InputView inputView = new InputView(); | ||
OutputView outputView = new OutputView(); | ||
|
||
RecommendController recommendController = new RecommendController(recommendService, inputView, outputView); | ||
recommendController.run(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
package menu.controller; | ||
|
||
import java.util.ArrayList; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import menu.domain.Recommend; | ||
import menu.service.RecommendService; | ||
import menu.view.InputView; | ||
import menu.view.OutputView; | ||
|
||
public class RecommendController { | ||
private RecommendService recommendService; | ||
private InputView inputView; | ||
private OutputView outputView; | ||
private Recommend recommend; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. final로 선언하지 않은 이유가 있나요? |
||
|
||
public RecommendController(RecommendService recommendService, | ||
InputView inputView, | ||
OutputView outputView) { | ||
this.recommendService = recommendService; | ||
this.inputView = inputView; | ||
this.outputView = outputView; | ||
} | ||
|
||
public void run() { | ||
outputView.printStart(); | ||
List<String> coachs = inputView.readCoach(); | ||
Map<String, List<String>> inedibles = new HashMap<>(); | ||
Map<String, List<String>> recommendMenus = new HashMap<>(); | ||
for (String coach : coachs) { | ||
List<String> inedible = inputView.readInedible(coach); | ||
List<String> recommendMenu = new ArrayList<>(); | ||
inedibles.put(coach, inedible); | ||
recommendMenus.put(coach, recommendMenu); | ||
} | ||
recommend = new Recommend(coachs, inedibles, recommendMenus); | ||
while (recommend.recommendAmount() < 5) { | ||
recommendService.recommendMenu(recommend); | ||
} | ||
outputView.printResult(recommend); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package menu.domain; | ||
|
||
import camp.nextstep.edu.missionutils.Randoms; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import menu.util.Exception; | ||
|
||
public enum Category { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 도메인의 특성을 생각했을 때 저는 이 enum클래스 명을 menu라고 설정하는게 좀 더 적절하다고 생각해요. 어떻게 생각하시나요? |
||
일식(1, List.of("규동", "우동", "미소시루", "스시", "가츠동", "오니기리", "하이라이스", "라멘", "오코노미야끼")), | ||
한식(2, List.of("김밥", "김치찌개", "쌈밥", "된장찌개", "비빔밥", "칼국수", "불고기", "떡볶이", "제육볶음")), | ||
중식(3, List.of("깐풍기", "볶음면", "동파육", "짜장면", "짬뽕", "마파두부", "탕수육", "토마토 달걀볶음", "고추잡채")), | ||
아시안(4, List.of("팟타이", "카오 팟", "나시고렝", "파인애플 볶음밥", "쌀국수", "똠얌꿍", "반미", "월남쌈", "분짜")), | ||
양식(5, List.of("라자냐", "그라탱", "뇨끼", "끼슈", "프렌치 토스트", "바게트", "스파게티", "피자", "파니니")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. enum의 이름을 한국어로 한다면 단점으론 무엇이 있을까요? (바꾸라는 의미는 아님) |
||
|
||
private int categoryNumber; | ||
private List<String> menus; | ||
|
||
Category(int categoryNumber, List<String> menus) { | ||
this.categoryNumber = categoryNumber; | ||
this.menus = menus; | ||
} | ||
|
||
public static String recommendMenu(String category) { | ||
List<String> menu = Category.valueOf(category).menus; | ||
return Randoms.shuffle(menu).get(0); | ||
} | ||
|
||
public static String randomCategory() { | ||
int randomNumber = Randoms.pickNumberInRange(1, 5); | ||
return Arrays.stream(Category.values()) | ||
.filter(value -> value.categoryNumber == randomNumber) | ||
.map(Category::name) | ||
.findFirst() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. findFirst와 findAny의 차이점은 무엇일까요? |
||
.orElseThrow(() -> new IllegalArgumentException(Exception.ERROR + Exception.RANDOM_NUMBER)); | ||
} | ||
|
||
public static List<String> getAllMenu() { | ||
List<String> allMenu = new ArrayList<>(); | ||
for (Category value : values()) { | ||
for (String menu : value.menus) { | ||
allMenu.add(menu); | ||
} | ||
} | ||
return allMenu; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,50 @@ | ||||||||||
package menu.domain; | ||||||||||
|
||||||||||
import java.util.ArrayList; | ||||||||||
import java.util.List; | ||||||||||
import java.util.Map; | ||||||||||
|
||||||||||
public class Recommend { | ||||||||||
private List<String> categories = new ArrayList<>(); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 위치에서 arrayList를 만드는 것과, 생성자에서 arrayList를 만드는 건 어떤 차이가 있을까요? |
||||||||||
private List<String> coach; | ||||||||||
private Map<String, List<String>> inedible; | ||||||||||
private Map<String, List<String>> recommendMenu; | ||||||||||
|
||||||||||
public Recommend(List<String> coach, Map<String, List<String>> inedible, Map<String, List<String>> recommendMenu) { | ||||||||||
this.coach = coach; | ||||||||||
this.inedible = inedible; | ||||||||||
this.recommendMenu = recommendMenu; | ||||||||||
} | ||||||||||
|
||||||||||
public boolean isCategoryAmountOverThree(String category) { | ||||||||||
long count = categories.stream().filter(c -> c.equals(category)).count(); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
if (count >= 2) { | ||||||||||
return true; | ||||||||||
} | ||||||||||
return false; | ||||||||||
} | ||||||||||
|
||||||||||
public void addCategory(String category) { | ||||||||||
categories.add(category); | ||||||||||
} | ||||||||||
|
||||||||||
public int recommendAmount() { | ||||||||||
return categories.size(); | ||||||||||
} | ||||||||||
|
||||||||||
public List<String> getCoach() { | ||||||||||
return coach; | ||||||||||
} | ||||||||||
|
||||||||||
public Map<String, List<String>> getRecommendMenu() { | ||||||||||
return inedible; | ||||||||||
} | ||||||||||
|
||||||||||
public List<String> getCategories() { | ||||||||||
return categories; | ||||||||||
} | ||||||||||
|
||||||||||
public List<String> getRecommendMenu(String coach) { | ||||||||||
return recommendMenu.get(coach); | ||||||||||
} | ||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package menu.service; | ||
|
||
import menu.domain.Category; | ||
import menu.domain.Recommend; | ||
|
||
public class RecommendService { | ||
public void recommendMenu(Recommend recommend) { | ||
String category = Category.randomCategory(); | ||
if (recommend.isCategoryAmountOverThree(category)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. category amount에 대한 조건이 변경되면 메서드명도 바껴야 하겠네요. 다른 메서드명을 선택해보는건 어떨까요? |
||
return; | ||
} | ||
recommend.addCategory(category); | ||
for (String coach : recommend.getCoach()) { | ||
String menu = null; | ||
while (true) { | ||
menu = Category.recommendMenu(category); | ||
if (!recommend.getRecommendMenu().get(coach).contains(menu)) { | ||
break; | ||
} | ||
} | ||
recommend.getRecommendMenu(coach).add(menu); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. depth가 너무 깊네요. 요구사항을 확인해주세요. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package menu.util; | ||
|
||
public class Exception { | ||
public static final String ERROR = "[ERROR]"; | ||
public static final String COACH_NAME_LENGTH = "코치의 이름은 최소 2글자, 최대 4글자여야 합니다."; | ||
public static final String COACH_AMOUNT = "코치는 최소 2명, 최대 5명까지 식사를 함께 할 수 있습니다."; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기에 이름의 길이, 코치 명수 제한에 대한 값이 들어 있으면 비즈니스 로직이 변경될 때 여러 클래스에 변경사항을 적용해야 하겠네요. 이 곳까지 그런 책임을 주는건 부적절해보입니다. 다른 방법을 찾아봅시다! |
||
public static final String RANDOM_NUMBER = "랜덤 숫자가 범위를 벗어났습니다."; | ||
public static final String INVALID_INEDIBLE = "유효하지 않는 메뉴입니다."; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 왜 유효하지 않은지 알려주면 더 좋은 에러 메세지가 될 것 같아요. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package menu.util; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
public class Parser { | ||
public static List<String> parseInputToList(String[] splitInput) { | ||
List<String> list = new ArrayList<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
for (String input : splitInput) { | ||
list.add(input); | ||
} | ||
return list; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package menu.util; | ||
|
||
public class Spliter { | ||
public static String[] splitInput(String input) { | ||
return input.split(","); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package menu.util; | ||
|
||
import java.util.List; | ||
import menu.domain.Category; | ||
|
||
public class Validator { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. validator 클래스의 내용이 Coach나 Menu 클래스 내부에 있으면 어떨까요? |
||
public static void validateCoach(String[] coachs) { | ||
validateNameLength(coachs); | ||
validateCoachAmount(coachs); | ||
} | ||
|
||
public static void validateNameLength(String[] coachs) { | ||
for (String coach : coachs) { | ||
if (coach.length() < 2 || coach.length() > 4) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 정수값인 2와 4는 상수로 뺄 수 있을 것 같아요 👀 |
||
throw new IllegalArgumentException(Exception.ERROR + Exception.COACH_NAME_LENGTH); | ||
} | ||
} | ||
} | ||
|
||
public static void validateCoachAmount(String[] coachs) { | ||
if (coachs.length < 2 || coachs.length > 5) { | ||
throw new IllegalArgumentException(Exception.ERROR + Exception.COACH_AMOUNT); | ||
} | ||
} | ||
|
||
public static void validateInedible(String[] inedible) { | ||
List<String> allMenu = Category.getAllMenu(); | ||
for (String inedibleMenu : inedible) { | ||
if (!allMenu.contains(inedibleMenu)) { | ||
throw new IllegalArgumentException(Exception.ERROR + Exception.INVALID_INEDIBLE); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,40 @@ | ||||||||||||||||||||||||||||||||||||||||||||
package menu.view; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
import camp.nextstep.edu.missionutils.Console; | ||||||||||||||||||||||||||||||||||||||||||||
import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||
import menu.util.Parser; | ||||||||||||||||||||||||||||||||||||||||||||
import menu.util.Spliter; | ||||||||||||||||||||||||||||||||||||||||||||
import menu.util.Validator; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public class InputView { | ||||||||||||||||||||||||||||||||||||||||||||
private static final String INPUT_COACH_MSG = "코치의 이름을 입력해 주세요. (, 로 구분)"; | ||||||||||||||||||||||||||||||||||||||||||||
private static final String INPUT_INEDIBLE = "\n%s(이)가 못 먹는 메뉴를 입력해 주세요.\n"; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public List<String> readCoach() { | ||||||||||||||||||||||||||||||||||||||||||||
while (true) { | ||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||
System.out.println(INPUT_COACH_MSG); | ||||||||||||||||||||||||||||||||||||||||||||
String input = Console.readLine(); | ||||||||||||||||||||||||||||||||||||||||||||
String[] splitInput = Spliter.splitInput(input); | ||||||||||||||||||||||||||||||||||||||||||||
Validator.validateCoach(splitInput); | ||||||||||||||||||||||||||||||||||||||||||||
return Parser.parseInputToList(splitInput); | ||||||||||||||||||||||||||||||||||||||||||||
} catch (IllegalArgumentException e) { | ||||||||||||||||||||||||||||||||||||||||||||
System.out.println(e.getMessage()); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
저는 보통 관련 로직을 컨트롤러에 작성하는 편이에요. view에 있는 입력 로직은 가능하면 1회만 가능하도록 구현하고, 컨트롤러에서 재입력 로직을 구현하려고 합니다. 왜냐하면 저는 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추가로 |
||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public List<String> readInedible(String name) { | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 먹지 못하는 메뉴가 없으면 빈 값을 입력하게 되는데, 지금은 빈 값을 입력할 수 없네요. |
||||||||||||||||||||||||||||||||||||||||||||
while (true) { | ||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||
System.out.printf(INPUT_INEDIBLE, name); | ||||||||||||||||||||||||||||||||||||||||||||
String input = Console.readLine(); | ||||||||||||||||||||||||||||||||||||||||||||
String[] splitInput = Spliter.splitInput(input); | ||||||||||||||||||||||||||||||||||||||||||||
Validator.validateInedible(splitInput); | ||||||||||||||||||||||||||||||||||||||||||||
return Parser.parseInputToList(splitInput); | ||||||||||||||||||||||||||||||||||||||||||||
} catch (IllegalArgumentException e) { | ||||||||||||||||||||||||||||||||||||||||||||
System.out.println(e.getMessage()); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package menu.view; | ||
|
||
import java.util.List; | ||
import menu.domain.Recommend; | ||
|
||
public class OutputView { | ||
private static final String START_MSG = "점심 메뉴 추천을 시작합니다.\n"; | ||
private static final String RECOMMEND_RESULT = "메뉴 추천 결과입니다.\n"; | ||
private static final String YOIL = "[ 구분 | 월요일 | 화요일 | 수요일 | 목요일 | 금요일 ]"; | ||
private static final String CATEGORY_RESULT = "[ 카테고리 | %s | %s | %s | %s | %s ]\n"; | ||
private static final String COACH_RESULT = "[ %s | %s | %s | %s | %s | %s ]\n"; | ||
private static final String COMPLETE_RECOMMENDATION = "\n추천을 완료했습니다."; | ||
|
||
public void printStart() { | ||
System.out.println(START_MSG); | ||
} | ||
|
||
public void printResult(Recommend recommend) { | ||
System.out.println(RECOMMEND_RESULT); | ||
System.out.println(YOIL); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
printCategory(recommend); | ||
for (String coach : recommend.getCoach()) { | ||
printMenu(recommend, coach); | ||
} | ||
System.out.println(COMPLETE_RECOMMENDATION); | ||
} | ||
|
||
private void printCategory(Recommend recommend) { | ||
System.out.printf(CATEGORY_RESULT, recommend.getCategories().get(0), recommend.getCategories().get(1), | ||
recommend.getCategories().get(2), recommend.getCategories().get(3), recommend.getCategories().get(4)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0, 1, 2, 3.... 같이 직접 정수 값을 넣기보다, 각 정수값이 어떤걸 의미하는지 알려주면 더 좋을 것 같습니다. |
||
} | ||
|
||
private void printMenu(Recommend recommend, String coach) { | ||
List<String> menus = recommend.getRecommendMenu(coach); | ||
System.out.printf(COACH_RESULT, coach, menus.get(0), menus.get(1), menus.get(2), menus.get(3), menus.get(4)); | ||
} | ||
} |
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.
README.md에 어떤 기능을 구현할 지 미리 정리해봅시다!