숫자야구 구현 #2

kimikhyeon1 · 2023-11-09T05:44:18Z

No description provided.

…baseball-playground into ikhyeon-branch

ca1af · 2023-11-10T00:55:54Z

src/main/java/view/InputView.java

+
+public class InputView {
+
+    BufferedReader in = new BufferedReader(new InputStreamReader(System.in));


접근제어자 및 final 키워드를 사용하는 것은 어떨까요?

적절한 접근제어자에 대한 중요성을 고민해보겠습니다!

ca1af · 2023-11-10T00:57:56Z

src/main/java/view/InputView.java

+    private final ValidationInput validationInput;
+
+    public InputView() {
+        validationInput = new ValidationInput();
+    }


BufferedReader 는 필드단에서 new 하셨는데, ValidationInput 클래스는 생성자로 new 하신 이유가 궁금합니당!

더하여 클래스, 매서드 등의 네이밍 컨벤션 링크 글을 공유드립니다!

음.. 크게 의미가 없는 것 같습니다. 통일성 있는 코드를 고민해보겠습니다!

ca1af · 2023-11-10T01:03:24Z

src/main/java/baseball/model/Computer.java

+
+        for (int i = 0; i < userNumber.length(); i++) {
+            int digit = Character.getNumericValue(userNumber.charAt(i));
+            Integer answerValue = answer.get(digit);


컴퓨터 측에서는 항상 유저의 "digit" 에 해당하는 키를 가지고 있을까요?

혹시 가지고 있지 않을 가능성이 있다면, if (containsKey(digit)) 정도의 로직이 필요할 것 같습니다!

바로 아래 로직에 NULL 인지 판별하는 조건을 넣었는데, 이보단 containsKey를 사용하는게 더 명확할 것 같네요 감사합니다!

ca1af

너무 멋집니다!! 생소한 구현이었을텐데, 너무 대단합니다 ㅎㅎ

이번 구현에는 없던 제약사항이지만, 다음 구현부터는 다음과 같은 제약 사항이 추가됩니다.

매서드 라인은 15줄을 넘겨선 안된다
인덴트 (들여쓰기) depth 는 2를 초과해선 안된다 (ex : for 문 안에 if 가 있으면 2)

해당 제약 사항을 지키는 방향으로 리팩토링 해보는 것도 좋은 경험이 될 것 같습니다!

Hint : 매서드 라인을 줄이기 위해서는, 또 인덴트를 줄이기 위해서는 매서드를 분리하는 방법을 사용하면 됩니다 (ctrl + alt + M)

deok-beom · 2023-11-10T02:13:31Z

src/main/java/global/ValidationInput.java

+    public boolean isEmpty(String input) {
+        if (input.isEmpty() || input.contains(" ")) {
+            System.out.println("공백을 입력하였습니다.");
+            return false;
+        }
+        return true;
+    }


isEmpty(String input) 메서드는 input 값이 비어있을 때 true가 반환되길 기대할 것 같습니다.
함수의 이름이 isNotEmpty()가 되거나 비어 있을 때 true가 반환되는 것이 좋아보입니다.

마찬가지로 같은 클래스 내의 isContainsZero(), isDuplicate()도 좀 더 직관적인 이름이 될 수 있을 것 같습니다.

앞으로 메서드명과 리턴값에 대한 관계까지 고려하여 설계하겠습니다!

deok-beom · 2023-11-10T03:29:00Z

src/main/java/baseball/controller/BaseballGame.java

+        while (true) {
+            HashMap<Integer, Integer> answer = computer.getAnswer();
+
+            while (true) {
+                String userNumber = inputView.validationNumber();
+                if (computer.validationStrikeAndBall(answer, userNumber)) {
+                    break;
+                }
+            }


validationStrikeAndBall(HashMap<Integer, Integer> *answer*, String userName) 메소드 파라미터의 answer가 결국에는 computer 인스턴스 자신이 갖고 있는 answer 변수를 그대로 사용하는 것 같습니다. 그렇다면 파라미터를 통해 answer를 전달 받을 필요 없이 자기 자신의 멤버 변수를 바로 참조하는 것이 좋을 것 같습니다.

생각해보니 play에서 answer값을 들고 있을 필요가 없었네요, 좋은 지적 감사합니다!

deok-beom · 2023-11-10T04:07:30Z

src/main/java/baseball/model/Computer.java

+        if (strike == 0 && ball == 0) {
+            System.out.println("낫씽");
+        }


아래의 나머지 조건들을 볼 필요도 없는 경우에는 메소드를 더 일찍 종료한다면 성능적으로나 가독성 부분에서 이득을 볼 수 있을 것 같습니다.

위 아이디어를 바탕으로 이 메소드를 아래와 같은 흐름으로 고칠 수 있습니다.

strike, ball 이 모두 0이라면 메세지를 출력하고 false 리턴

strike, ball 이 모두 0보다 크다면 메세지를 출력하고 false 리턴

ball이 0보다 크다면 메세지를 출력하고 false 리턴

strike 의 개수를 출력하고 true 리턴

위 흐름으로 고친다면 if문의 개수는 3개로 줄고 앞서 말한 아이디어처럼 조기에 메소드를 마무리 할 수도 있으면서 로직과 출력문을 그대로 가져가는데 무리 없을 겁니다.
이와 같은 작성이 가지는 장점이 있습니다. 컴퓨터가 판단해야 하는 조건문이 줄어들면서 트랜지스터를 절약할 수 있고, 읽는 사람도 if문에 잡혔을 때 바로 메소드가 false를 반환하며 종료하기 때문에 이후에 조건들을 고려하지 않아도 돼서 어떤 조건에 걸려 메소드가 false를 반환했는지 생각하기가 더 쉬울 겁니다.

추가로, 메소드를 읽을 때 작성하는 사람은 위에서부터 아래로 꼼꼼하게 읽기를 바라지만 읽는 사람은 빨리 원하는 부분만을 알고 싶어하는 경우가 있을 수도 있습니다. 예를 들어 예외를 발생시키며 메소드를 종료 시키는 중간의 여러 조건문은 고려하지 않고 마지막에 반환되는 결과를 빠르게 보고 싶을 수도 있겠죠.

본문에서 설명한 흐름의 4번에서 strike의 개수를 출력하고 true를 리턴하는 게 아니라, 작성하신 것처럼 마지막에 return strike == 3으로 작성해놓는 다면 성능적인 측면에서는 불필요한 조건문이 하나 늘어나는 것이기 때문에 더 안좋다고 말할 수는 있으나, 가독성 측면에서는 훨씬 뛰어다나고 볼 수 있습니다. (이와 같은 작성 흐름을 얼추 이해하는 사람이라면 메소드 가장 마지막 return 되는 조건문만 읽고 어떤 조건에서 true가 반환되는 지 알 수 있을 테니까요)

deok-beom · 2023-11-10T04:19:00Z

src/main/java/baseball/model/Computer.java

+        while (answer.size() < NUMBER_SPACE_SIZE) {
+            int randomNumber = getRandomNumber();
+            if (answer.containsKey(randomNumber)) {
+                continue;
+            }
+            answer.put(randomNumber, answer.size() + 1);
+        }


저도 작성 초기에 이와 같은 방법으로 난수 클래스와 컬렉션을 사용했으나 이 로직에는 치명적인(?) 단점이 하나 있습니다. 바로 난수로 뽑아낸 값이 계속 같은 값을 뱉었을 때 조건문이 이론상 무한히 반복될 수도 있다는 겁니다.

그래서 저는 이전 리뷰에서 이야기를 나눴던 List에 값을 넣고 무작위로 셔플하는 방법을 사용했습니다.

혹시 모를 무한루프를 예방하는것도 좋은 습관일 수 있겠다고 생각이 드네요, 감사합니다.

deok-beom · 2023-11-10T04:24:10Z

src/main/java/global/ValidationInput.java

+    public boolean isValidationUserNumber(String input) {
+        if (!isEmpty(input) || !isValidationLength(input) || !isContainsZero(input) || !isDuplicate(
+            input)) {
+            return false;
+        }
+        return true;
+    }


여러 개의 조건문과 복잡한 요구사항이 있는 검증 로직 같은 경우에는 작성하신 방법처럼 조건문 들을 메소드로 빼고 하나의 if문처럼 보이도록 한 것이 한 메소드 안에서 여러개의 if문을 거치게 하고 복잡한 검증 로직을 노출하는 것보다 더 좋은 방법인 것 같습니다. 👍

저도 그렇게 생각합니다. if문의 검증로직이 길어질때마다 항상 고민했었는데 이렇게 쓰면 된다는것을 또 배워갑니다...! 😀

일급컬렉션에 대한 개념이 아직 부족해서 위와 같이 처리하였는데 좋은 방법이라고 해주셔서 감사합니다!

minseon33 · 2023-11-13T07:39:09Z

src/main/java/baseball/controller/BaseballGame.java

+    public void play() throws IOException {
+
+        while (true) {
+            HashMap<Integer, Integer> answer = computer.getAnswer();


validationStrikeAndBall(answer, userNumber)의 answer은 컴퓨터에서 가져온 map인데 왜 2중 while문을 사용해서 또다시 넣어주는지 궁금합니다..! 첫번째 �while문을 빼고 밑의 while문만 사용해도 괜찮지 않을까요..? validationStrikeAndBall(answer, userNumber)에서 answer은 어차피 컴퓨터가 갖고있는 필드인데..

첫 번째 while문은 게임을 시작하고 게임 종료 후 재시작에 대한 루프,
두 번째 while문은 게임을 진행하고 맞출 때까지의 루프였습니다.
answer에 대해서는 굳이 play메서드로 전달 할 필요가 없었네요 깊게 생각하지 못한 것 같습니다. 감사합니다!

minseon33 · 2023-11-13T07:55:40Z

src/main/java/global/ValidationInput.java

+    public boolean isValidationUserNumber(String input) {
+        if (!isEmpty(input) || !isValidationLength(input) || !isContainsZero(input) || !isDuplicate(
+            input)) {
+            return false;
+        }
+        return true;
+    }


저도 그렇게 생각합니다. if문의 검증로직이 길어질때마다 항상 고민했었는데 이렇게 쓰면 된다는것을 또 배워갑니다...! 😀

minseon33 · 2023-11-13T08:01:13Z

src/main/java/global/ValidationInput.java

+    }
+
+}
+


유저가 입력한 input값에 대한 여러가지 예외 상황을 고민한 모습이 멋집니다.

Ikhyeon-Kim and others added 9 commits November 7, 2023 10:24

feat: hardcoding to main

8c3a299

feat: add output data

1a601a3

refactor: change number type to String

07693d9

feat: mvc패턴 적용

8c1c5e5

feat: 메서드 분리

afc08f4

refac: google style convention & method refactoring

80417b0

refac: remove test method

f2cf53f

refac: remove test method

4e5981e

Merge branch 'ikhyeon-branch' of https://github.com/kimikhyeon1/java-…

215a22a

…baseball-playground into ikhyeon-branch

ca1af reviewed Nov 10, 2023

View reviewed changes

deok-beom reviewed Nov 12, 2023

View reviewed changes

minseon33 reviewed Nov 13, 2023

View reviewed changes


		public class InputView {

		BufferedReader in = new BufferedReader(new InputStreamReader(System.in));

숫자야구 구현 #2

Are you sure you want to change the base?

숫자야구 구현 #2

Uh oh!

Conversation

kimikhyeon1 commented Nov 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ca1af left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants