Java baseball game 송세연 by amaran-th · Pull Request #3 · Java-Spring-Study/java-baseball-game

amaran-th · 2022-09-23T09:07:56Z

생각보다 기능 별로 매번 커밋을 날린다는 걸 준수하는 게 어렵네요...
다음 챌린지 땐 더 노력해보겠습니다!

shkisme

짧은 시간이었을텐데 구현하시느라 정말 수고하셨습니다!!!👍👍👍
클래스를 조금만 더 세밀하게 분리하고, 메서드가 하나의 역할만 하도록 수정해보는 것은 어떨까요?
테스트 코드도 한번 작성해보시면 좋을 것 같습니다😄

shkisme · 2022-09-25T11:09:06Z

baseball/src/main/java/baseball/baseball/BaseballApplication.java

+			while(true){
+				BaseballTool tool=new BaseballTool();
+				tool.makeRamdomNumbers();	//랜덤한 숫자를 생성한다.
+


소개해드렸던 formatter를 사용해보시는게 좋을 것 같습니다! 포메터 적용을 하지 않으면, 수정사항이 띄어쓰기까지 반영되면서 나중에 확인할 때 불편하다는 단점이 있습니다.
https://withhamit.tistory.com/411

shkisme · 2022-09-25T11:09:50Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+
+public class BaseballTool{
+    private Scanner sc=new Scanner(System.in);
+    private int[] solution=new int[3];


배열 대신 List를 사용해보시는 건 어떨까요? 이번 프로젝트에서는 문제 될 게 없지만, Effective java item 28을 보면, 배열보다는 리스트를 사용하라는 이야기가 나와있습니다. 책을 갖고 계시지 않을 것 같아 아래에 블로그 주소를 참고하셔서 한번 공부해보셔도 좋을 것 같아요.

https://velog.io/@new_wisdom/Effective-java-item-28.-%EB%B0%B0%EC%97%B4%EB%B3%B4%EB%8B%A4%EB%8A%94-%EB%A6%AC%EC%8A%A4%ED%8A%B8%EB%A5%BC-%EC%82%AC%EC%9A%A9%ED%95%98%EB%9D%BC

shkisme · 2022-09-25T11:16:02Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+            if(inputNums[i]==solution[i]) strike++;
+            else{
+                for(int j=0;j<this.solution.length;j++) {
+                    if (inputNums[i] == solution[j]){


프로그래밍 요구사항을 보면, 다음과 같이 나와있습니다.

indent(인덴트, 들여쓰기) depth를 3이 넘지 않도록 구현한다. 2까지만 허용한다.

예를 들어 while문 안에 if문이 있으면 들여쓰기는 2이다.

힌트: indent(인덴트, 들여쓰기) depth를 줄이는 좋은 방법은 함수(또는 메소드)를 분리하면 된다.

클래스를 분리하고, 메서드를 역할에 따라 나눠보시다 보면 해결될 문제같습니다!

shkisme · 2022-09-25T11:20:40Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+    }
+
+    public void makeRamdomNumbers(){
+        Random random=new Random();


메서드를 호출할 때마다 Random 클래스를 생성할 필요가 없지 않을까요? Scanner 처럼 두는 것도 좋아 보입니다.

shkisme · 2022-09-25T11:23:16Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+import java.util.Scanner;
+
+public class BaseballTool{
+    private Scanner sc=new Scanner(System.in);


sc와 같은 필드들은 선언되고 값이 수정될 필요가 없으니, final로 선언해주는 것은 어떨까요?

shkisme · 2022-09-25T11:27:06Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+        }
+    }
+    public boolean checkStrike(){
+        if(this.strike==3) {


checkStrike 메서드는 아래와 같이 코드를 줄일 수 있을 것 같습니다!

return strike == 3;

shkisme · 2022-09-25T11:30:05Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+        for(int i=0;i<this.inputNums.length;i++){
+            if(inputNums[i]==solution[i]) strike++;
+            else{
+                for(int j=0;j<this.solution.length;j++) {


여기에서는 enhanced for loop으로 코드를 작성해보는 것은 어떨까요?

for (int k : this.solution) { if (inputNums[i] == k) { ball++; break; } }

코드가 좀 더 깔끔해질 것 같은데 어떠신가요?

shkisme · 2022-09-25T11:33:09Z

baseball/src/main/java/baseball/baseball/BaseballApplication.java

+					tool.calcResult();		//게임 결과를 계산한다.
+					tool.printResult();		//게임 결과를 출력한다.
+				}
+				if(tool.getSignalCode()==2) break;


종료 조건이 이렇게 명확한 경우에 do-while 문으로 코드를 작성해 볼 수 있지 않을까요?

shkisme · 2022-09-25T11:34:10Z

baseball/src/main/java/baseball/baseball/BaseballApplication.java

+	public static void main(String[] args) {
+		try{
+			while(true){
+				BaseballTool tool=new BaseballTool();


마찬가지로 while 문을 돌 때마다 BaseballTool을 새로 생성할 필요가 있을까요?

shkisme · 2022-09-25T11:34:50Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+    public void makeRamdomNumbers(){
+        Random random=new Random();
+        for(int i=0;i<3;i++) {
+            this.solution[i] = random.nextInt(9) + 1;


this. 로 필드 값을 가져오지 않아도 되지 않나요? this.로 값을 가져오신 이유가 따로 있나요? 궁금합니다!

- Random() 인스턴스를 멤버변수화 - 변경되지 않는 멤버변수에 final 키워드 추가 - do-while문으로 수정 - 메서드 간략화

fienestar · 2022-09-29T04:35:10Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+    }
+  }
+
+  public boolean checkStrike() {


메서드 이름이 명확하진 않은 것 같아요. 어떤걸 체크하는지 이름에서 나타나면 좋을 것 같아요

fienestar · 2022-09-29T04:39:26Z

baseball/src/main/java/baseball/baseball/BaseballApplication.java

+  public static void main(String[] args) {
+    BaseballGame game=new BaseballGame();
+    try {
+      while(game.tryGame() != 2);


tryGame이 int를 반환하는것과 그걸 2와 비교하는 것이 어떤 의미인지 파악하기 어려웠어요. enum을 만들면 좀 더 명확하게 표현할 수 있지 않을까요?

저도 말씀드리고 싶었던 부분이였습니다! 아래처럼 enum을 생성해서 이것을 이용하면 가독성을 높일 수 있을 것 같습니다.

enum Signal{ END(2); private int signal; Signal(int signal) { this.signal= signal; } public int getSignal() { return this.signal; } }

fienestar · 2022-09-29T04:43:23Z

baseball/src/main/java/baseball/tool/BaseballTool.java

+    int num;
+    if (inputString != null && inputString.matches("[1-9]+")) {
+      num = Integer.parseInt(inputString);
+      if (num < 1000 && num > 99) {


matches를 [1-9]{3}으로 검사하면 num에 대한 검사를 안해도 괜찮지 않을까요?

- 메서드 이름 명확하게 변경 - ENUM 사용하여 가독성 향상 - 정규식 수정하여 코드 단순화

shkisme

굿! 수고하셨습니다! 자바 챌린지 2로 넘어가셔도 좋을 것 같아요!
머지는 제가 나중에 한꺼번에 진행하겠습니다.

amaran-th added 4 commits September 19, 2022 00:27

test : init

1c6eec4

init : README.md 파일 작성

cdea2dd

init : README.md 파일 최종

6c0e173

feat : 프로젝트 구현

907935c

shkisme assigned amaran-th Sep 23, 2022

shkisme self-requested a review September 23, 2022 15:37

shkisme reviewed Sep 25, 2022

View reviewed changes

amaran-th added 5 commits September 26, 2022 21:19

refactor : 코드스타일 설정

e563dbd

refactor : 배열을 List로 변경, 불필요한 this 제거

46f3a6c

refactor

660d049

- Random() 인스턴스를 멤버변수화 - 변경되지 않는 멤버변수에 final 키워드 추가 - do-while문으로 수정 - 메서드 간략화

refactor : enhanced for loop로 코드 작성

4243032

refactor : 클래스 분리 및 메서드 세분화

63b6931

shkisme requested review from fienestar and hdh4952 September 28, 2022 09:50

fienestar reviewed Sep 29, 2022

View reviewed changes

refactor : 코드리뷰 반영

177084f

- 메서드 이름 명확하게 변경 - ENUM 사용하여 가독성 향상 - 정규식 수정하여 코드 단순화

shkisme approved these changes Oct 28, 2022

View reviewed changes

chore: 폴더명 변경

c0fb946

Conversation

amaran-th commented Sep 23, 2022

Uh oh!

shkisme left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

shkisme Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shkisme left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shkisme left a comment •

edited

Loading

shkisme Sep 29, 2022 •

edited

Loading