스프링 ATDD 강의를 수강하며 주어진 미션을 구현하면서 복잡한 비즈니스 로직을 추상화한 과정을 기술하겠다.
1. 요구사항
지하철 노선(Line)에 구간(Section)이, 구간에 역(Station)이 포함되어 있다. 노선은 구간 리스트를 갖는다. 하나의 구간에는 상행역, 하행역이 포함되어 있다.
처음에는 구간을 노선의 맨 뒤에 추가하는 것이 요구사항이었다. 맨 뒤에 추가하는 것은 쉽다. 코드 한 줄이면 된다.
sections.add(newSection);
그 다음의 요구사항은 구간을 어느 위치에서나 추가할 수 있게 하는 것이었다. 어느 위치에서든지라는 조건은 세 가지로 나뉜다.
- 처음에 추가하기: 쉬움
- sections.add(0, newSection);
- 중간에 추가하기: 어려움
- sections.add(?, newSection); < 이 한 줄로 될까?
- 끝에 추가하기: 쉬움
- sections.add(newSection);
중간에 구간을 추가하는 것이 코드 한 줄로 해결이 안 되는 이유는, 거리가 있기 때문이다.
위 그림처럼 7m였던 거리를 각각 4m, 3m로 쪼개는 과정이 필요하다.
예를 들어 [방화 - 5 - 마천]만 있는 노선의 가운데에 강동역을 추가하여 [방화 - 3 - 강동 - 2 - 마천]으로 만들려면,
- 새 구간(방화 - 3 - 강동)을 삽입할 위치를 찾기
- 새 구간과 기존 구간 연결하기
- 새 구간을 삽입: [방화 - 강동], [방화 - 마천]
- 구간 연결: [방화 - 강동], [강동 - 마천]
- 거리 쪼개기
- [방화 - 3 - 강동 - 2 - 마천]
2. 개선 과정
@Embeddable
public class Sections implements Iterable<Section> {
...
public void addSection(Section newSection) {
boolean upstationExists = sections.stream().anyMatch(section ->
section.isUpstation(newSection.getUpstation()) ||
section.isDownstation(newSection.getUpstation()));
boolean downstationExists = sections.stream().anyMatch(section ->
section.isUpstation(newSection.getDownstation()) ||
section.isDownstation(newSection.getDownstation()));
if (upstationExists && downstationExists) {
throw new InvalidInputException("새로운 구간의 상행역과 하행역 둘 다 이미 노선에 등록되어 있습니다.");
}
if (!upstationExists && !downstationExists) {
throw new InvalidInputException("새로운 구간은 기존 노선의 역과 최소 하나 이상 연결되어야 합니다.");
}
if (upstationExists) {
int size = sections.size();
for (int i = 0; i < size; i++) {
Section currentSection = sections.get(i);
if (currentSection.isUpstation(newSection.getUpstation())) {
int newDistance = currentSection.getDistance() - newSection.getDistance();
if (newDistance <= 0) {
throw new InvalidInputException("유효하지 않은 거리입니다.");
}
currentSection.setUpstation(newSection.getDownstation());
currentSection.setDistance(newDistance);
sections.add(i, newSection);
return;
}
}
} else {
int size = sections.size();
for (int i = 0; i < size; i++) {
Section currentSection = sections.get(i);
if (currentSection.isDownstation(newSection.getDownstation())) {
int newDistance = currentSection.getDistance() - newSection.getDistance();
if (newDistance <= 0) {
throw new InvalidInputException("유효하지 않은 거리입니다.");
}
currentSection.setDownstation(newSection.getUpstation());
currentSection.setDistance(newDistance);
sections.add(i + 1, newSection);
return;
}
}
}
}
}
처음이나 끝이 아닌, 중간에 추가하는 메소드이다. 메소드 하나에 로직이 모두 뭉쳐져있고 분리되어 있지 않다. 처음엔 이것을 대체 어떻게 개선해야할지 막막했다. 그래서 수강생 분에게서 피드백을 받았는데, 그 내용은 요약하면 다음과 같다.
- 한 번에 추상화를 하지 않고 아래의 순서대로 접근하기
- 각 판단 로직을 일단 메소드로 분리해보기
- 최대 1 depth가 넘지 않게끔 메소드로 분리해보기
- 필요한 경우에 해당 기능의 책임을 갖는 새로운 클래스로 분리하기
- 객체지향 생활체조 원칙
- 한 메소드에 오직 한 단계의 들여쓰기만 하기
- else 예약어 쓰지 않기
- 모든 원시값, 문자열을 포장하기
- 한 줄에 점을 하나만 찍기
- 줄여쓰지 않기
- 모든 엔티티를 작게 유지하기
- 3개 이상의 인스턴스 변수를 갖는 클래스를 쓰지 않기
- 일급 콜렉션 쓰기
- 게터/세터/프로퍼티를 쓰지 않기
위 내용을 숙지하여 리팩토링을 시작해보겠다.
1) 단순하게 메소드 분리부터
우선 근본적으로 따져보거나, 멋지게 추상화하기 보다는 단순히 메소드 분리만 해보겠다.
우선 새 구간이 노선에 상행역이나 하행역으로 등록되어있는지 확인하는 부분을 메소드로 분리하고, 아래의 if-else문 내부의 for문으로 이뤄진 부분도 메소드로 분리하겠다.
간단히는 아래와 같은 형태가 될 것이다.
- 상행역 기준으로 삽입할지, 하행역 기준으로 삽입할지 찾기
- boolean upstationExists = isUpstationExistsInLine(newSection);
- boolean downstationExists = isDownstationExistsInLine(newSection);
- 위 변수들로 유효성 검증
- 구간 연결, 추가, 거리 쪼개기 등 주요 복잡한 계산 욱여넣은 함수 호출
- addAndConnectSection(upstationExists, newSection);
코드는 아래와 같다.
public void addSection(Section newSection) {
boolean upstationExists = isUpstationExistsInLine(newSection);
boolean downstationExists = isDownstationExistsInLine(newSection);
if (upstationExists && downstationExists) {
throw new InvalidInputException("새로운 구간의 상행역과 하행역 둘 다 이미 노선에 등록되어 있습니다.");
}
if (!upstationExists && !downstationExists) {
throw new InvalidInputException("새로운 구간은 기존 노선의 역과 최소 하나 이상 연결되어야 합니다.");
}
addAndConnectSection(upstationExists, downstationExists, newSection);
}
private boolean isUpstationExistsInLine(Section newSection) {
return sections.stream().anyMatch(section ->
section.isUpstation(newSection.getUpstation()) ||
section.isDownstation(newSection.getUpstation()));
}
private boolean isDownstationExistsInLine(Section newSection) {
return sections.stream().anyMatch(section ->
section.isUpstation(newSection.getDownstation()) ||
section.isDownstation(newSection.getDownstation()));
}
private void addAndConnectSection(boolean upstationExists, Section newSection) {
if (upstationExists) {
sections.stream()
.filter(section -> section.isUpstation(newSection.getUpstation()))
.findFirst()
.map(section -> {
section.setUpstation(newSection.getDownstation());
sections.add(sections.indexOf(section), newSection);
int newDistance = section.getDistance() - newSection.getDistance();
if (newDistance <= 0) {
throw new InvalidInputException("유효하지 않은 거리입니다.");
}
section.setDistance(newDistance);
});
} else {
sections.stream()
.filter(section -> section.isDownstation(newSection.getDownstation()))
.findFirst()
.map(section -> {
section.setDownstation(newSection.getUpstation());
sections.add(sections.indexOf(section) + 1, newSection);
int newDistance = section.getDistance() - newSection.getDistance();
if (newDistance <= 0) {
throw new InvalidInputException("유효하지 않은 거리입니다.");
}
section.setDistance(newDistance);
});
}
}
단순히 한 덩어리를 두 조각으로 나눴다고 보면 된다.
2) 메소드마다 하나의 책임
이번에는 단순 메소드 분리가 아닌, 책임의 관점에서 생각해보겠다. 구간을 연결하는 메소드인 addAndConnectSection()는 다음과 같은 로직을 수행한다:
- 구간 삽입할 위치 찾기
- 새 구간 삽입
- 새 구간과 기존 구간 연결
- 연결된 구간의 거리 재설정
한 메소드에 책임이 과중하게 몰려있다. addAndConnectSection()이라는 이름에 맞게 새 구간 삽입과 구간 연결만 남겨놓고 나머지 로직은 분리할 것이다. 구간 삽입할 위치 찾기는 이 로직에 맞는 메소드 findConnectableSection()을 새로 정의할 것이고, 연결된 구간의 거리 재설정도 updateDistance()라는 이름의 메소드로 정의할 것이다. 이렇게 하면 메소드마다 하나의 책임을 맡게 된다.
public void addSection(Section newSection) {
boolean upstationExists = isUpstationExistsInLine(newSection);
boolean downstationExists = isDownstationExistsInLine(newSection);
if (upstationExists && downstationExists) {
throw new InvalidInputException("새로운 구간의 상행역과 하행역 둘 다 이미 노선에 등록되어 있습니다.");
}
if (!upstationExists && !downstationExists) {
throw new InvalidInputException("새로운 구간은 기존 노선의 역과 최소 하나 이상 연결되어야 합니다.");
}
Section connectableSection = findConnectableSection(upstationExists, newSection);
addAndConnectSection(upstationExists, connectableSection, newSection);
updateDistanceWhenAdd(newSection, connectableSection);
}
private Section findConnectableSection(boolean upstationExists, Section newSection) {
if (upstationExists) {
return sections.stream()
.filter(section -> section.isUpstation(newSection.getUpstation()))
.findFirst()
.orElseThrow();
}
return sections.stream()
.filter(section -> section.isDownstation(newSection.getDownstation()))
.findFirst()
.orElseThrow();
}
구간 삽입할 위치 찾는 메소드
private void addAndConnectSection(boolean upstationExists, Section newSection, Section connectableSection) {
if (upstationExists) {
connectableSection.setUpstation(newSection.getDownstation());
sections.add(sections.indexOf(connectableSection), newSection);
} else {
connectableSection.setDownstation(newSection.getUpstation());
sections.add(sections.indexOf(connectableSection) + 1, newSection);
}
}
새 구간 삽입 & 구간 연결
private void updateDistanceWhenAdd(Section newSection, Section connectedSection) {
int newDistance = connectedSection.getDistance() - newSection.getDistance();
if (newDistance <= 0) {
throw new InvalidInputException("유효하지 않은 거리입니다.");
}
connectedSection.setDistance(newDistance);
}
연결된 구간의 거리 재설정
3) 추상화
이 상태에서 받은 피드백은 다음과 같다.
<addSection()의 추상화 레벨이 맞지 않다>
public void addSection(Section newSection) {
boolean upstationExists = isUpstationExistsInLine(newSection);
boolean downstationExists = isDownstationExistsInLine(newSection);
if (upstationExists && downstationExists) {
throw new InvalidInputException("새로운 구간의 상행역과 하행역 둘 다 이미 노선에 등록되어 있습니다.");
}
if (!upstationExists && !downstationExists) {
throw new InvalidInputException("새로운 구간은 기존 노선의 역과 최소 하나 이상 연결되어야 합니다.");
}
Section connectableSection = findConnectableSection(upstationExists, newSection);
addAndConnectSection(upstationExists, connectableSection, newSection);
updateDistanceWhenAdd(newSection, connectableSection);
}
구간을 추가하는 입장에서, 예외를 처리하는 구체적인 코드는 추상화 레벨이 맞지 않다. upstationExists, downstationExists 예외 처리 세부 구현을 한 단계 감출 필요가 있다.
public void addSection(Section newSection) {
boolean upstationExists = validateWhenAdd(newSection);
Section connectableSection = findConnectableSection(upstationExists, newSection);
addAndConnectSection(upstationExists, newSection, connectableSection);
updateDistanceAndDurationWhenAdd(newSection, nextSection);
}
private boolean validateWhenAdd(Section newSection) {
boolean upstationExists = isUpstationExistsInLine(newSection);
boolean downstationExists = isDownstationExistsInLine(newSection);
if (upstationExists && downstationExists) {
throw new InvalidInputException("새로운 구간의 상행역과 하행역 둘 다 이미 노선에 등록되어 있습니다.");
}
if (!upstationExists && !downstationExists) {
throw new InvalidInputException("새로운 구간은 기존 노선의 역과 최소 하나 이상 연결되어야 합니다.");
}
return upstationExists;
}
위와 같이 별도의 메소드로 분리해 추상화 레벨을 맞췄다.
<메소드 이름도 추상적으로>
addAndConnectSection()의 메소드 이름에 행위가 and로 연결되어 두 개 이상이라면 적신호이다. 내부 구현을 직접적으로 나타내는 이름보다는 더 추상적으로 어떤 행위를 하는지 나타내는 이름이 좋다. updateDistanceWhenAdd()도 마찬가지다.
내 생각에 addAndConnectSection()은 connectSection()이란 이름으로 바꿔도 괜찮을 것 같다. 나는 새 구간을 추가하고, 새 구간과 기존 구간을 연결하는 것을 하나의 연결하는 행위라는 관점으로 봤다. '새 구간을 기존 구간과 연결한다' 라는 하나의 행위다.
updateDistanceWhenAdd()는 이름 그대로, 구간을 추가하는 경우에 distance를 변경하는 행위이지만 추상적으로 생각해보면 새 구간이 가운데를 비집고 들어와 기존 거리가 쪼개지는(짧아지는) 행위로 볼 수 있다. 그래서 Section 클래스로 옮기고 shorten()이라는 이름으로 바꿨다.
<if-else문 지양하도록 개선>
이 부분은 직접 피드백받진 않았지만...
private Section findConnectableSection(boolean upstationExists, Section newSection) {
if (upstationExists) {
return sections.stream()
.filter(section -> section.isUpstation(newSection.getUpstation()))
.findFirst()
.orElseThrow();
}
return sections.stream()
.filter(section -> section.isDownstation(newSection.getDownstation()))
.findFirst()
.orElseThrow();
}
조건에 따라 필터링에 들어가는 조건과 인자가 달라지지만 유사한 스트림을 쓰고 있다. 이 부분을 공통 메소드로 합치는 방법을 고민해봤다.
private Section findConnectableSection(boolean upstationExists, Section newSection) {
Map<Boolean, Function<Section, Boolean>> conditionMap = new HashMap<>();
conditionMap.put(true, section -> section.isUpstation(newSection.getUpstation()));
conditionMap.put(false, section -> section.isDownstation(newSection.getDownstation()));
Predicate<Section> condition = section -> conditionMap.get(upstationExists).apply(section);
return sections.stream()
.filter(condition)
.findFirst()
.orElseThrow();
}
이렇게 하면 인자를 받아 스트림 하나만 쓸 수 있긴 한데 괜히 더 복잡하고 이해하기 어려워보이기도 한다. 이 부분은 무엇이 더 나을지 모르겠다.
결론적으로 개선 전과 개선 후 구조를 살펴보면:
addSection()에 모든 행위의 세부 구현이 몰려있는 구조에서, 추상화된 행위를 호출하는 구조로 바꿨다.
3. 다른 방식
@Embeddable
public class Sections implements Iterable<Section> {
...
private void connectMiddle(final Section section) {
if (getDistance() <= section.getDistance()) {
throw new SectionConnectException("가운데에 생성할 구간의 길이가 해당 노선의 총 길이보다 길거나 같을 수 없습니다.");
}
final Section upSection = findSectionByUpStation(section.getUpStation());
upSection.shorten(section);
sections.add(sections.indexOf(upSection), section);
}
private Section findSectionByUpStation(final Station upStation) {
return sections.stream()
.filter(s -> s.getUpStation().equals(upStation))
.findFirst()
.orElseThrow(() -> new SectionsStateException("해당 상행역을 가진 구간을 찾을 수 없습니다."));
}
}
@Entity
public class Section implements Comparable<Section> {
...
public void shorten(final Section section) {
this.upStation = section.downStation;
this.distance -= section.distance;
}
public void extend(final Section section) {
this.upStation = section.upStation;
this.distance += section.distance;
}
}
가장 인상깊었던 수강생 분의 코드이다. shorten과 extend라는 직관적인 메소드명으로, 중간에 구간을 추가할 때 어떤 행위들이 필요한지 쉽게 이해할 수 있게 구현되었다. 나는 구간 연결과 거리 재설정을 분리했지만 이 코드는 shorten, extend라는 행위에 구간 연결과 거리 재설정이 포함되어있는 관점이라는 것을 알 수 있다. 이 관점이 새로우면서도 논리적으로 맞다는 생각이 들었다. 또한 새 구간 추가와 구간 연결도 따로 분리된 것으로 보는 관점이다.
이 코드를 보고 구간 중간 추가에서 거리를 쪼개는 것을 shorten이라는 행위로 추상화할 수 있다는 것을 알게 되었다.
그림으로 구조화해보면 위와 같다. 상행역(UpStation)으로 구간을 찾고, 그 구간의 거리를 축소(shorten)한 다음 구간을 추가한다는 행위가 매끄럽게 연결된다는 것을 알 수 있다.
4. 결론
처음엔 구간 추가가 동작하는 것만으로도 만족해서 리팩토링할 생각이 없었는데, 다른 수강생 분들의 코드를 둘러보다보니 리팩토링할 욕심이 생겼다. 그런데 완성된 코드만 생각하다 보니 어떻게 리팩토링해야할 지 막막해서 수강생 분과 리뷰어 분에게 상세한 피드백을 받아 개선할 수 있었다.
'스프링 > 프로젝트' 카테고리의 다른 글
[스프링] 배치 멱등성 보장하는 과정 (0) | 2024.07.09 |
---|---|
[스프링] 책임 연쇄 패턴으로 요금 정책 구현하기 (0) | 2024.03.09 |
[스프링부트] 트랜잭션에서 외부 API 호출하는 문제에 대해 (0) | 2023.11.14 |
Spring WebFlux + OAuth2 로그인 구현 과정 (0) | 2023.11.11 |
[스프링부트] 교착 상태 테스트 & 해결 과정 (0) | 2023.10.29 |