본문 바로가기
테스트코드

TDD, 클린 코드 with Java 피드백 정리

by moonstal 2022. 12. 15.

간략한 소감: 넥스트스텝 하길 잘했다! 리뷰요청을 하고 나면 편지를 받는 것처럼 무슨 내용이 들어있을지 너무 기다려졌다. 2주에 한번씩 라이브로 줌에 참여했을 때 내가 성장하려는 사람들 속에 함께 속해있다는 것도 공부를 지속시키는 힘이 되었던 것 같다. 언젠간 나도 저렇게 될 수 있지 않을까라는 희망이 생겼다. 그리고 무엇보다 내 코드에 대해 피드백 해줄 수 있는 사람이 생겼다는 것이 좋았고 개선될 때의 짜릿함을 느낄 수 있었다.

PR 링크

자동차 경주용 게임 구현

로또 게임 구현

사다리 타기 구현

볼링 게임 점수판 구현

자동차 경주

컬렉션 사이즈 검증

// before
assertThat(numbers.size()).isEqualTo(3);

// after
assertThat(numbers).hasSize(3);

contains

// before
assertThat(numbers.contains(input)).isTrue();

// after
assertThat(numbers).contains(input);

파일 마지막에 newline 경고

하나의 클래스에서 두가지 역할을 하는데 각각의 역할을 별도의 클래스로 분리

  1. 문자열을 받아서 숫자 배열로 파싱하는 역할
  2. 숫자 배열의 덧셈 결과를 계산하는 역할
  • Number 객체를 만들어서 StringAddCalculator의 메세지를 받아 스스로 계산을 할 수 있도록 클래스를 분리
  • Number클래스인데 내부적으로는 numbers, 숫자 배열을 관리하고 있으니까 Numbers
    또는 PositiveNumbers와 같은 이름을 사용
public class Numbers {

    private final int[] numbers;

    public Numbers(String[] numbers) {
        this.numbers = Arrays.stream(numbers).mapToInt(Number::parsePositiveInt).toArray();
    }

    public int sum() {
        return Arrays.stream(numbers).sum();
    }

    private static int parsePositiveInt(String input) {
        int number = Integer.parseInt(input);
        if (isNegative(number)) {
            throw new RuntimeException("음수는 입력할 수 없습니다.");
        }
        return number;
    }

    private static boolean isNegative(int number) {
        return number < 0;
    }
}

상수로 패턴 관리

  • 패턴을 한번만 만들고 재사용 할 수 있도록 Pattern.compile의 결과를 상수로 관리
  • 이름만 보고도 이 패턴이 무슨 역할을 하는지 알 수 있는 이름
private static final Pattern CUSTOM_DELIMITER_PATTERN = Pattern.compile("//(.)\n(.*)");

상수의 이름은 상수를 서술하는 형태보다, 해당 상수가 무엇을 나타내는지

//before
private static final String COMMA_OR_COLON = ",|:";

//after
private static final String DEFAULT_DELIMITER = ",|:";

InputView에서 List를 만들어도 될까?

//before(InputView)
public static List<Car> countCar() {
        System.out.println("자동차 대수는 몇 대 인가요?");
        return generateCars(scanner.nextInt());
}

//after(Racing)
public class Racing {
    private List<Car> cars;

    public Racing(int carCount) {
            generateCars(carCount);
        }

    private void generateCars(int carCount) {
        this.cars = new ArrayList<>();
        for (int i = 0; i < carCount; i++) {
            cars.add(new Car());
        }
    }
}
  • InputView혹은 UI 에서 도메인을 직접 접근하는 경우, 나중에 다른 UI나 API가 생길 때 그 곳에서도 도메인을 직접 관리해야 하는 문제
  • InputView에서는 값을 입력받는 역할을 하고, 도메인 생성은 ControllerRacing 에서 관리하면 코드 재사용 가능
  • Racing에서 자동차 생성을 하도록 옮김

출력에 대한 고민

  • getCars.get(0).getPosition();
    • 도메인에서 UI로 넘어오는 시점에는 Cars를 그대로 전달하지 않고 List<Car>를 꺼내서 전달
    • UI에서는 비즈니스 로직이 없어야 함
  • Car에서 PositionName을 전달할 때 값 객체로 전달해야 하는지(출력할 때 result.getPosition().getPosition())
    1. 도메인에서는 가능하면 값을 표현하는 객체를 사용하는 것이 좋다고 생각
    2. 도메인에서는 UI 등 출력화면에 대해 의존하지 않아야 한다고 생각
    3. → 도메인에서 사용할 수 있는 getter 에서는 값 객체를 반환하고, 타입을 변환하는 것은 UI로 전달하는 객체에서 변경

