[우테코] 자동차 경주 2단계 학습 로그

---

목차

1. step 2 목표

2. 변경 사항

3. 생각할 점

4. 개선할 점

---

 

자동차 경주 1단계 학습로그: https://yeonyeon.tistory.com/189
GitHub: https://github.com/yeon-06/java-racingcar/tree/step3
Pull Request: https://github.com/woowacourse/java-racingcar/pull/419

---

1. step 2 목표

 

• MVC 패턴 기반으로 리팩토링
• domain 패키지의 객체는 view 패키지 객체에 의존 X
• 테스트 가능/불가능 부분을 분리해 테스트 가능 부분에 대해서만 단위 테스트

 

사실 1단계에서부터 MVC를 도입했기 때문에 큰 변경 사항은 없었다.

이름 짓기에 좀 더 생각해보았고 toString()이나 util 클래스에서 생성자의 private화 등 기본적인 것만 변경해서 PR을 요청했다.

 

---

2. 변경 사항

🙋‍♀️ 내 생각, 🙍‍♂️ 리뷰어님

 

2-1. 불필요한 stream 제거

// AS-IS
IntStream.range(0, length).forEach(i -> copyArr[i] = array[i].strip());

// TO-DO
for (int i = 0; i < length; i++) {
    copyArr[i] = array[i].strip();
}

🙋‍♀️ 굳이 Stream 처리를 해가며 forEach() 사용할 필요 없이 for문으로 처리하는게 더 간단하다고 느낌

 

🙍‍♂️ 이 글을 보면 항상 Stream이 좋은 건 아니라는 것을 알 수 있죠🙂 개인적으로 현재는, 스트림을 통한 성능적인 이슈보단 현재 연로그가 고민하는 것처럼 가독성적인 측면을 고려하는 것이 바람직할 것 같아요. 다만 이런 부분은 항상 같이 고민하면 좋을 것 같아요. 스트림을 처음 쓰거나 낯선사람에게는 당연하게도 for문이 익숙하고 가독성이 높을거에요. 즉 새로운 기술은 언제나 처음은 낯설기에 일정 기간 적응하고 난 뒤에도 가독성이 떨어지는가? 를 기준으로 판단해나가면 좋을 것 같아요.

 

2-2. ThreadLocalRandom 사용

// AS-IS
Random random = new Random();
return min + random.nextInt(max - min);

// TO-DO
return min + ThreadLocalRandom.current().nextInt(max - min);

🙋‍♀️ thread-safe한 클래스를 사용하고자 시도

 

2-3. Controller의 멤버 변수들을 지역 변수로 변경

// AS-IS
private Cars cars;
private TryCount tryCount;
public void start() { ... }

// TO-DO
public void start() {
    Cars cars;
    TryCount tryCount;
    ...
}

 

🙋‍♀️ Controller에가 여러 곳에서 접근하는 경우 타인에 의해 상태가 변경될 수 있음. 상태를 갖지 않도록 수정함

 

2-4. @DisplayName 사용

// AS-IS
@SuppressWarnings("NonAsciiCharacters")
class CarTest {
    @Test
    public void 테스트() { ... }
}

// TO-DO
class CarTest {
    @Test
    @DisplayName("테스트")
    public void test() { ... }
}

🙋‍♀️ 한글 메소드명 외로도 해당 오류가 발생하는 경우가 있으면 어떡하지? 라는 생각이 들어서 강제로 오류를 무시하는게 꺼려짐. @DisplayName을 사용하는 방향으로 결정

 

🙍‍♂️ 좋은 방향이네요🙂 다만 위에서 한글인 경우에 대한 워닝을 무시한다고 명시하셨는데, 한글 메소드명 외에 해당 오류가 발생한다라는 케이스가 있을까요? DisplayName을 사용하는 것과 한글 메소드를 사용하는 것에 대한 기준은 주관적이지만 DisplayName을 사용하기로 한 이유가 조금 이해가 덜 되어서요!

 

🙋‍♀️ 제가 직접 겪지는 않았지만 추후에 발생 가능성이 있지 않을까 불안함이 있었습니다😂 자주 보는 에러가 아니다보니 어디서 발생할지 예상이 안되더라고요💦

 

2-5. util 클래스의 생성자 private화

public class StringUtils {
    private StringUtils() {
    }
}

