1, 2주차 기간동안 진행된 자동차 경주 게임 미션에 대한 회고를 작성한다.
https://github.com/woowacourse/java-racingcar/pull/181
https://github.com/woowacourse/java-racingcar/pull/199

테스트를 하고 안하고의 기준은 신뢰도

image

난수를 어떻게 테스트 하는가? 난수를 테스트 할 수 있는 거의 유일한 방법은 난수의 범위를 지정하고 그 범위에 속하는 값들인지 알아보는 것이다. 그렇다면 Random API는 랜덤이니까 테스트 할 필요가 없을까?

나와 샐리는 랜덤 자체를 테스트 할 필요는 없다고 결론을 내렸다. 그러나 이는 틀린 생각이었다.

Random API를 테스트하지 않는 이유는 유명하고 신뢰할 수 있기 때문이다. 이미 수도 없이 많이 검증되었기 때문에 믿고 쓸 수 있다. 테스트할 필요가 없다. 그래서 테스트를 안하는 것 뿐이다. 애초에 테스트를 하는 이유가 무엇인가? 믿고 쓰기 위함이 아니었나? 그러니까 리팩토링도 더 수월하고!

본질을 까먹지 말자. 테스트의 기준은 신뢰도다.


네이밍은 내가 아닌 객체 입장으로

image image image

변수 네이밍, 특히 파라미터의 이름은 내가 아닌 객체의 입장에서 생각해야 한다. 도메인 내부의 파라미터는 내가 사용하는게 아니다. 객체가 사용하는거다.

메서드도 마찬가지. 특히나 클래스/인스턴스 네이밍이 충분히 예상되는 객체의 메서드에는 이름을 포함시킬 필요가 없다. 변수와 메서드는 내가 아닌 해당 객체가 사용하는 것이다. 내가 사용하는건 컨트롤러뿐!


package-private 접근제어자

image image

package-private 접근제어자 이슈에 관해서는 이전에도 포스팅을 해두었었다.
https://hyeon9mak.github.io/Java-dont-use-package-private/

결론적으로 API 개발자가 아닌 이상 package-private을 사용할 일은 흔하지 않다. 또한 비어있는 접근제어자보단, 채워져있는 접근제어자가 더 명확하므로 public, private, protected 셋 중 하나는 꼭 기입하는 습관을 들이자.


예측 가능한 한 가지 행동만 시키자

image image

validateDuplicte라는 이름을 가지고 있지만, 실제 동작은 검증 후 List<Car>를 반환하고 있었다. 코드를 작성한 나는 메서드의 동작을 알고 있지만, 내 코드를 처음보는 리뷰어 휴는 메소드 이름만으론 List<Car>를 반환할 것이라는걸 예상하지 못했다. 명백한 나의 잘못이다.

이름을 통해 예측 가능한 동작만 유도하자. 새로 생성된 List 하날 버린다고 해도 크게 문제될 것은 없다. 나의 구현 능력을 믿지 말고, GC의 성능을 믿자. 😅

image

여기서도 메서드가 두 가지 일을 하게 하는 악행을 저질렀다. 메서드는 한 가지 일만 해야한다. 굳이 두 가지 일을 하게 하고 싶다면 네이밍을 잘 해주자. 예를 들자면, ‘비누로 거품을 낸다.’ 라는 이름으로 ‘물로 헹군다’ 까지 해선 안된다. 호출하는 쪽에서는 거품을 내는 것까지만 예측 가능하다. 굳이 사용하고 싶다면 두 개의 메서드를 만든 후 ‘손을 씻는다’ 메서드로 합치자.


객체에 메세지를 보내라

image image

매번 인지하고 있다고 생각하면서도, 매번 까먹는 객체에 메세지를 보내라.

‘이정도 getter 메서드는 괜찮겠지?’ 라고 시도하면 어김없이 피드백이 날아온다. 리뷰어분들이 이정도까지 체크해주시니, 매우 중요하다는게 느껴진다. 코드짜기 전에 3번씩 되뇌이자. 객체에 메세지를 보내라. 객체에 메세지를 보내라. 제발 좀 보내라.


주입, 주입, 주입!!

image

자동차 전진에 관해서, 대략적으로 아래와 같은 구조를 가지고 있었다.

public class Car {
    private final GasTank gasTank;

    private Car() {
        this.gasTank = new GasTank();
    }

    public void forward() {
        if (gasTank.isEnoughGas()) {
            position.forward();
        }
    }
}
public class GasTank {
    private static final int MINIMUM_GAS = 0;
    private static final int MAXIMUM_GAS = 9;
    private static final int ENOUGH_GAS = 4;
    
    boolean isEnoughGas() {
        return RandomUtils.nextPositiveInt(MINIMUM_GAS, MAXIMUM_GAS) >= ENOUGH_GAS;
    }

}

car.forward() 를 실행시키면 랜덤하게 전진을 진행했는데, 말 그대로 랜덤이라서 테스트코드 작성이 어려웠다. 여기에 대해서 리뷰어 휴가 “랜덤 값 자체를 외부에서 주입받는 걸로 바꾸어 보라” 는 피드백을 주셨다. 그렇게 변경된 코드가 아래와 같다.

