[프리코스] 숫자 야구 리뷰
조금 늦었지만 지난주에 수행했던 숫자 야구 미션에 대해서 리뷰해보고자 한다.
숫자 야구 게임이란 1~9까지의 서로 다른 수로 이루어진 3자리의 수를 맞추는 게임이다. 컴퓨터가 생성한 3자리 숫자와 사용자가 입력한 3자리 수의 각 자리수를 비교한다. 만약 자리수, 숫자가 모두 같으면 스트라이크이고 자리수는 다르지만 숫자가 같으면 볼이다. 자리수, 숫자 같은게 아무것도 없으면 낫싱이다.
이를 토대로 다음과 같은 기능 명세서를 먼저 만들었다.
기능 명세서
- 컴퓨터 난수 생성하기
- 수의 유효성을 검사한다. (길이가 3인지, 숫자인지, 중복된 수인지)
- 사용자로부터 숫자를 입력 받기
- 입력 숫자의 유효성을 검사한다. (null검사, 길이 3인지, 숫자인지, 중복된 수 존재하는지)
- 컴퓨터 수와 사용자로부터 입력받은 수를 비교한다.
- 볼 개수 세기
- 스트라이크 개수 세기
- 게임 성공 여부를 판단한다.
- 스트라이크의 수가 3인지 체크한다.
- 성공 시 게임 재시작 숫자를 입력한다.
- 실패 시 게임 숫자를 입력한다.
- 결과 출력
- 성공시 성공 메시지와 재시작 안내 메시지를 출력한다.
- 실패시 게임 결과를 출력하고 게임 숫자 입력 안내 메시지를 출력한다.
- 종료 숫자를 입력받는다.
- 종료 입력 수의 유효성을 검사한다. (null검사, 길이 1인지, (1, 2)에 속하는지)
이제 이를 토대로 구현한 코드를 살펴볼건데 먼저 리팩토링하기 전의 단순히 테스트만 통과하는 코드를 먼저 보자
리팩토링 전 코드
public class Application {
final static int length= 3;
public static void main(String[] args) {
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
Map<String, Integer> map;
System.out.println("숫자 야구 게임을 시작합니다.");
while (true) {
List<Integer> computer = new ArrayList<>();
while (computer.size() <length) {
int randomNumber = Randoms.pickNumberInRange(1, 9);
if (!computer.contains(randomNumber)) {
computer.add(randomNumber);
}
}
for (int i = 0; i <length; i++) {
System.out.print(computer.get(i));
}
System.out.println();
try {
while (true) {
map = new HashMap<>();
System.out.print("숫자를 입력해주세요 : ");
String inputNumber = br.readLine();
if (inputNumber.length() !=length) {
throw new IllegalArgumentException();
}
for (int i = 0; i <length; i++) {
char cur = inputNumber.charAt(i);
if (cur < '1' || cur > '9') {
throw new IllegalArgumentException();
} else {
if (map.containsKey(String.valueOf(cur))) {
throw new IllegalArgumentException();
} else {
map.put(String.valueOf(cur), 1);
}
}
}
int strike = 0;
int ball = 0;
for (int i = 0; i <length; i++) {
int cur = inputNumber.charAt(i) - '0';
if (computer.get(i) == cur) {
strike++;
} else if (computer.contains(cur)) {
ball++;
}
}
if (strike ==length) {
System.out.println("3스트라이크");
System.out.println(length+ "개의 숫자를 모두 맞히셨습니다! 게임 종료");
System.out.println("게임을 새로 시작하려면 1, 종료하려면 2를 입력하세요.");
break;
}
if (strike == 0) {
if (ball == 0) {
System.out.println("낫싱");
} else {
System.out.println(ball + "볼");
}
} else {
if (ball == 0) {
System.out.println(strike + "스트라이크");
} else {
System.out.println(ball + "볼 " + strike + "스트라이크");
}
}
}
String isExit = br.readLine();
if (isExit.length() != 1) {
throw new IllegalArgumentException();
}
if (isExit.charAt(0) == '1' || isExit.charAt(0) == '2') {
if (isExit.charAt(0) - '0' == 2) {
break;
}
} else {
throw new IllegalArgumentException();
}
} catch (Exception e) {
throw new IllegalArgumentException();
}
}
}
}
진짜 말그대로 테스트만 통과하게 작성한 코드이다. 코테 문제푸는 느낌? 으로 구현했던 것 같다..
하지만 단순히 p/f로 나뉘는 코테와 달리 실제로 다른 사람과 협업하고 유지보수적인 관점까지 고려한다면, 위 코드에는 다음과 같이 여러 문제가 있다.
문제점
- 하나의 클래스에서 모든 작업이 이루어진다. -> 너무 많은 책임의 부여
- 가독성이 매우 떨어진다. -> 코드만 보고 어떤 일을 하고 있는지 파악이 어렵다.
- 하드코딩 했기 때문에 유지보수가 어렵다.
- 하나의 메서드에서 모든 일이 수행되기 때문에 단위 테스트가 불가능하다.
- 에러 메시지가 없기 때문에 정확히 어디서 예외가 발생한지 확인하기 어렵다.
문제해결
이런 문제를 해결하기 위해서 필자는 우선 메서드를 나눠보기로 했다.
위 코드를 토대로 메서드를 추출하려고 노력했는데 스파게티 코드라서 나누는 기준을 잡는것이 쉽지 않았다..
(주말에 숫자 야구 피드백 강의영상을 보고 반성을 많이했다. 기능 명세서의 중요성과 설계의 중요성을 다시금 느꼈다…)
필자는 결과를 출력하는 부분, 게임 숫자를 입력받는 부분, 숫자를 검증하는 부분, 종료 숫자를 입력받는 부분, 난수 생성 부분, 게임 실행 부분을 추출하였다.
그리고나서 각각의 메서드 기능을 테스트하기 위해서 테스트 코드를 작성하고자 했다. 그런데 여기서 문제가 발생했다.
private 메서드를 test할 수 없었다.. 그래서 어떻게 하면 테스트 할 수 있을지 찾아봤다. 그리고 private 메서드는 강제로 테스트할 수는 있지만 private 메서드의 기능이 테스트가 필요한 기능이면 클래스 분리를 생각해봐야 한다는 답을 얻었다.
그래서 필자는 앞서 분리한 메서드들이 테스트가 필요한지, 어떤 기준으로 나누면 더 코드가 명확해지는지 고민했다. 그리고 필요한 부분에 대해서 클래스 승격을 진행했다.
승격된 클래스들
우테코 커뮤니티
그리고 이 과정에서 우테코 커뮤니티의 글들이 도움이 많이 됐다. 함께 나누기에 여러 유익한 글들이 많이 올라왔는데 이중에 일급 컬렉션이라는 주제가 올라왔었다. 처음 보는 키워드였기 때문에 바로 공부를 해서 블로그에 정리하였다 .
일급 컬렉션 정리 : https://jinsu868.github.io/%ED%81%B4%EB%A6%B0%EC%BD%94%EB%93%9C/40/
그리고 공부를 해보니 일급 컬렉션이 딱 난수 생성 클래스에 적용하면 좋겠다는 생각이 들었다. 그래서 다시 코드를 리팩토링하여 적용시켰다.
ComputerNumber class (일급 컬렉션)
public class ComputerNumber {
private final List<Integer> number;
public ComputerNumber() {
this.number = new ArrayList<>();
createComputerNumber(number);
}
private void createComputerNumber(List<Integer> number) {
while (number.size() < 3) {
int randomNumber = Randoms.pickNumberInRange(1, 9);
if (!number.contains(randomNumber)) {
number.add(randomNumber);
}
}
}
public int get(int index) {
return number.get(index);
}
public boolean contains(int value) {
if (number.contains(value)) {
return true;
} else {
return false;
}
}
}
매직넘버 -> 상수
그리고 나서 가독성 및 유지보수를 위해 매직넘버를 상수로 빼고, ENUM을 사용하여 리팩토링을 진행했다. 메서드 이름, 변수 명도 역할을 명확하게 표현하기 위해 수정했다.
//상수 사용
public class Notification {
private static final String STRIKE = "스트라이크";
private static final String BALL = "볼";
private static final String NOTHING = "낫싱";
private static final String END_MESSAGE = "3스트라이크\n3개의 숫자를 모두 맞히셨습니다! 게임 종료";
private static final String RESTART_MESSAGE = "게임을 새로 시작하려면 1, 종료하려면 2를 입력하세요.";
private static final String START_MESSAGE = "숫자 야구 게임을 시작합니다.";
public static void notifySuccessResult() {
System.out.println(END_MESSAGE);
System.out.println(RESTART_MESSAGE);
}
~~
}
ENUM 사용
//ENUM 사용
public enum ErrorCode {
INVALID_PLAY_NUMBER("유효하지 않은 게임 수를 입력하였습니다."),
INVALID_TERMINATE_NUMBER("유요하지 않은 종료 수를 입력하였습니다");
private final String message;
private ErrorCode(String message) {
this.message = message;
}
public String getErrorMessage() {
return message;
}
}
이번 미션에서는 기능 명세서를 제대로 활용하지 못한 것 같다. 다음 미션부터는 기능 명세서를 확실하게 먼저 작성하고 설계를 제대로 한 다음에 코드를 작성하도록 노력하겠다:)
전체 코드는 아래 링크에서 확인할 수 있다.
https://github.com/woowacourse-precourse/java-baseball-6/pull/408/files#diff-c71c95b782b88469a2947a817aa6945baf76f4e52c732f226e056fbfae0dcd52