이번 초록 스터디 사다리 미션의 1단계 리뷰 과정에서는 테스트가 용이한 코드란 무엇일까?에 대해 깊이 고민할 수 있는 기회가 되었습니다.
미션 설명
사다리 미션의 1단계는 4x4 사다리를 생성하는 것이었습니다.
4x4 사다리를 위의 그림과 같이 생성을 하되 핵심은 |-----|-----| 모양이 포함되지 않는 것이었습니다. (|-----|-----| 이 포함될 경우 이후 단계에서 사다리를 탈때 어느 특정 방향으로 이동하는 것이 어렵기 때문에 포함되지 않게 구성해야 했었습니다)
대부분의 스터디원은 크게 Point, Line, Ladder 객체로 사다리를 관리했습니다.
Point 객체 : 건널 수 있는지 혹은 건널 수 없는지를 나타내는 객체
Line 객체 : Point의 일급컬렉션으로 사다리 중 가로 한줄을 나타내는 객체
Ladder 객체 : Line의 일급컬렉션으로 사다리 전체를 나타내는 객체
코드리뷰하기 전 요구사항을 읽은 후 나의 생각
이 요구사항을 보고 가장 먼저 떠오른 생각은, 사다리가 생성될 때 |-----|-----| 와 같은 구조를 포함하고 있는지를 우선으로 검증하는 것이 필요하겠다고 생각했습니다.(Line 객체에서 연속된 두 Point 객체가 모두 true를 가지고 있는지 검증 필요) 더불어 그 부분을 테스트하는 것이 이번 미션의 핵심이라고 생각했습니다. 따라서 이번 코드 리뷰에서는 해당 검증이 코드로 나타나 있는 것을 우선으로 생각을 했고 그 다음으로는 테스트 코드로 직접 테스트가 되어 있는지를 차순으로 생각했습니다.
스터디원들의 코드리뷰를 하면서 생각한 개선방향
위와 같이 제 나름의 가이드라인을 가지고 코드리뷰를 진행했습니다. 코드 리뷰 결과 스터디원 세 분이 작성한 코드에는 공통점이 있었습니다. 세 분 모두 Line 객체에서 연속된 건널목에 대한 검증 과정이 포함되어 있지 않았습니다. 대신 스터디원들은 Line을 생성하는 과정에서 연속된 건널목이 생성되지 않게 코드를 통해 Line을 생성했습니다.
한 스터디원의 코드를 예시로 들겠습니다.
public class Line {
private final List<Boolean> points;
public Line(int lineSize) {
points = generatePoint(lineSize);
}
private List<Boolean> generateLine(int lineSize) {
// 연속된 건널목이 없이 생성(Random 객체 사용)
Random random = new Random();
List<Boolean> points = new ArrayList<>();
for (int i = 0; i < lineSize; i++) {
generatePoint(points, random);
}
}
private void generatePoint(List<Boolean> points, Random random) {
if (points.isEmpty()) {
points.add(random.nextBoolean());
return;
}
generateNext(points);
}
private void generateNext(List<Boolean> points, Random random) {
boolean isConnected = false;
int lastIndex = points.size() - 1;
if(!points.get(lastIndex)){
isConnected = random.nextBoolean();
}
points.add(new Point(isConnected));
}
}
// 테스트 부분
@DisplayName("사다리 행 연결 중복 테스트")
@Test
public void lineDuplicationTest() {
Line line = new Line(4);
assertThat(isDuplicate(line)).isEqualTo(false);
}
public boolean isDuplicate(Line line) {
boolean result = false;
boolean before = false;
for(Point point : line.getPoints()) {
result = result || (before && point.isConnected());
before = point.isConnected();
}
return result;
}언뜻 보면 현재 코드는 요구 상황에 맞게 동작하고 테스트 결과도 올바르게 나타나지만 제 생각에는 좋은 테스트 방식은 아니라고 생각했습니다.
- 요구사항에 변화의 변경점이 2개가 된다.
위와 같은 상황에서 요구사항이 변경된다면 Line의 요소를 생성하는 generatePoint() 메서드와 테스트 코드 내부에 연속된 건널목 유무를 검증하는 isDuplicate() 메소드가 수정될 것입니다. 테스트 코드 역시 비즈니스 코드와 마찬가지로 하나의 리소스라고 생각하기 때문에 변경에 민감하지 않게 코드를 구성하는 것이 좋다고 생각합니다.
- 제어불가능한 부분이 테스트에 포함되어 있다.
현재 generateLine() 메소드를 보면 내부에 Random 객체를 활용해서 사다리의 건널목을 추가할지 말지를 랜덤하게 정해나가고 있습니다. 그렇기에 테스트에서는 이 부분을 제어하기 어렵다고 생각합니다. 물론 현재의 코드는 많이 복잡하지 않기에 비즈니스부분의 코드를 보면 손쉬게 연결된 건널목이 등장하지 않을 것이라는 것을 알 수 있습니다. 하지만 로직이 복잡해진다면 이를 파악하는데 어려울 수 있다고 생각했습니다. 따라서 Line 객체의 핵심인 연속된 건널목의 유무만 판단하면 되는 것이기에 외부에서 완성된 Boolean 리스트를 받아서 연결된 건널목 유무만 검증만 하면 좋을 것 같다고 생각했습니다.
이 리뷰 남긴 후 제가 생각한 테스트 코드를 구현해보았고 다음과 같습니다.
public class Line {
private final List<Boolean> points;
public Line(List<Boolean> points) {
validateHasContinuousCross(points);
this.points = points;
}
private void validateHasContinuousCross(List<Boolean> points) {
// 연속적으로 건널목이 있는 검증
}
}
@Test
@DisplayName("사다리 가로 한 라인에서 연속으로 건널목이 있으면 예외를 발생한다.")
void constructorTest() {
// given
List<Boolean> points = List.of(true, true, false, true);
// when & then
Assertions.assertThatThrownBy(() -> new Line(points))
.isInstanceOf("[정의한 예외 객체]");
}위의 코드와 같이 테스트를 구성한 이유는 다음과 같습니다.
- 테스트 코드 내용이 직관적으로 확인이 가능하다.
제가 생각하기에 테스트 코드는 작성한 비즈니스 코드가 올바르게 동작하는지 검증하는 역할도 하지만 기능 명세서 역할도 한다고 생각합니다. 제 경험상 저는 협업을 할 때 팀원의 코드가 잘 이해되지 않는 경우에는 테스트 코드를 통해 가이드 라인을 잡고 코드를 이해했었습니다. 그렇기에 테스트 코드를 작성할 때 직관적으로 알아볼 수 있는 것도 중요하다고 생각합니다.
- 테스트의 목적이 명확하다.(제어가 불가능한 부분을 배제했다.)
위의 테스트 코드는 Line 객체가 List을 인자로 받아 연속된 true만 있는지 검증하고 있을 뿐 해당 List가 어떻게 생성되었는지에 대해서는 고려를 하고 있지 않기 때문에 테스트의 목적이 명확하다고 생각했습니다. 또한 외부에서 List를 제공하는 것이기에 우리가 원하는 다양한 케이스를 손 쉽게 정할 수 있습니다. (ex List.of(true, true, true, false) 등 다양한 경우를 빠르게 테스트할 수 있다.)
코드리뷰 후 느낀 점
이번 코드리뷰를 하면서 느낀 점은 제어하기 어려운 부분이 비즈니스 로직에 포함이 되어 있으면 테스트 하는 것이 어렵다는 것을 느꼈습니다. 그렇기에 제어하기 어려운 부분을 외부에서 넘겨받게 코드를 구성하는 것이 다양한 케이스에 대해서 손쉽게 테스트 할 수 있다는 것을 배웠습니다.