🙋‍♀️ 모든 메소드가 static + 상태가 없고 앞으로도 가지면 안된다고 판단해서 명시적으로 생성자를 막았습니다.

 

🙍‍♂️ 사소한 부분이지만 좋은 습관인 것 같아요🙂 실제로 나중에 사용하실 Lombok에서도 유틸클래스에서 사용하는 어노테이션은 퍼블릭 생성자를 못 만들도록 되어 있습니다.👏

 

---

3. 생각할 점

 

3-1. 자료형의 범위가 넘어가는 입력 값

🙋‍♀️❓ int의 범위가 넘는 입력 값이 들어오면 어떻게 처리하는게 좋을까요?

 

🙍‍♂️ 이 부분도 연로그가 고민하신 부분의 결정이 답이 될 것 같아요. 방어적인 형태로 코드를 작성하는 것은 매우 좋은 습관이라고 생각합니다. 21억건 이상의 입력을 방지하기 위해 long을 사용할 수도 있겠지만, 저라면 21억건 이상의 시도횟수는 프로그램에 대한 부적절한 시도라고 생각해서 다른 예외를 반환할 것 같아요. 인트의 최대값을 초과하는 입력에 대해서 접근할 때도 단순히 수를 늘리고 줄이는 것 보단, 21억 이상의 값이 입력되는 것이 프로그램에 어떤 의미인지 고민해보면 좋을 것 같아요. 추가로 그 숫자가 들어와도 정상동작해야한다면 long타입을 쓰겠지만 그 수가 들어왔을 때 예외를 뱉는다면 연로그가 말씀주신 예외보단 조금 더 의미있는 예외가 있을 것 같아요🙂

 

👉 최대값 제한함으로써 해결

 

3-2. 역할과 책임의 범위

🙋‍♀️❓ 현재 Car에서는 파라미터로 넘겨준 값이 기준 값보다 큰 경우 position을 +1 해줍니다. 이 기준 값보다 크다는 조건은 Car에 넣는게 맞을까요 Cars에 넣는게 좋을까요? 책임의 범위가 헷갈려요.

 

🙍‍♂️ 전략 패턴을 사용해서 수정해보는건 어떨까요? 완벽하게 이해하고 넘어가야한다기보단, 한 번정도 사용해보며 이런 식으로 해결할 수도 있구나라고 경험해보시면 좋을 것 같아요. 전략 패턴을 사용한다면 4라는 기준값은 어디에 들어갈지도 같이 고민해볼 수 있을 것 같아요.

 

🔻 전략 패턴; Strategy Pattern

: 객체들이 할 수 있는 행위 각각에 대해 전략 클래스를 생성하고 유사한 행위들을 캡슐화 하는 인터페이스로 정의. 객체의 행위를 동적으로 바꾸고 싶은 경우 직접 행위를 수정하지 않고 전략을 바꿔주기만 함으로써 행위를 유연하게 확장하는 방법

 

예를 들어 move()에 대한 로직을 캡슐화하고 싶다라고 한다면

1. move() 메소드를 포함한 MoveStrategy 인터페이스를 생성
2. MoveStrategy를 구현하는 클래스에서 move()를 정의
   (move의 로직을 다르게 하기 위해 클래스를 여러개 생성해도 ok)

이후 필요한 move()를 선언한 클래스를 사용하면 된다.

 

3-3. 사용자의 입력 값도 테스트해야 하는가?

🙋‍♀️❓ 사용자가 직접 입력하는 값도 테스트를 할 수 있나요? 처음엔 전략 패턴을 적용시키면 가능할까해서 만들어보다가 그냥 문자열에 대한 테스트가 될뿐 사용자가 의도한대로 입력이 되었는지에 대한 테스트가 아닌 것 같아서 롤백했습니다.😭

 

🙍‍♂️ 사용자가 직접 입력하는 것은 우리가 제어할 수 있는 대상이 아니니 테스트 대상이 아니라고 판단하는게 맞을 것 같아요. 그렇다면 우리가 제어할 수 없는 사용자 입력을 어떻게 제어할 수 있는 범위로 넣을 것인가?를 고민했을 때 전략패턴으로 이를 해결할 수 있구요. 다만 연로그가 말한 것 처럼 사용자가입력한 값 == 문자열에 대한 테스트가 될 확률이 높겠죠? 그렇기에 불필요하다면 추가하지 않는 것도 방법이에요. 조금 더 나아가서 프로그램 전체가 올바르게 동작한다 즉 레이싱컨트롤러.start()를 했을 때 전체 프로그램이 동작한다를 증명하려면 입력에 대한 전략패턴이 분명 필요해질 수 있겠죠? 어떤 것을 테스트할 것인가에 따라 전략패턴도, 다른 기법들도 사용될지 아닐지가 달라질 수 있다는 점까지 염두해두면 더 좋을 것 같습니다🙂

 

3-4. 테스트를 위한 클래스 생성

🙋‍♀️❓ '테스트를 위한 클래스'이 늘어나도 괜찮은건지에 대한 고민이 있습니다

 

🙍‍♂️ 

• 테스트를 위한 클래스라고 간주된다면 테스트코드 패키지에 추가하는 것이 맞을 것 같아요🙂
  재사용의 가능 여부가 충분하더라도, 그게 확실하지 않다면 프로덕션코드에 구현하는 것은 오히려 혼란을 가중하는 역효과가 있다고 생각합니다.
• 추가로 람다를 이용해서 구현체를 만들지 않고 테스트에서 사용할 수도 있습니다! 
  (람다와 FunctionalInterface 라는 키워드로 검색해보시면 좋을 것 같아요.)

 

---

4. 개선할 점

 

4-1. DTO의 도입

🙍‍♂️ 도메인과 뷰의 분리에서 조금 더 나아가서, 뷰는 도메인을 알지 못한다는 규칙도 존재하는데요. 현재는 Car라는 도메인을 직접 알고 있어요. 요 부분의 의존성을 약화시킬 수 있는 혹은 의존하지 않고도 출력할 수 있는 다른 방법도 고민해보면 좋을 것 같아요.

 

🙋‍♀️ 뷰를 위한 dto를 생성한다에 대해서도 생각해보았는데요. 자동차 경주에서는 오히려 오버 프로그래밍이 되지 않을까하여 도입하지 않았습니다! 혹시 다른 방법도 있을까요?

 

🙍‍♂️ 저도 오버프로그래밍이이라고 생각합니다ㅎㅎ 리뷰어에 따라 다르겠지만, 저는 개인적으로 해당 미션에서 얻어갈 수 있는 것들에 대해서 충분히 인지하신 분들에게는 오버프로그래밍도 괜찮다고 권하고 있어서 말씀드렸어요🙂 다른 방법도 결국 도메인과 뷰를 분리한다는 맥락은 같은 것이 있겠지만, 동일하게 오버프로그래밍 영역일 것 같아요.

 

👉 아직 우테코 과정이니까 오버 프로그래밍 해가며 다양한 경험 해보기

 

4-2. 전략의 범위

public class RandomMoveStrategy implements MoveStrategy {
    private static final int MOVE_MIN = 0;
    private static final int MOVE_MAX = 9;
    private static final int STANDARD_VALUE = 4;
    private static final int MOVING_DISTANCE = 1;

    @Override
    public int move() {
        int random = RandomUtils.generateNumber(MOVE_MIN, MOVE_MAX + 1);
                if (random >= STANDARD_VALUE) {
            return MOVING_DISTANCE;
        }
        return 0;
    }
}

🙍‍♂️ 현재 해당 전략들은 관점에 따라 아래와 같은 2가지 일을 하고 있는데요. 이 두가지 모두가 해당 전략의 역할로 간주했을 때와, 움직이는 범위는 자동차가 결정하는 것 중 현재 방법을 택한 이유가 있을까요?

• 움직일지 아닐지를 판단한다.
• 실제로 움직이는 양(현재 1 또는 0)을 결정하여 반환한다

 

🙋‍♀️ Car에서 move할 거리를 받고 바로 움직이고 싶어서 Strategy에 책임을 위임했습니다! 움직일지 아닐지만 판단하는 기능만 하면 기존 로직이랑 크게 다른게 없지 않나? 라는 생각이 들었습니다

 

🙍‍♂️ 좋은 관점인 것 같습니다. 다만 인트를 반환하는게 움직이는 정도를 반환한다는 것이 의미가 잘 드러나지 않는 것 같아요. 그래서 말씀드렸습니다!

 

👉 메소드 이름을 변경해 의미 드러내기

---

참고

• 우테코 리뷰어 - 카일
• https://victorydntmd.tistory.com/292