[Review] 사다리 구현 4단계 - 리팩토링 2 (2022/02/20)

 

코드리뷰

이번 리뷰는 Dion이 해주셨다!! 감사합니다 Dion!!

1.  Depth 2 지키기

모든 부분에서 depth2를 지켰다고 생각했는데, 인식하지 못한 부분이 있었다...

 

기존의 코드는 다음과 같았다.

public int[] makeStartPositions() {
    int totalPositions = ladderSize.getTotalPositions();
    int countOfLine = ladderSize.getCountOfLine(DEFAULT_PERCENT);
    int[] startPositions = new int[countOfLine];
    for (int i = 0; i < startPositions.length; i++) {
        int randomPosition = randomRangeInt(1, totalPositions);
        if(ladderSize.isMultipleOfPersonNum(randomPosition) || isExisted(startPositions, randomPosition)){
            i--;
            continue;
        }
        startPositions[i] = randomPosition;
    }
    return startPositions;
}

위 코드에서 for문 안의 if문 때문에 depth가 2까지 증가하여 버렸다.

이를 다음과 같이 해결하였다.

public List<Integer> makeStartPositions() {
    int totalPositions = ladderSize.getTotalPositions();
    int countOfLine = ladderSize.getCountOfLine(DEFAULT_PERCENT);
    return makeUniquePositions(totalPositions, countOfLine);
}

private List<Integer> makeUniquePositions(int totalPositions, int count) {
    List<Integer> list = new LinkedList<>();
    while (list.size() < count) {
        addRandomNumToList(list, randomRangeInt(1, totalPositions));
    }
    return list;
}

private void addRandomNumToList(List<Integer> list, int randomPosition) {
    if(!ladderSize.isMultipleOfPersonNum(randomPosition) && !isExisted(list, randomPosition)){
        list.add(randomPosition);
    }
}

 

2. error 출력하기

에러 출력을 println으로 하고 있었다. 다음과 같이 추천해주신 내용으로 변경하였다.

System.err.println(e.getMessage());

System.err is a PrintStream. System.err works like System.out except it is normally only used to output error texts.

Some programs (like Eclipse) will show the output to System.err in red text, to make it more obvious that it is error text.

 

3. 상수의 사용

일단 여기서 static을 사용한 이유는 InputView 자체가 static 메서드만을 포함하고 있기 때문이다.

내가 static을 사용한 이유는, class level에서 생성해 두면 각 객체마다 String 인스턴스를 가지고 있을 필요가 없어지기 때문이라 생각했다.

=> Dion의 답변이 오면 수정하겠다.

 

디온의 답변이 왔다!!

 

4. Validation 하는 부분에서 예외 던지기

다음과 같이 사용자의 입력을 받는 requestPerson() 메서드 안에서 예외를 던지고 있다.

public static String[] requestPerson() {
    Scanner scanner = new Scanner(System.in);
    System.out.println(INPUT_PERSON_GUIDANCE_MESSAGE);
    String[] names = scanner.nextLine().split(",");
    if(invalidNameLength(names)){
        throw new IllegalArgumentException(INPUT_NAME_LENGTH_ERROR_MESSAGE);
    }
    return names;
}

이를 isvalidNameLength()메서드 내부에서 예외를 던지도록 변경해 보자.

public static String[] requestPerson() {
    Scanner scanner = new Scanner(System.in);
    System.out.println(INPUT_PERSON_GUIDANCE_MESSAGE);
    String[] names = scanner.nextLine().split(",");
    invalidNameLength(names);
    return names;
}

private static boolean invalidNameLength(String[] names) {
    boolean isMatch = Arrays.stream(names).anyMatch(i -> i.length() > 5);
    if(!isMatch){
        throw new IllegalArgumentException(INPUT_NAME_LENGTH_ERROR_MESSAGE);
    }
    return isMatch;
}

- 변경 전
이전에는 requestPerson() 내부에서 name길이가 5를 넘길경우 예외를 던지고 있었다.

- 변경 후
변경 후 invalidNameLength 에서 검증을 할때, 길이가 5 초과라면 예외를 던지도록 변경하였다.

생각해보니 검증 하는 메서드 스스로 예외를 던지도록 하는것이 더 적합하였습니다.

 

5. OutputView에게 출력할 데이터 전달해 주기

이전에 호눅스 에게도 물어봤었던 부분이다. 수정을 안했었는데 이번 기회에 수정하였다.

도메인(Line)에게 데이터를 반환해주는 쿼리 를 만들어 주었다.

따라서 값을 반환할수 있게 되었다. OutputView에서는 이렇게 반환받은 데이터를 출력해주게 된다.

 

6. @ParameterizedTest

@ParameterizedTest를 사용하니 2가지 장점을 얻을 수 있었습니다.

1) 테스에 전달되는 인자의 이름 덕분에 의미파악이 수월해 졌다.

2) 단순 반복부분을 줄일 수 있다. 위와같은 4줄을 다음과 같이 1줄로 변경하였습니다.

@ParameterizedTest
@CsvSource({"1,0", "1,-1", "0,1", "-1,1"})
void invalidInputTest(int height, int numOfPerson){
    assertThatThrownBy(() -> new RandomLadder(height, numOfPerson)).isInstanceOf(IllegalArgumentException.class);
}