테스트하기 위해 public으로 열어주는 것에 대한 고민

  • 테스트를 위해 프로덕션의 코드가 변경되면 좋지 않다고 생각
  • 테스트 코드가 프로덕션 코드에 영향을 준다는 것은, 프로덕션 코드가 테스트 코드에 의존한다는 것과 같다고 생각

자동차의 위치를 값 객체를 이용해서 관리

  • 값 객체란?
  • 위치의 특성 상 값의 범위에 제한이 있음
  • Position에게 전진하라고 이야기하면 다음 위치를 반환
public class Position {
    public Position move() {
        return new Position(this.position + 1);
    }
}
  • 자동차 초기화 위치
public class Position {
    public static final Position ZERO = new Position(0);
}

public class Car {
    public Car(Name name) {
        this.name = name;
        this.position = Position.ZERO;
    }
}

사용자의 입장에서 생각

//before
public void move(int random) {}
//after
public Position move(boolean isMovable) {}
  • move를 호출하는 사용자가 임의의 값을 전달하는지 move메소드가 알 수 있을까?
  • MoveStrategy를 만들어 임의의 값을 전달하는 대신 움직일 수 있는 지의 여부를 전달

try with resources 문법

  • Scanner의 사용이 끝나면 자원 닫기
public static GameRequestDto inputGameRequest() {
        try (Scanner scanner = new Scanner(System.in)) {
            return new GameRequestDto(inputCarCount(scanner), inputTimes(scanner));
        }
}

난수 생성으로 인한 테스트 어려움 해결

//before
public class Racing {

    private final Random random = new Random();

    public void race() {
        for (Car car : cars) {
            car.move(random.nextInt(10));
        }
    }
}

//after
public List<Position> race(MoveStrategy moveStrategy) {
        List<Position> result = new ArrayList<>();
        for (Car car : cars) {
            result.add(car.move(moveStrategy.isMovable()));
        }
        return result;
    }

public MoveStrategy(RandomNumber randomNumber) {
        this.randomNumber = randomNumber;
}

public boolean isMovable() {
        return numberGenerator.generate() >= MOVABLE_NUMBER;
    }
  • Racing에게 임의의 값을 반환하거나, 항상 자동차가 이동할 수 있는 값을 반환하도록 주입
  • MoveStrategy를 만들 때 난수 생성 객체를 외부에서 주입
  • 값 생성에 대해 인터페이스를 추출하고, 람다를 이용
@FunctionalInterface
public interface NumberGenerator {
    int generate();
}

//테스트에서
NumberGenerator numberGenerator = () -> 4;

race의 결과를 반환

public List<Result> race(MoveStrategy moveStrategy) {
    List<Result> result = new ArrayList<>();
            for (Car car : cars) {
                car.move(moveStrategy.isMovable());
                result.add(new Result(car.getPosition(), car.getName()));
            }
            return result;
}
  • 출력하는 화면에서 Car 에 대한 의존성도 끊고, Racing 에게 Cars를 달라는 요청도 안 할 수 있음

자동차 목록을 관리하는 일급 컬렉션 클래스

  • 차를 이동할 때마다 매번 최대 위치를 알아야 할 필요가 있을까?
  • 우승자를 선발하는 시점에만 값을 알아도 괜찮지 않을까?
  • 해당 목록에 포함된 자동차 중 가장 앞에 있는 자동차 목록을 가져오는 역할
public List<String> getWinners() {
        List<String> winners = new ArrayList<>();
        chooseWinners(winners, getMaxPosition());
        return winners;
    }

    private int getMaxPosition() {
        int maxPosition = 0;
        for (Car car : cars) {
            maxPosition = Math.max(maxPosition, car.getPosition());
        }
        return maxPosition;
    }

    private void chooseWinners(List<String> winners, int maxPosition) {
        for (Car car : cars) {
            if (isWinner(car, maxPosition)) {
                winners.add(car.getName());
            }
        }
    }

    private boolean isWinner(Car car, int maxPosition) {
        return car.getPosition() == maxPosition;
    }

유효성 검사 null, isEmpty, length

return name != null && !name.isEmpty() && name.length() < MAX_NAME_LENGTH;

Position에서 Comparable을 구현

  • Position 간의 비교를 좀 더 쉽게
// before
private int getMaxPosition() {
        int maxPosition = 0;
        for (Car car : cars) {
            maxPosition = comparePosition(maxPosition, car.getPosition());
        }
        return maxPosition;
    }

    private int comparePosition(int maxPosition, Position position) {
        return Math.max(maxPosition, position.getPosition());

// after
private Position getMaxPosition() {
        return cars.stream().map(Car::getPosition).max(Position::compareTo).orElse(Position.ZERO);
}

//Position
@Override
    public int compareTo(Position o) {
        return Integer.compare(this.position, o.position);
}

Car의 입장에서 봤을 때, maxPosition으로 넘어오는 값이 최댓값인지 알 수 없음

// before
public boolean isWinner(Position maxPosition) {
    return position.equals(maxPosition);
}

// after
public boolean isSamePosition(Position position) {
        return this.position.equals(position);
    }

cars 에 null 이 들어오면 다른 메소드에서 예외가 발생할 것, 방어 코드 추가

public Cars(List<Name> names) {
        if (names == null) {
            throw new IllegalArgumentException("자동차를 생성할 수 없습니다.");
        }
        generateCars(names);
    }

로또

ParameterizedTest

Calculator calculator = new Calculator();

@ParameterizedTest
@CsvSource(value = {"1 + 2, 3", "1 + 2 * 3, 9", "1 + 2 * 4 / 2, 6"})
void calculate(String input, int expected) {
    Expression expression = new Expression(input);
        assertThat(calculator.calculate(expression.getNumbers(), expression.getOperator())).isEqualTo(expected);
}

@ParameterizedTest
@NullAndEmptySource
void validateExpressionIsBlank(String input) {
    assertThatExceptionOfType(IllegalArgumentException.class)
            .isThrownBy(() -> {
                new Expression(input);
            });
}

@ParameterizedTest
@ValueSource(ints = {-1, 0, 46})
void validateLottoNumber(int input) {
        assertThatExceptionOfType(IllegalArgumentException.class)
                .isThrownBy(() -> {
                    new LottoNumber(input);
                });
    }

메소드가 호출될 때마다 values() 메소드가 호출→변하지 않는 값이기 때문에 static 으로 선언

//before
public static Operator of(String operation) {
        return Arrays.stream(values())
                                .filter(operator -> operator.operation.equals(operation))
                .findFirst()
                .orElseThrow(() -> new IllegalArgumentException("사칙연산 기호가 아닙니다."));
    }

//after
private static final Operator[] VALUES = values();

불필요한 인스턴스 생성을 막기 위해 기본 생성자를 private로 선언

private InputView() {
    }

컬렉션을 리턴할 때 불변으로 리턴

//before
public List<LottoNumber> getLotto() {
        return lotto;
    }

//after
return Collections.unmodifiableList(ranks);

LottoGenerator에서 생성할 때 set 으로 생성

  • size 만 확인하면 두번 검증할 필요가 없음
Set<Integer> lottoNumbers = fullLottoNumbers.stream()
                .limit(Lotto.LOTTO_SIZE)
                                .collect(Collectors.toCollection(TreeSet::new));

Lotto에 생성자나 팩토리 함수를 통해 생성

//before
splitLotto를 InputView에서 하고있었음

//after
public Lotto(String winningNumbers) {
        this(splitLotto(winningNumbers));
    }

private static Set<Integer> splitLotto(String winningNumbers) {
    return Arrays.stream(winningNumbers.split(","))
            .map(String::trim)
            .map(Integer::parseInt)
            .collect(Collectors.toSet());
}

사다리

밖에서 Height와, Line을 만들어 주입할지, 안에서 생성할지 고민

public Ladder(int directionCount, int height, DirectionGenerator directionGenerator) {
        this.height = new Height(height);
        create(directionCount, directionGenerator);
    }
  • Ladder를 사용하는 입장이라면 굳이 Line, Height 객체를 모르고 숫자 값 (int) 만 알아도
    Ladder를 사용할 수 있으니까 편리할 것 같습니다. 라는 피드백
  • 사용자의 입장에서 생각해야겠다는 생각을 했다.

정적 상수 대문자 컨벤션

private static final Random random = new Random();
-> private static final Random RANDOM = new Random();

position도 원시객체이므로 객체로 추출

  • 객체로 분리하면서 꽤 많은 로직이 Position에 생성되었고,
  • 객체의 역할에 맞게 테스트를 진행할 수 있었다.
public static Position first() {
      return new Position(FIRST_POSITION);
}

public Position goNext() {
    return new Position(position + 1);
}

public Position goBack() {
    return new Position(position - 1);
}

'테스트코드' 카테고리의 다른 글

ATDD, 클린 코드 with Spring 피드백 정리  (0) 2024.03.17