-
Notifications
You must be signed in to change notification settings - Fork 2
[Step1]리뷰 요청 드립니다! #5
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
base: kimjinwook
Are you sure you want to change the base?
Changes from all commits
36c50b4
66f64ac
a400529
0a2d0ad
b07dbf5
2fe5655
8310f2b
431107a
f3b7db5
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 |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package baseball; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class Computer { | ||
|
|
||
| public List<Integer> makeNumbers() { | ||
| RandomNumber randomNumber = new RandomNumber(); | ||
|
|
||
| ListUtil listUtil = new ListUtil(); | ||
|
|
||
| List<Integer> computerNumbers = new ArrayList<>(); | ||
|
|
||
| while (!listUtil.checkListSize(computerNumbers)) { | ||
| int number = randomNumber.make(); | ||
| listUtil.distinctNumberAdd(computerNumbers, number); | ||
| } | ||
|
|
||
| System.out.println("computerNumbers = " + computerNumbers); | ||
| return computerNumbers; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package baseball; | ||
|
|
||
| public class Game { | ||
| public static void main(String[] args) { | ||
| GameUtil gameUtil = new GameUtil(); | ||
| gameUtil.runGame(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| package baseball; | ||
|
|
||
|
|
||
| import java.util.List; | ||
|
|
||
| public class GameUtil { | ||
|
|
||
| public void runGame() { | ||
|
|
||
| if (runOnlyOnceGame()) { | ||
| showMenu(); | ||
| } | ||
| } | ||
|
|
||
| public boolean runOnlyOnceGame() { | ||
|
|
||
| Computer computer = new Computer(); | ||
| List<Integer> computers = computer.makeNumbers(); | ||
|
|
||
| return gameResult(computers); | ||
| } | ||
|
|
||
| private boolean gameResult(List<Integer> computers) { | ||
| boolean threeStrike = true; | ||
| while (threeStrike) { | ||
|
|
||
| Player player = new Player(); | ||
| List<Integer> playerNumbers = player.makeNumbers(); | ||
| Referee referee = new Referee(); | ||
|
|
||
| String result = referee.informStrikeBall(computers, playerNumbers); | ||
| System.out.println(result); | ||
| threeStrike = isGame(threeStrike, result); | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private boolean isGame(boolean game, String result) { | ||
| if (result.equals("3스트라이크")) { | ||
| System.out.println("3개의 숫자를 모두 맞히셨습니다! 게임 종료"); | ||
| game = false; | ||
| } | ||
| return game; | ||
| } | ||
|
|
||
| private void showMenu() { | ||
| ScannerUtil scannerUtil = new ScannerUtil(); | ||
| System.out.println("게임을 새로 시작하려면 1, 종료하려면 2를 입력하세요."); | ||
|
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.
프로그래밍 요구사항 중에 위와 같은 요구사항이 있었는데요. 이 부분을 어떻게 구현할지 생각해보시면 좋을 것 같습니다. |
||
|
|
||
| int choice = scannerUtil.insertInt(); | ||
| if (choice == 1) { | ||
| runGame(); | ||
| } | ||
|
|
||
| while (choice != 1 && choice != 2) { | ||
| System.out.println("잘못 입력하셨습니다. 다시입력해주세요."); | ||
| choice = scannerUtil.insertInt(); | ||
| } | ||
| System.out.println("수고하셨습니다."); | ||
| } | ||
| } | ||
|
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. 파일 마지막에 엔터(개행문자)를 넣어주세요 :) 이유는 리뷰를 진행할 때 깃허브에서 경고메시지를 지우고 혹시 모르는 파일 읽기 오류에 대비하기 위함입니다. 좀 더 알고싶으시면 참고 링크를 보셔도 재밌을 것 같네요 :) Intellij 를 사용하실 경우엔Preferences -> Editor -> General -> Ensure line feed at file end on save 를 체크해주시면파일 저장 시 마지막에 개행문자를 자동으로 넣어줍니다! |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package baseball; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class ListUtil { | ||
|
|
||
| public boolean checkListSize(List<Integer> list) { | ||
| return list.size() == 3; | ||
| } | ||
|
|
||
| public void distinctNumberAdd(List<Integer> numberList, int randomNumber) { | ||
| if (!numberList.contains(randomNumber)) { | ||
| numberList.add(randomNumber); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package baseball; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class Player { | ||
|
|
||
| public List<Integer> makeNumbers() { | ||
|
|
||
| ScannerUtil scannerUtil = new ScannerUtil(); | ||
|
|
||
| List<Integer> integers = scannerUtil.makeScannerNumbers(); | ||
|
|
||
| return integers; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package baseball; | ||
|
|
||
| import java.util.Random; | ||
|
|
||
| public class RandomNumber { | ||
|
|
||
| public int make() { | ||
| Random random = new Random(); | ||
| return random.nextInt(9) + 1; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package baseball; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class Referee { | ||
|
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. Referee 클래스의 인스턴스는 어떤 상태(내부 필드)도 가지지 않기 때문에 이 클래스의 모든 객체들은 동일한 객체이겠네요 ! 너무 많은 상태를 가지는 것은 좋은 설계가 아니지만, 아무것도 가지지 않는 방식 또한 바람직한 설계는 아닙니다. 적절한 상태를 가진 객체를 설계해보면 좋을 것 같아요. 다른 클래스들에도 함께 적용되는 피드백일 것 같습니다 :) |
||
|
|
||
| public int countBall(List<Integer> list1, List<Integer> list2) { | ||
|
|
||
| int count = 0; | ||
| int strike = countStrike(list1, list2); | ||
|
|
||
| for (Integer number : list1) { | ||
| count = getCount(list2, count, number); | ||
| } | ||
| return count - strike; | ||
| } | ||
|
|
||
| public int countStrike(List<Integer> list1, List<Integer> list2) { | ||
|
|
||
| int count = 0; | ||
| for (int index = 0; index < 3; index++) { | ||
|
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. 임의의 숫자를 사용하는 매직넘버는소스 코드를 읽기 어렵게 만드는데요 ! 이 부분을 어떻게 개선할 수 있을까요? 관련 링크 남겨놓겠습니다 :) |
||
| count = getCountStrike(list1, list2, count, index); | ||
| } | ||
| return count; | ||
| } | ||
|
|
||
| public String informStrikeBall(List<Integer> list1, List<Integer> list2) { | ||
|
|
||
| int ball = countBall(list1, list2); | ||
| int strike = countStrike(list1, list2); | ||
|
|
||
| if (strike == 3) { | ||
| return strike + "스트라이크"; | ||
| } | ||
|
Comment on lines
+32
to
+34
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. 핵심 로직에 화면(콘솔 출력)에 대한 요구사항이 섞여있네요!
|
||
|
|
||
| if (strike == 0 && ball != 0) { | ||
| return ball + "볼"; | ||
| } | ||
|
|
||
| if (strike != 0 && ball != 0) { | ||
| return ball + "볼 " + strike + "스트라이크"; | ||
| } | ||
|
|
||
| return "낫싱"; | ||
| } | ||
|
|
||
| private int getCountStrike(List<Integer> list1, List<Integer> list2, int count, int index) { | ||
| if (list1.get(index) == list2.get(index)) { | ||
| count++; | ||
| } | ||
| return count; | ||
| } | ||
|
|
||
| private int getCount(List<Integer> list2, int count, Integer number) { | ||
| if (list2.contains(number)) { | ||
| count++; | ||
| } | ||
| return count; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package baseball; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Scanner; | ||
|
|
||
| public class ScannerUtil { | ||
|
|
||
| public List<Integer> makeScannerNumbers() { | ||
| System.out.print("숫자를 입력하세요: "); | ||
|
|
||
| ListUtil listUtil = new ListUtil(); | ||
|
|
||
| List<Integer> integers = stringListToIntegerList(); | ||
|
|
||
| while (!listUtil.checkListSize(integers)) { | ||
| System.out.println("잘못된 숫자입니다."); | ||
| System.out.print("숫자를 입력하세요: "); | ||
|
|
||
| integers = stringListToIntegerList(); | ||
| } | ||
| return integers; | ||
| } | ||
|
|
||
| public String insertString() { | ||
|
|
||
| Scanner scanner = new Scanner(System.in); | ||
|
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. Scanner는 매우 비싼 자원입니다. 미리 생성해두고 재활용하도록 해보면 어떨까요 ? static 메서드를 활용해보면 좋을 것 같아요. |
||
| return scanner.next(); | ||
| } | ||
|
|
||
| public int insertInt() { | ||
|
|
||
| Scanner scanner = new Scanner(System.in); | ||
| return scanner.nextInt(); | ||
| } | ||
|
|
||
| private String[] splitStringList() { | ||
| String string = insertString(); | ||
|
|
||
| return string.split(""); | ||
| } | ||
|
|
||
| private List<Integer> stringListToIntegerList() { | ||
| String[] strings = splitStringList(); | ||
|
|
||
| List<Integer> temp = new ArrayList<>(); | ||
| ListUtil listUtil = new ListUtil(); | ||
|
|
||
| for (String string : strings) { | ||
| int number = Integer.parseInt(string); | ||
| listUtil.distinctNumberAdd(temp, number); | ||
| } | ||
|
|
||
| return temp; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package baseball; | ||
|
|
||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class ListUtilTest { | ||
|
|
||
| @Test | ||
| @DisplayName("리스트의 사이즈가 3이면 true 반환") | ||
| void checkListSize() throws Exception { | ||
| //given | ||
| ListUtil listUtil = new ListUtil(); | ||
|
|
||
| List<Integer> list = new ArrayList<>(); | ||
| list.add(1); | ||
| list.add(2); | ||
| list.add(3); | ||
|
|
||
| //when | ||
| boolean checkListSize = listUtil.checkListSize(list); | ||
|
|
||
| //then | ||
| assertThat(checkListSize).isTrue(); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("리스트에 중복 숫자가 들어갈 경우 리스트에 add 안함") | ||
| void checkDuplicate() throws Exception { | ||
| //given | ||
| ListUtil listUtil = new ListUtil(); | ||
|
|
||
| int number = 1; | ||
| List<Integer> list = new ArrayList<>(); | ||
|
|
||
| //when | ||
| for (int i = 0; i < 2; i++) { | ||
| listUtil.distinctNumberAdd(list, number); | ||
| } | ||
| //then | ||
| assertThat(list.size()).isEqualTo(1); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package baseball; | ||
|
|
||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class RandomNumberTest { | ||
|
|
||
| @Test | ||
| @DisplayName("랜덤 넘버 만들기 테스트 -> 숫자 범위가 1~9 사이") | ||
| void randomNumberMakeTest() throws Exception { | ||
| //given | ||
| RandomNumber number = new RandomNumber(); | ||
|
|
||
| //when | ||
| int randomNUmber = number.make(); | ||
|
|
||
| //then | ||
| assertThat(randomNUmber).isBetween(1, 9); | ||
| } | ||
| } |
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.
Util 이라는 이름의 클래스는 적합하지 않은 것 같아요 :)
유틸 클래스는 보통 실용성이나 유용성에 중점을 둔 정적 메서드들을 모아두고 여러 곳에서 호출하는 형식으로 사용하며 여러 곳에서 호출하기 때문에 유틸 클래스는 공통적인 로직을 다루도록 만들어야하며 합니다.개인적으로는 실제로는 이러한 유틸 클래스의 메서드를 변경했을 때 영향이 너무 크기 때문에 최대한 사용을 지양하는 것을 선호합니다 :)