public class Car {
    private final GasTank gasTank;

    private Car() {
        this.gasTank = new GasTank();
    }
    
    public void fillUpsGas(final int gas) {
        gasTank.fillUp(gas);
    }

    public void forward(final int gas) {
        if (gasTank.isEnoughGas()) {
            position.forward();
        }
    }
}

이렇게 되니 car.forward에 대한 테스트가 훨씬 간단해졌다.

게다가 자동차 전진 로직이 랜덤이 아닌 확정으로 바뀔 경우에도 직접적인 내부 도메인 수정 없이 input을 충분히 큰 값으로 주는 것만으로도 전진을 구현할 수 있게 되었다. 의존성 주입을 지킬수록 프로그램이 유연해진다. 이것도 3번씩 되뇌이자. 주입, 주입, 주입!!


없어도 되는 키워드를 기입하면, 강조의 의미를 갖는다.

image

메서드 내부에서 임시로 사용하는 ArrayList 에도 final 키워드를 붙였었다. 그런데 리뷰어 휴가 이에 대한 본인의 생각을 공유해주셨다.

우선 자바에선 나열형 자료구조를 대상으로 final 키워드를 사용해 내부 원소의 불변을 지키려고 사용할 수는 없다. final 키워드가 사용되는 이유는 해당 변수 이름에 새로운 인스턴스 할당을 막기 위함인데, “이게 과연 메서드 내부에서만 사용되는 인스턴스에게도 유의미한가?” 라는 생각을 가질 수 있다.

이에 관한 결론은 의외로 쉽게 내릴 수 있었는데, 얼마전 질문 채널에서 크루들의 이야기를 구경하다가 java.lang.String 클래스 내부에서 value[] 필드에 final 키워드가 붙어있는 이유가 “원소들의 불변을 지키는 기능은 없지만, 개발자가 유의해서 지켜야 한다. 주의해라.” 라는 것을 알게 됐다. 이와 마찬가지로 내가 메서드 안의 지역변수에 final 키워드를 붙이게 되면, 주의하라는 의미를 갖게 되지 않을까? 주의를 환기시킬 필요가 없다면 final 키워드는 붙이지 않는 것이 더 나을 것 같다.


유틸 클래스를 지양하자

image image

리뷰어 휴가 유틸 클래스에 대한 본인의 취향을 공유해주셨다. 본인은 유틸 클래스를 선호하지 않는다고 하신다.

첫 번째 이유로 재정의 가능성을 문제로 드셨다. Math.max(double a, double b)와 같은 경우 수학적인 비교로 할 일이 명확하고, Math 객체가 따로 가져야할 상태(필드)도 전혀 없이 a와 b의 비교만 진행해주면 된다. 또한 이름에 따른 동작이 재정의 될 가능성이 전혀 없다. 인간의 문명이 갑자기 초기화 되지 않는 이상…

두 번째 이유로 만능 클래스의 탄생을 문제로 드셨다. 이름만 잘 짓는다면, 모든 행위(메서드)가 잘 어울리는 만능 유틸 클래스가 탄생해서 모든 책임을 떠맡게 된다. 휴 본인도 그런 경험이 있다고 하셨는데 곰곰히 생각해보니 나도 프리코스 1차 미션을 그렇게 진행했던 기억이 떠올랐고, 잘못됐다는 걸 바로 이해할 수 있었다. 🤤

그 외에도 지양해야할 이유는 아래 블로그에 잘 정리 되어있다.
https://unabated.tistory.com/entry/왜-자바에서-static의-사용을-지양해야-하는가


데이터는 Domain, 표현은 View

image

자동차 경주 미션에서 자동차의 이동거리를 표현하는 방법은 아래와 같았다.

현구막 : ---
휴    : -----

나는 이동거리 계산에서 ‘어차피 - 기호로 출력할거라면, StringBuilder- 기호를 계속 더해서 가지고 있으면 되지 않을까?’ 라는 생각으로 자동차의 위치를 표현하는 Position 객체의 필드로 StringBuilder를 갖게 했었다. 그러자 리뷰어 휴가 데이터와 표현을 구분지으라는 피드백을 해주셨다.

Position이 가지고 있는 이동거리의 데이터는 정수 값이고, 표현이 -일 뿐이다. 만일 이 표현이 - 가 아닌 *로 바뀌게 된다면, Position Domain을 찾아 들어가 내부 로직을 변경해주어야 한다. 표현이 바뀐 것인데, Domain의 수정이 일어나는 것이다. 이러면 MVC 패턴을 사용한 이유가 없다.

‘모든 로직은 View가 아닌 Domain이 가져야 하고, View는 출력 창구일 뿐이다.’ 라는 생각에 갖혀서 위와 같은 구조를 설계했다. 완전히 잘못된 생각이다. View는 출력에 한해서는 얼마든지 로직을 가질 수 있다. View도 Domain만큼 역할의 무게감이 있음을 항상 생각하자.


