Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
### 기능 목록

- 컴퓨터 랜덤 숫자 3개 만들기
- 중복되는 숫자 있는지 체크, 있을 경우 다시 생성



- 게임 시작
- 플레이어 3개 숫자 입력하기
- 자릿수 계산하기
- 모두 맞지 않았다면 결과를 출력하고 다시 플레이어 숫자 입력 받음
- 모두 맞췄다면 성공 메시지 출력함
- 게임을 새로 시작할지 여부에 따른 분기 처리

8 changes: 8 additions & 0 deletions src/main/java/Main.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import controller.BaseballConsole;

public class Main {
public static void main(String[] args) {
BaseballConsole baseballConsole = new BaseballConsole();
baseballConsole.start();
}
}
11 changes: 11 additions & 0 deletions src/main/java/constant/Message.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package constant;

public final class Message {
public static final String INPUT_NUMBER_MESSAGE = "숫자를 입력해주세요 : ";
public static final String INCORRECT_DIGIT_MESSAGE = "세 자리 수를 입력해야 합니다.";
public static final String DUPLICATE_DIGIT_MESSAGE = "중복되는 수가 존재합니다.";
public static final String STRIKE_MESSAGE = " 스트라이크 ";
public static final String BALL_MESSAGE = "볼";
public static final String NO_STRIKE_NO_BALL_MESSAGE = "낫싱";
public static final String SUCCESS_MESSAGE = "3개의 숫자를 모두 맞히셨습니다! 게임 종료" + System.lineSeparator() + "게임을 새로 시작하려면 1, 종료하려면 2를 입력하세요.";
Comment on lines +4 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메시지들 상수 분리! 👍

}
75 changes: 75 additions & 0 deletions src/main/java/controller/BaseballConsole.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package controller;
import constant.Message;
import domain.Computer;
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java 코딩 컨벤션에 따르면 package문과 import문은 한칸 공백이 있어야 합니다.
IDE의 자동 포매팅 기능을 활용하면 기본적인 포매팅문제는 해결할 수 있을거에요!
(인텔리제이 , 맥 기준 command + option + L 단축키를 활용하시면 됩니다!)

import domain.Result;
import domain.User;
import service.BaseballService;

import java.util.Scanner;

public class BaseballConsole {
private Scanner scanner;
private Computer computer;
private User user;
private Result result;
private BaseballService baseballService;

Comment on lines +11 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

하나의 클래스에서 최대한 필드수를 최소화 하는것을 권장하고 있어요.
필드수가 많다는 것은 그만큼 해당 클래스가 수행하는 역할이 많아진 다는 것이고 이는 클래스 분리의 신호라는 뜻이에요!

그런데 해당 클래스에서 과연 User, Result, Computer, Scanner 등의 필드가 필수적으로 필요할까요?
다시한번 고민해보시고 필드가 가벼운 클래스로 만들어보시면 좋을것 같아요.

public BaseballConsole() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 클래스는 컨트롤러 역할을 하고 있네요.
MVC 패턴을 기반으로 프로그램을 만들기 위해 시도했군요! 👍
그런데 지금의 컨트롤러 클래스는 컨트롤러 (프로그램 흐름 제어)와 뷰(외부 입출력) 두가지를 모두 수행하는 무거운 클래스로 보이네요.

단일 책임의 원칙 관점에서 컨트롤러에서 뷰의 역할을 수행하는 기능들을 별도의 View 클래스로 분리하면 보다 가벼운 컨트롤러로 만들 수 있을 것 같아요!

this.scanner = new Scanner(System.in);
this.computer = new Computer();
this.user = new User();
this.result = new Result();
this.baseballService = new BaseballService();
}

public void start() {
while (true) {
baseballService.setRandomNumber(computer);

enterNumber();
selectMenu();
}
}

