[우아한테크코스] LV1 - 사다리 타기 미션 회고
우아한테크코스 Lv1 두 번째 미션은 사다리 타기 게임을 구현하는 것이었다. step1은 사다리 게임에 필요한 플레이어 이름과 사다리 높이를 입력받아 게임에 사용될 사다리를 생성하여 출력하는 것이 목표였다. 그 다음 step2는 게임 결과 값을 추가로 입력 받아 생성한 사다리를 기반으로 참가자들의 게임 결과를 계산하고 출력하는 것이 목표였다.
step1은 페어 프로그래밍으로 진행되었으며 이번 나의 페어는 오션이었다. 하지만 하루 페어 프로그래밍을 진행하고 바로 코로나 확진 판정을 받아서 step1 미션은 개별적으로 진행하게 되었다...😭 이러한 이유로 이번에는 거의 혼자서 step1과 step2를 진행하게 되었다.
step1
[디미터의 법칙]
이번 미션에서 모든 원시 값과 문자열을 포장한다는 새롭게 추가된 요구사항이 있었다. 그 덕분에 기존에 원시 값으로 표현하던 데이터들을 모두 Wrapper 클래스로 객체화 하면서 겪었던 고민들이 굉장히 많았는데 그 중 하나는 연쇄적인 getter 사용으로 인해 코드가 난잡해진 것이었다. 이번 리뷰어였던 코다는 이 부분에 대해 디미터의 법칙을 위반한 것이라고 말해주었고 리팩토링할 수 있도록 도와주셨다. 해당 과정에서 디미터의 법칙에 대해 정리한 부분을 공유하고자 한다.
디미터의 법칙은 최소한의 지식 원칙(The Principle Of Least Knowledge)을 강조하며, 모듈은 사용하는 객체의 속사정을 몰라야 한다는 원칙이다.
이렇게만 봤을 때는 의미가 굉장히 추상적으로 다가왔었는데 이를 조금 더 구체화 시켜서 정리해보면 다음과 같다.
디미터의 법칙 기본적 요약 : 어떤 클래스 C의 메서드 f 내부에서는 다음과 같은 객체의 메서드만 호출해야 한다.
- 클래스 C의 메서드
- 클래스 C가 저장하고 있는 인스턴스 객체의 메서드
- f 의 매개변수로 넘겨받은 객체의 메서드
- f 내부에서 생성된 객체의 메서드
위와 같은 법칙을 지켰을 때 우리는 객체 간의 결합도를 낮추고 자율성을 높여 객체 간 올바른 협력관계를 구축할 수 있다는 것이 결론이다. 그러면 step1에서의 코드가 어떻게 바뀌었는지 보면서 과연 그러한지 생각해보자.
// 결과값 출력을 위해 참가자 이름을 우측 정렬하는 메서드
private String makeRightFormattingNamesFromSecond(Players players) {
return IntStream.range(STARTING_INDEX_OF_RIGHT_FORMATTING, players.size())
.mapToObj(players::getPlayer)
.map(Player::getName)
.map(name -> String.format("%5s", name.getName()))
.collect(Collectors.joining(" "));
}
위 메서드는 outputView에서 사다리 생성 결과를 출력할 때 참가자 이름 출력값들을 우측 정렬시켜주는 기능을 수행한다. 해당 부분에 대한 코다의 리뷰는 다음과 같았다.
이 부분이 히이로가 언급해주신 첫번째 질문에 대한 내용인 것 같은데요,
일급컬렉션을 사용한 것은 좋지만, 지금 mapToObj -> map -> map 연달아 호출하며 내부구현을 View에 노출하고 있다고 볼 수 있을 것 같아요 ㅎㅎ 디미터의 법칙 을 참고해보세요.
현재 코드는 view에게 players 내부 구성이 어떻게 되어 있는지 map 메서드 체이닝을 통해 드러나고 있다. 이는 view가 Players 뿐만 아니라 Player, Name 객체에 대해서도 강한 결합성을 가지도록 하는 원인이 된다. 만약 Players, Player, Name 객체들에 대해서 어떤 수정사항이 발생했을 때 view에도 수정사항이 생길 확률이 높아지게된 것이다. view가 어느정도 domain에 의존하는 것은 어쩔 수 없는 부분이지만 디미터의 법칙을 공부하고 나니 지금 같은 경우는 필요 이상으로 의존하고 있다는 생각이 들었다.
이에 대해 디미터의 법칙을 준수하고 view와 domain의 의존성 수준을 낮추기 위해 정리한 나의 생각은 다음과 같았다.
- Player가 생성자에 의해 초기화 될때는 외부에서 Name이라는 Wrapping 객체를 받아 저장하고 있다. 그러나 Player가 외부로 자신의 이름을 반환해야 할 때에도 과연 Name Wrapping 객체 그대로 넘겨야 할까?
- 분명 플레이어가 외부로 자신의 이름을 반환할 때는 이름 그 자체 문자열이 필요한 경우일 가능성이 거의 100프로이다.
- 따라서 Wrapping 객체인 Name을 벗겨내고 내부 문자열 이름 값을 꺼내서 반환하는 역할을 Player 내부에서 수행해야 한다는 결론으로 귀결된다.
- 그러면 외부로 Name 클래스를 벗겨내는 책임을 미루지 않고 바로 처리할 수 있게 되므로 외부에서 Name 으로 인해 디미터의 법칙이 깨졌던 부분을 수습할 수 있게 된다.
이러한 사고과정을 거쳐 Player 클래스 내부에서 이름을 반환할 때 Name 클래스 내부의 String 이름 값을 반환하도록 리팩토링하였다.
public class Player {
private final Name name;
public Player(final Name name) {
this.name = name;
}
// Name 클래스를 unwrap 하여 반환한다.
public String getName() {
return name.getName();
}
}
또한 일급 컬렉션인 Players 클래스에서도 모든 참가자의 이름을 반환하는 경우를 고려하여 내부에서 참가자의 String 이름 값을 컬렉션에 새로 담아서 반환할 수 있도록 하였다.
public class Players {
private final List<Player> players = new ArrayList<>();
public Players(Names names) {
IntStream.range(0, names.size()).forEach(index ->
players.add(new Player(names.getName(index)))
);
}
// Player 내부 요소 값을 꺼내주는 역할
public List<String> getAllPlayerNames() {
return players.stream()
.map(Player::getName)
.collect(Collectors.toList());
}
}
이렇게 하니 OutputView에서 지고 있던 String 이름 값을 찾아가기 위한 책임을 각 객체들에게 분담할 수 있었고 결과적으로 디미터의 법칙도 지킬 수 있게 되어 domain에 대한 view의 의존도를 낮출 수 있었다.
private String makeRightFormattingNamesFromSecond(List<String> allPlayerNames) {
return allPlayerNames.subList(STARTING_INDEX_OF_RIGHT_FORMATTING, allPlayerNames.size()).stream()
.map(name -> String.format(RIGHT_FORMATTING_TEMPLATE, name))
.collect(Collectors.joining(" "));
}
위 코드들은 step2에서 출력을 위해 Wrapper 클래스들을 unwrap하는 책임을 view에게 위임하면서 다시 변경되었다. 하지만 핵심적인 부분은 한 메서드 내에서 어떤 객체의 내부 구조에 대해 깊숙하게 알고 있다면 그 책임을 분산시킬 필요가 있다는 의미를 남기고자 정리하였다.
[Inner 클래스는 static을 붙인다]
테스트 코드에서 전략 패턴으로 사용할 테스트용 클래스를 테스트 클래스 내부에 선언해주었다.
public class LineTest {
private class TestPointGenerator implements PointGenerator {
private final List<Boolean> points;
public TestPointGenerator(List<Boolean> points) {
this.points = new ArrayList<>(points);
}
@Override
public boolean generate() {
return points.remove(0);
}
}
}
해당 테스트 클래스 내부에서만 사용하기에 private 접근 제어자만 선언해주고 넘어갔었다. 이에 대해 코다는 inner 클래스에는 왜 static을 붙여주어야 하는지 생각해보면 좋을 것 같다고 리뷰해주었다.
레퍼런스들을 찾아보며 정리한 내용은 다음과 같다.
- 왜 Inner class에 static을 붙이는 것이 좋을까?
- 내부 클래스를 static으로 선언하지 않으면 내부 클래스는 생성시 외부 클래스에 대한 '숨은 외부 참조' 현상을 발생시키게 된다.
- 이렇게 되면 외부 클래스는 실제로 사용되지 않음에도 내부 클래스에서 숨은 외부 참조를 하고 있기 때문에 GC의 수거 대상에서 제외된다.
- 결국 GC에 의해 수거되지 않는 외부 클래스는 메모리 누수 문제의 원인이 된다.
- 따라서 내부 클래스가 외부 클래스의 멤버를 가져와서 사용해야 하는 경우가 아니라면 반드시 static으로 선언해주는 것이 좋다.
이것에 대해 조금 더 보충설명을 붙여보자면 static으로 선언되지 않은 inner 클래스는 스스로 인스턴스를 유지하기 위해서는 outer 인스턴스에 대한 참조를 할 수 밖에 없다는 것이다. 다른 레퍼런스를 더 찾아보니 inner 클래스 코드가 바이트 코드로 컴파일 되었을 때 inner 클래스의 생성자는 outer 클래스 객체를 매개변수로 받아서 초기화 한다고 한다. 이러한 이유로 inner 클래스에서 outer 클래스에 대한 참조가 필요한 경우가 아니라면 inner 클래스는 반드시 static을 붙여주는 것이 좋다.
step2
[ Factory 패턴]
기존 step1에서 참여자의 이름과 게임 결과 값을 각각 wraping하는 Name 클래스를 구현하였다. 그리고 사용자 입력을 받아서 검증하고 Name 객체들을 생성하는 Names 클래스를 구현하였다.
public class Names {
private static final String NAMES_DELIMITER = ",";
private final List<Name> names = new ArrayList<>();
public Names(String inputNames) {
splitInputNames(inputNames).stream()
.map(String::strip)
.forEach(name -> names.add(new Name(name)));
}
public Name getName(int index) {
return names.get(index);
}
public List<String> getValues() {
return names.stream()
.map(Name::getValue)
.collect(Collectors.toList());
}
public int size() {
return names.size();
}
private List<String> splitInputNames(String inputNames) {
return Arrays.asList(inputNames.split(NAMES_DELIMITER));
}
}
하지만 "입력 값이 형식에 맞게 들어왔는지 확인하고 Wrapper 객체들을 만들어서 반환해주는 역할을 하는 클래스가 일급 컬렉션일까?" 라는 의문이 들었다. 그리고 "객체들을 생성하는 역할을 해주는 클래스가 뭐지?" 라는 의문을 통해 이게 Factory 클래스 아닐까? 라는 생각까지 이르게 되었다.
public class NameFactory {
private static final String DUPLICATED_NAME_ERROR = "[ERROR] 참가자 이름은 중복될 수 없습니다.";
private static final String NAMES_DELIMITER = ",";
public static List<Name> create(String inputNames) {
List<String> names = splitInputNames(inputNames);
validateDuplicatedNames(names);
return names.stream()
.map(Name::new)
.collect(Collectors.toList());
}
private static List<String> splitInputNames(String inputNames) {
return Arrays.stream(inputNames.split(NAMES_DELIMITER))
.map(String::strip)
.collect(Collectors.toList());
}
private static void validateDuplicatedNames(List<String> names) {
if (names.size() != getDistinctCount(names)) {
throw new IllegalArgumentException(DUPLICATED_NAME_ERROR);
}
}
private static int getDistinctCount(List<String> names) {
return (int) names.stream()
.distinct()
.count();
}
}
그래서 해당 클래스를 VO 객체들을 생성하여 반환하는 Factory 클래스로 리팩토링하였다. 하지만 이게 정확한 팩토리 클래스의 개념으로 사용한 것일까? 라는 의문은 지우기 힘들었고 해당 부분에 대해 코다에게 질문을 드렸다. 아래는 코다의 답변이다.
히이로 말씀대로 입력 형식을 확인하고 파싱하는 역할만 일급컬렉션에서 가지고 있으면 좀 아쉬울 것 같아요~
고민하신 부분은 매우 공감합니다. 하지만 팩토리 클래스 구현에 대해 몇가지 질문이 있어요.
1. 팩토리 클래스는 어떤 역할을 담당하며 왜 사용하는 건가요 ?
2. 지금 구현하신 팩토리 클래스의 역할은 model과 view 중 무엇에 더 가까운가요?
다음은 팩토리 클래스에 대해 던져주신 질문들을 고민하면서 사고한 과정이다.
- 팩토리 클래스는 어떤 역할을 담당하며 왜 사용하는 건가요?
- 팩토리 클래스는 객체를 생성하는 책임을 담당하는 클래스를 의미합니다.
- 팩토리 패턴을 사용하는 방식에는 팩토리 메서드 패턴과 추상 팩토리 패턴이 존재합니다.
- 팩토리 메서드 패턴은 어떤 객체를 생성하는 책임을 수행하는 팩토리 클래스를 추상화하고 해당 팩토리 클래스(슈퍼클래스)에서 어떤 타입의 객체를 생성하는지는 하위 팩토리 클래스(서브클래스)가 결정하도록 위임하는 방식을 말합니다.
- 추상팩토리 패턴은 생성하는 객체들 중 연관되는 객체들을 모아서 생성할 수 있도록 추상화된 팩토리 클래스(슈퍼클래스)에 이를 명시하고 연관된 객체들의 타입을 결정하여 실제 생성하는 책임은 하위 팩토리 클래스(서브클래스)가 결정하도록 위임하는 방식을 말합니다.
- 즉 추상팩토리 패턴은 팩토리 메서드 패턴을 사용하는 방식입니다.
- 단순하게 팩토리 클래스가 어떤 객체를 생성하는 책임을 가지는 클래스라는 관점에서 봤을 떄는 지금 구현한 방식이 맞습니다.
- 하지만 팩토리 패턴이라는 것을 학습한 결과 생성하고자 하는 객체의 생성부를 캡슐화하여 결합도를 느슨하게 하고 구체적인 객체 타입에 의존하지 않도록 하는 것임을 알았습니다.
- 이런 관점에서 봤을 때 클래스 명이 Factory임에도 불구하고 단순 객체 생성의 책임을 수행할 뿐 위와 같은 책임을 수행하는 것은 아니라는 생각이 들었습니다.
- 지금 구현하신 팩토리 클래스의 역할은 model과 view 중 무엇에 더 가까운가요?
- 현재 구현한 팩토리 클래스의 역할은 사용자의 입력값을 받아서 파싱 및 검증과정을 거친 후 VO 객체들을 생성하는 책임을 갖고 있습니다.
- 여기에서 사용자의 입력값을 받아서 파싱 및 검증과정을 거친 후 VO 객체들을 생성하는 책임은 충분히 view에서 수행할 수 있다고 생각이 들었습니다.
- 오히려 팩토리 패턴을 사용하는 이유에 대해서 학습하고 다시 생각해보니 특별한 이유 없이 view의 책임을 도메인이 과도하게 지게 된 것 같다는 생각이 듭니다.
결과적으로 Factory 클래스에서 수행하던 기능들을 모두 view에게 위임하는 방향으로 리팩토링하게 되었다. 예전에도 그렇고 지금도 그렇고 inputView에서 수행할 수 있는 책임을 과도하게 domain에서 책임지려고 하는 관성이 있다. 앞으로의 미션에서 다음과 같은 기능은 view에서 수행할 수 있도록 의식적으로 노력해봐야겠다.
- 입력값을 파싱하고 검증하는 기능
- 입력값을 wrapping하는 기능
- wrapping 객체를 unwrap하는 기능
[코드가 언제나 의도대로 사용되는 것은 아니다]
원시타입을 포장하기 위해 VO를 구현하면서 의문점이 하나 들었다.
출력을 위한 getter를 구현해두고 해당 객체의 속성값이 일치하는지 판단하는 메서드는 따로 만드는 것이 옳은가?
예를 들어 VO인 Name 객체는 다른 Name 객체와 동일한 값을 갖는지 확인하는 isSame() 메서드를 구현해 참/거짓 값을 반환한다. VO도 객체이기에 메세지를 던져 책임을 수행하는 방향이 옳다고 생각했기에 해당 방식으로 구현을 했다.
public class Name {
private final String name;
public Name(String name) {
this.name = name;
}
// 출력을 위한 getter
public String getName() {
return name;
}
// 다른 Name 객체와 비교를 위한 메서드
public boolean isSame(Name other) {
return this.name.equals(other.name);
}
}
위 경우처럼 구현하면 Name 객체에 메세지를 던지는 좋은 케이스가 되겠지만... 출력을 위한 getter가 Name 내에 있어서 다른 개발자는 getter를 사용해 내부 값을 반환받아 사용해 버린다면 어떡하나라는 생각이 문득 들었다. 처음 의도했던 대로 메서드들이 사용되지 못할 것 같은데 이런 경우는 어떻게 하는 것이 좋은지 궁금했고 해당 부분을 코다에게 질문드렸다.
코다는 개발적인 정답은 정해져있지 않고 오히려 비개발적인 자세를 강조하며 현업에서도 여전히 숙제인 부분이라고 얘기를 해주셨다. 그래서 같이 개발하는 페어 혹은 동료 개발자와 커뮤니케이션이 매우 중요하며 이런 컨벤션적인 부분이 너무 맞지 않는다면 클린코드, 리팩토링에 대한 스터디를 진행하며 서로 맞춰가는 방법도 있다고 하셨다.
또는 단일 클래스를 설계할 때 책임을 적절하게 분배하여 클래스 단위를 작게 한다면 동료 개발자로 하여금 해당 클래스가 어떤 기능을 어떻게 수행하는지 파악하기 쉽게 만드는 방법도 있다고 하셨다. 그러면 대부분 목적에 부합하는 메서드를 찾아서 사용하게 될 가능성이 높아질 수 있는 것이다. 반대로 단일 클래스의 책임이 너무 커서 메서드가 수십 개 이상 존재하게 된다면 그냥 getter를 써버리게 될 확률이 커진다고 생각할 수도 있겠다.
2번째 미션을 진행하고 있지만 항상 코드 외적인 부분도 절대 경시할 수 없다는 생각을 다시 한번 들게 해준 계기가 되었다. (개발자는 잘해야 하는게 너무 많은 것 같다...)
[equals & hashCode]
vo 객체 간 비교를 해야하는 기능을 구현하면서 equals와 hashCode를 언제 재정의해줘야 하냐는 고민을 하게 되었다. 원래 알고 있던 것으로는 Hash를 사용한 자료구조인 HashMap, HashSet, HashTable 등을 사용자 정의 객체에 대해 사용할 때 재정의를 해줘야 하는 것으로만 알고 있었다.
하지만 Name의 논리적 동치성을 보장하기 위해서는 equals를 재정의 해줄 필요성이 있다는 것을 알게 되었다. 도메인 로직 측면에서 사다리 게임 참여자는 이름이 같으면 같은 사람으로 취급되어야 한다. 즉 동명이인은 존재해서는 안된다.
// 다른 Name 객체와 비교를 위한 메서드
public boolean isSame(Name other) {
return this.name.equals(other.name);
}
현재 코드에서 Name 객체들을 상호 비교하는 기능은 isSame 메서드를 구현하여 같은 이름을 가진 Name 객체인지 확인하고 있다. 하지만 equals 메서드를 재정의해주지 않아서 같은 이름을 가지는 Name 객체지만 인스턴스 주소 값이 달라 equals 메서드에서는 다른 객체라고 판단하는 문제가 발생한다. 따라서 같은 이름 값을 가지는 Name객체들은 equals 메서드를 사용해도 동일한 객체라고 판단할 수 있도록 재정의하였다.
이를 통해 equals 메서드를 재정의하는데에는 도메인적 측면에서 논리적 동등성이 확보되었는지도 고려하는 것이 반드시 필요하다는 것을 알게 되었다.
[TDD를 진행하며...]
TDD 방식으로 이번 미션을 진행하다보니 기능이 구현되지 않은 상태에서 해당 기능을 어떻게 검증해야 할지가 굉장히 고민이 되었다. 특히 객체 내부 상태 값을 조작하는 기능을 테스트하고자 할 때 더 고민이 됐다.
단순하게만 생각한다면 getter 메서드를 사용하여 변화한 내부 상태값을 반환받아 assertThat 구문을 사용하면 간단하다. 그런데 만약 해당 객체가 출력을 위한 getter 구현 의무가 없다고 한다면 해당 getter 메서드는 오로지 테스트만을 위해 존재하게 된다고 생각이 들었다.
하지만 TDD는 최대한 빠르게 실패하는 테스트를 만들고 이를 통과시키기 위한 코드를 작성하는 것이 핵심이다. 리팩토링 과정을 거치면서 삭제하면 되지 않겠냐는 생각도 들긴 했지만 애시당초 getter가 필요하지 않은데 작성하게 되는 과정 자체가 마음에 걸렸다.
코다는 이에 대해서 미리 기능의 스펙을 고민하고 정의하는 것이 중요하다고 말해주셨다. 즉 어떤 객체에 어떤 책임(public 메서드)가 있을지, 어떤 메세지를 반환해야 할 지 미리 고민하는 것이 가장 중요하다고도 덧붙여주셨다. 맨 처음부터 구현되지 않은 객체의 책임을 정한다는 것은 굉장히 어려운 일이다. 특히 나는 나중에 전체 프로젝트 안에서 어떻게 동작하게 될 지 무의식적으로 생각하며 TDD로 진행을 해왔다.
하지만 이번 사다리 타기 피드백 강의에서 in->out, out->in 접근방식에 대해 알게 되면서 생각을 많이 바꿔보게 되었다. 어플리케이션 개발 과정에서 in->out, out->in 접근 방식은 모두 사용된다. 맨 처음 도메인 로직에 대한 이해가 없을 때는 눈에 보이는 큰 개념부터 접근해서 TDD를 시작해보는 것이다. 요구사항을 분석하며 이를 기반으로 객체를 도출하는데 이 과정에서 점점 도메인 지식이 쌓이면서 점점 더 작은 단위로 객체를 분리하게 된다. 이 과정을 반복하면서 더 이상 분리할 수 없는 객체까지 작아졌다는 확신이 들면 여기부터 in->out 접근 방식으로 구현해 올라가기 시작한다.
나는 이번 미션에서 TDD를 진행할 때 in->out 방식으로 구현하면서 더 상위 개념의 객체들에서 지금 구현하는 하위 객체가 어떻게 사용될 지를 너무 많이 고민했던 것 같다. 이것은 네오가 말하기를 큰 틀이 잡힌 상태로 작은 것을 구현하게 되는 것이고 실시간으로 레거시를 만드는 것과 동일한 것이라 말했다. 작은 단위부터 시작하는 것은 하나의 개념으로 독립적으로 생각할 수 있기 때문에 그렇게 시작하는 것인데 이 시점에 상위 개념을 고려하게 되면 상위 개념에 맞춤화된 하위 객체를 만들게 될 가능성이 높아진다고 하며 잘못 설계된 객체를 만드는 것이라 했다.
결론적으로 상위 개념은 그저 작은 단위를 찾기 위한 과정에 불과한 것이다. 그래서 좀 극단적일 수도 있겠지만 기존에 작성하던 상위 개념 객체의 TDD 코드를 삭제하고 작은 객체부터 다시 시작하는 방법이 더 나은 경우가 많다고도 한다. 중요한 것은 상위 개념이 하위 개념 구현의 발목을 잡으면 안된다는 것이다.
앞으로 TDD를 진행하면서 진도가 쉽사리 나가지 못하고 있는 스스로를 인지했을 때 너무 상위 개념을 끌어와서 고민하고 있지는 않은지 항상 점검할 수 있도록 해야겠다.
Reference
- https://prohannah.tistory.com/204#:~:text=%EB%94%94%EB%AF%B8%ED%84%B0%EC%9D%98%20%EB%B2%95%EC%B9%99(Law,%EB%AA%B0%EB%9D%BC%EC%95%BC%20%ED%95%9C%EB%8B%A4%EB%8A%94%20%EB%B2%95%EC%B9%99%EC%9D%B4%EB%8B%A4.)
- https://inpa.tistory.com/entry/JAVA-%E2%98%95-%EC%9E%90%EB%B0%94%EC%9D%98-%EB%82%B4%EB%B6%80-%ED%81%B4%EB%9E%98%EC%8A%A4%EB%8A%94-static-%EC%9C%BC%EB%A1%9C-%EC%84%A0%EC%96%B8%ED%95%98%EC%9E%90