우선 생성(복사)를 한 뒤에 검증을 진행하자

image

프리코스 미션때까지 내가 생성자에 검증로직을 두는 방식은 아래와 같았다.

    public Car (final String name) {
        validateLength(name);
        this.name = name;
    }

그런데 멀티 스레딩 환경에서는 이 순서에 문제가 생긴다고 한다. 생성자 파라미터 name이 검증이 끝난 후 this.name에 대입되기 직전에 다른 스레드에 의해 변질될 가능성이 존재한다는 것이었다. 때문에 우선 필드 멤버에 대입하고, 그 필드 멤버를 검증하는 순서를 가지라고 한다.

    public Car (final String name) {
        this.name = name;
        validateLength(this.name);
    }

보기엔 부자연스러운 흐름일지 몰라도, 이렇게 하면 스레드에 안전한 순서가 된다.

image

추가적으로 “검증 로직을 생성자가 아닌 정적 패터리 메서드 쪽에 두어도 되나요?” 라는 질문에는 “NO” 라는 답변을 받을 수 있었다. 다른 정적 팩터리 메서드가 추가되면, 그 팩터리 메서드는 검증 로직을 거치지 못하므로, 모든 메서드/인스턴스가 거칠 수 밖에 없는 공통된 생성자에 두는 것이 안전하다! 명쾌해!


값 객체(VO)의 방어적 복사는 좋다! 그러나 나머지들은…

image image

(메서드의 역할이 줄어든다는 표현은 내가 봐도 뭔 말인지… 아마 객체의 상태변화가 줄어드므로 책임이 줄어드는 것 같다는 말을 하고 싶었나보다.)

제이슨의 불변객체 강의 를 보고 무작정 여기저기 불변객체를 만들기 위해 방어적 복사를 진행했었다. 리뷰어 휴는 공부를 위해 이렇게 사용한 것에 대해서는 긍정적이지만, 본인은 VO 객체가 아니라면 불변 객체를 만드는 것을 지양한다고 하셨다.

사실 이펙티브 자바를 읽으면서도 불변 객체에 대한 인사이트를 얻었었기 때문에 무작정 불변이 좋구나 생각했었었는데, 고민할 거리를 던져주신 것 같다.

어쨌거나 저쨌거나Money, Position 등 값 자체가 의미를 내포하고 있는 VO는 불변 객체로 유지시키는 것이 더 좋다는 것은 확실하니, 그것만이라도 잘 지키기 위해 노력해봐야겠다.


페어 회고

자동차 경주 미션을 함께 진행한 샐리 는 밝고 에너지가 넘치는 크루였다. 첫 페어프로그래밍의 긴장감과 불편함, 답답함 속에서도 분위기가 쳐지지 않도록 힘써주었고, 우리집 똥강아지 설이가 크게 짖어서 깜짝깜짝 놀랄 때도 설이를 귀엽게 여겨줘서 잘 넘어갈 수 있었다. 한 없이 고마운 페어였다.

샐리가 부족한 나에게 맞춰준 것인지, 아니면 정말로 그랬던 것인진 모르겠지만 프로그래밍 수준이 비슷비슷해서 페어기간동안 의견충돌 한 번 없이 “이거 어때요?” “오 좋아요!” 하면서 순식간에 미션을 완성할 수 있었다. 조금 아쉬운 점이라면 서로 수준이 너무 똑같아서 그런지 “왜요?” 라는 질문을 많이 주고 받지 못했다는 점. 서로 어떻게 생각하는지 비교해보는 것도 페어프로그래밍의 재미지 않았을까 생각해서 아쉬움이 남는다.

반성해야겠다고 생각되는 점은 아무래도 처음이다보니 ‘나는 어떤 사람이고, 이럴 땐 이렇게 한다.’ 라는 걸 미리 충분히 이야기하지 못했다는 것이다. 나는 상대방의 이야기를 듣고 고민할 때(머리 속으로 연산할 때) 멍을 때리거나 “어…” 하고 말 끝을 흐리곤 하는데, 샐리는 내 반응이 부정을 뜻하는 것이라 해석하고 의견을 철회하곤 했다. 페어 이틀차가 되어서야 샐리에게 “제가 그러고 있다면 생각중인거고, 부정적인게 아니니까 조금만 기다려줘요!” 라고 이야기해서 오해를 풀 수 있었다. 괜히 포비가 충분히 서로를 알아가는 시간을 가지라 하셨던 것이 아니었다.

“샐리와 다시 페어를 하고 싶은가?” 라는 질문에는 “당연!” 이라고 답하고 싶다. 모니터를 뚫고 나오는 텐션 소유자 샐리와 함께라면 즐겁게 할 수 있겠지! 나는 샐리에게 좋은 페어였을까? 샐리 눈에 보인 내 텐션은 어느정도였을까? 나는 과연 다시 페어를 같이하고 싶은 사람인가? 고민해봐야겠다.

끗.

댓글남기기