public void enterNumber() {
while (true) {
try {
System.out.print(Message.INPUT_NUMBER_MESSAGE);
user.inputNumberToString(scanner);
Comment on lines +37 to +38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 콘솔을 통한 외부 입력이 수행되는 부분인데요, 외부 입력을 책임지는 View 클래스를 부여하고 view에게 역할을 위임하면 좋을 것 같아요. 지금의 구조에서는 View의 역할을 컨트롤러와 모델(User 도메인)까지 수행하게 되어서 MVC 의 역할분리가 제대로 이뤄지지 않고 있어요.

result = baseballService.getGameResult(computer, user);
printGameResult(result);
if (result.isThreeStrike()) {
break;
}
} catch (Exception e) {
System.out.println(e.getMessage());
}
}
}

private void selectMenu() {
System.out.println(Message.SUCCESS_MESSAGE);

int selectMenuNum = scanner.nextInt();

switch (selectMenuNum) {
case 1 :
break;
case 2 :
System.exit(0);
}
}

private void printGameResult(Result result) {
int strike = result.getStrike();
int ball = result.getBall();

if (strike > 0)
System.out.print(strike + Message.STRIKE_MESSAGE);
if (ball > 0)
System.out.print(ball + Message.BALL_MESSAGE);
if (strike == 0 && ball == 0)
System.out.print(Message.NO_STRIKE_NO_BALL_MESSAGE);
Comment on lines +67 to +72
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java 컨벤션에 따르면 한줄짜리 if 문이더라도 중괄호문을 생략하지 않는 것을 권장하고 있어요.
중괄호문을 생략할 수 있는 조건문이더라도 중괄호문을 사용해주세요!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당하는 기능 같이 출력을 책임지는 View 클래스 등으로 분리한 뒤 controller는 출력의 역할을 View 클래스에게 위임하면 controller가 가벼워지고 보다 응집력 있는 객체가 될 수 있을것 같아요!

System.out.println("");
}
}
19 changes: 19 additions & 0 deletions src/main/java/domain/Computer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package domain;

public class Computer {
private String computerNum;

public void makeRandomNumberToString(int randomNumber) {
this.computerNum = String.valueOf(randomNumber);
}

public boolean notExistsDuplicateDigit() {
return (computerNum.charAt(0) != computerNum.charAt(1)) &&
(computerNum.charAt(0) != computerNum.charAt(2)) &&
(computerNum.charAt(1) != computerNum.charAt(2));
}

public String getComputerNum() {
return computerNum;
}
}
39 changes: 39 additions & 0 deletions src/main/java/domain/Result.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package domain;

public class Result {
private int strike;
private int ball;

public Result() {
this.strike = 0;
this.ball = 0;
}

public void plusStrike() {
this.strike++;
}

public void plusBall() {
this.ball++;
}

public int getStrike() {
return strike;
}

public void setStrike(int strike) {
this.strike = strike;
}

public int getBall() {
return ball;
}

public void setBall(int ball) {
this.ball = ball;
}
Comment on lines +24 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용하지 않는 setter들은 제거해주세요.
더불어서 setter는 실제로 사용하는 것을 지양하고 있어요. setXXX 라는 이름으로는 해당 메서드로 무엇을 하려는지 의도를 파악하기 힘들기 때문이죠.


public boolean isThreeStrike() {
return this.strike == 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위와 같이 3이라는 값을 직접 하드코딩한 값을 매직넘버라 합니다. 매직넘버를 사용하는 경우, 프로그램 규모가 조금만 커져도 해당 매직넘버 의미를 파악하기 힘들어지며, 값을 변경해야하는 경우에는 모든 값을 하나하나 바꿔주다가 실수로 변경을 누락하는 경우도 생길 수 있겠죠?

매직넘버들은 특정한 의미를 가진 상수로 분리하는 것이 바람직해보여요!

}
}
33 changes: 33 additions & 0 deletions src/main/java/domain/User.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package domain;

import constant.Message;

import java.util.Scanner;

public class User {
private String userNum;

public void inputNumberToString(Scanner scanner) throws Exception {
this.userNum = String.valueOf(scanner.nextInt());

if (notNumberLengthIsThree()) {
throw new Exception(Message.INCORRECT_DIGIT_MESSAGE);
} else if (existsDuplicateDigit()) {
throw new Exception(Message.DUPLICATE_DIGIT_MESSAGE);
}
}
Comment on lines +10 to +18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SOLID 5원칙중 단일 책임원칙에 따르면 하나의 객체의 변경 포인트는 한가지만 존재해야 하는 것이 바람직합니다.
변경 포인트가 한가지여야 한다는 것을 다시 해석하면 객체들은 한가지 역할만 수행해야 한다는 의미입니다.

도메인 객체는 주요 비즈니스 로직중 한가지 기능에 대해서만 책임지는 것이 바람직합니다.
현재 User는 비즈니스 로직 외 Scanner를 사용한 외부 입력에 대해서도 책임지고 있어요.
이러한 구조는 단일책임원칙에 위배되는 구조이죠!

예를 들어, 현재는 게임에 필요한 숫자 값을 콘솔을 통해 입력 받고 있어요.
그런데 요구사항이 변경되어 숫자 값에 대한 입력을 파일, 혹은 웹 요청을 통한 입력받아야 한다면 그때마다 User를 변경해야겠죠?

User를 포함한 도메인 객체는 비즈니스 로직이 변경되는 경우에만 변경이 일어나야 합니다.

따라서 User에서 외부 입출력부분을 책임지는 대신 입출력을 책임지는 객체를 별도로 분리하는 것이 바람직해 보여요.
이는 MVC 패턴을 생각하면 더 이해가 쉬울것 같아요.


public boolean notNumberLengthIsThree() {
return this.userNum.length() != 3;
}

public boolean existsDuplicateDigit() {
return (userNum.charAt(0) == userNum.charAt(1)) ||
(userNum.charAt(0) == userNum.charAt(2)) ||
(userNum.charAt(1) == userNum.charAt(2));
}

public String getUserNum() {
return userNum;
}
}
Empty file removed src/main/java/empty.txt
Empty file.
60 changes: 60 additions & 0 deletions src/main/java/service/BaseballService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package service;

import domain.Computer;
import domain.Result;
import domain.User;

import java.util.Random;

public class BaseballService {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서비스를 사용하셨네요!
그런데 과연 서비스에서 수행되는 로직들이 정말 서비스에서 수행해야 하는지 의문이 들어요.

보다 객체지향적인 프로그래밍을 위해서 객체가 가지고 있는 상태를 기반으로 스스로 행동하는 응집도 높은 객체를 만들어 줘야합니다!

컴퓨터의 랜덤값 부여의 경우 컴퓨터 객체가, 결과를 계산하는 로직은 Result 객체가 직접 책임지는 것이 보다 응집도 있는 Computer와 Result 객체를 만들 수 있다 생각 드네요.

이부분에 대한 의견이 궁금합니다.

public void setRandomNumber(Computer computer) {
Random random = new Random();

while (true) {
computer.makeRandomNumberToString(random.nextInt(900) + 100);
if (computer.notExistsDuplicateDigit())
break;
}
System.out.println("computerNum : " + computer.getComputerNum());
}

public Result getGameResult(Computer computer, User user) {
Result gameResult = calculateGameResult(computer, user);
return gameResult;
}

private Result calculateGameResult(Computer computer, User user) {
String userNum = user.getUserNum();
String computerNum = computer.getComputerNum();
Result gameResult = new Result();

for (int i=0; i<userNum.length(); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (int i=0; i<userNum.length(); i++) {
for (int i = 0; i < userNum.length(); i++) {

java 컨벤션 위반! 마찬가지로 IDE의 자동 포매팅을 활용하는 습관을 가지면 좋을듯 해요!

int userDigit = userNum.charAt(i);
int computerDigit = computerNum.charAt(i);

if (isStrike(userDigit, computerDigit)) {
gameResult.plusStrike();
} else if (isBall(userDigit, computer)) {
gameResult.plusBall();
}
}

return gameResult;
}

private boolean isStrike(int userDigit, int computerDigit) {
return userDigit == computerDigit;
}

private boolean isBall(int userDigit, Computer computer) {
String computerNum = computer.getComputerNum();

for (int i=0; i<computerNum.length(); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java 컨벤션 위반!

int computerDigit = computerNum.charAt(i);
if (userDigit == computerDigit) {
return true;
}
}
return false;
}
}