[우테코 5기] 지하철 미션 회고

가비와 진행한 레벨 2 세 번째 페어 프로그래밍 미션인 지하철 미션이다...!

우테코 하면서 진행했던 미션 중에서 가장 어려운 미션이 아니었나 싶다.

객체지향적인 코드를 작성하는 것과 단순 구현 그 사이에서 엄청 헤맸던 것 같다...

앞으로 개발할 때도 레벨 1에서 배웠던 것들을 정말 잊지 않아야겠다 🥲

 

---

 

✔️ 작성한 코드

GitHub - Cl8D/jwp-subway-path: 레벨 2 지하철 미션 레파지토리

 

✔️ 1차 PR

[1단계 - 지하철 정보 관리 기능] 져니(이지원) 미션 제출합니다. by Cl8D · Pull Request #45 · woowacourse/

 

✔️ 2차 PR

[2단계 - 경로 조회 기능] 져니(이지원) 미션 제출합니다. by Cl8D · Pull Request #111 · woowacourse/jwp-subwa

 

---

✔️ 기능 요구사항

- 지하철 노선에 역을 등록 / 제거하는 API 구현

- 하나의 역은 여러 노선에 등록될 수 있으며, 노선은 갈래길을 가질 수 없다.

- 최초 등록 시 두 역을 동시에 등록해야 한다.

- 이미 등록된 역 사이에 노선을 등록할 경우 거리 정보를 고려해야 한다.

- 노선 제거 시 기존 역 간 거리 정보를 업데이트 해줘야 한다.

- 노선에 2개만 역이 남았을 경우 모두 제거해야 한다.

- 노선 조회 시 등록된 역도 함께 조회되도록 API 구현

- 출발역에서 도착역까지 갈 수 있는 최단 경로 출력 및 요금 정보 조회 API 추가

- 노선에 대한 추가 요금 정책 및 연령별 요금 할인 정책 추가

 

이번에도 고민했던 부분들에 대해서 적어보고자 한다!

 

---

 

💬 도메인 설계하기

페어랑 진행하면서 DB 위주가 아닌, 도메인 위주의 코드를 작성하고 싶어서 처음부터 도메인을 위주로 작성했었다.

노선과 지하철 역, 그리고 노선에 등록된 역 정보를 나타내기 위해 구간 정보를 나타내는 3개의 도메인을 메인으로 잡았다.

아마 대부분의 크루들이 이렇게 설계한 것 같았다.

 

🌱 Station

사실 말 그대로 지하철 역에 대한 정보를 가지고 있는 도메인이다.

지하철 이름 말고 특별한 정보가 없기 때문에 딱히 설명할 것이 없다 ㅎㅎ

 

🌱 Section

노선에 대한 각 구간 정보를 저장하는 도메인이다.

출발역과 도착역, 그리고 두 역 사이의 거리 정보를 관리하고 있다. 

처음에는 역의 하행, 상행 정보를 알기 위해서 이에 대한 boolean 필드를 추가할까 고민했었는데, map을 사용해서 이는 해결할 수 있는 문제여서 추후 수정하게 되었다. (허브 갓 👍)

 

🌱 Line

노선 도메인의 경우 이름과 색상, 추가 요금, 그리고 노선에 대한 역 정보인 SubwayLine을 가지고 있도록 하였다.

Section에서 Line을 참조하는 식으로 설계를 하신 크루들도 몇몇 봤었는데, 테이블상에서는 Section 테이블이 Line 테이블을 참조하는 것이 자연스럽지만 실세계에서 '노선이 역을 가진다'라는 조건에 부합하지 않는다고 생각했다.

그래서 노선 도메인이 SubwayLine이라는 도메인을 가지도록 만들었고, 도메인 객체를 설계할 때 어떤 식으로 모델링하는 게 중요한지 알 수 있었던 시간인 것 같다. (내가 지금까지 정말 DB 주도 개발을 했었구나 다시금 생각할 수 있던 시간이었다...)

 

🌱 SubwayLine

SubwayLine이라는 도메인은 하나의 노선에 대한 구간 정보 리스트와 순서대로 정렬되어 있는 역 리스트를 가지고 있으며, Line

사실 DB와 가장 간극이 큰 도메인이 아닐까 싶다. (이 과정에서 객체지향적 설계를 정말 많이 고민했다.)

도메인이 할 수 있는 일은 사실 서비스에서도 충분히 DB 조회를 통해서 해결할 수 있는 일들이기 때문이다.

 

객체에게 책임을 주기 위해 DB에서 조회해온 도메인과 실제 연산을 통해 변경된 부분에 대해서 변경 감지 (프록시 객체를 이용하거나, 혹은 차집합을 이용하거나...)를 활용하여 업데이트를 한 크루들도 보였었는데, 오히려 프로그램의 복잡도가 너무 증가하는 것 같아서 실제 sections 정보가 업데이트 되더라도 도메인의 필드는 변경되지 않도록 했다.

대신, 새로운 구간 정보가 들어올 수 있는지에 대한 '검증 작업'을 위주로 도메인에게 책임을 전가하도록 설계하였다.

 

코드 부분을 대충 가져왔다. 이런 식으로 Line 도메인을 생성한 다음에, subwayLine을 뽑아냈다.

💡 여기서 Line의 subwayLine을 뒤늦게 업데이트 하는 이유는 단순히 라인에 대한 조회 후, 라인과 연관된 섹션 정보를 따로 조회하기 때문이다. 
findById를 통해서 요청받은 노선의 아이디가 유효한지 검증하고, 유효하다면 섹션 정보를 조회하는 식으로 구현하려고 하다 보니 저런 구조가 됐다. 또한, findById에서 굳이 여러 정보를 가져오려고 조합하는 것보다 단순히 line에 대한 정보만 조회되도록 만들고 싶었기 때문이다. findById는 비교적 가볍게 쓰이는 메서드라고 생각했기 때문이다... ㅎㅎ

 

그리고 validateSection을 통해서 요청받은 도메인에 대한 검증 작업을 진행한다.

그리고 내부적으로 요청받은 정보가 비어있는지, 혹은 이미 등록된 정보인지, 실제로 저장된 역 정보인지 확인하게 된다.

이런 식으로 도메인이 내부적으로 검증할 수 있는 내용은 검증하도록 만들면서 책임을 넘기고, 실제 DB에 삽입, 삭제, 수정해야 하는 부분은 서비스에게 책임을 넘겨 구현하였다.

 

또한, SubwayLine이 정렬된 역 정보에 대해서 필드로 관리하고 있는 게 약간 찝찝하기도 했었다.

sections 정보를 통해서 역 정보는 충분히 유추 가능한 정보이기 때문에, 사실 필드로 관리하지 않고 필요할 때마다 정렬해서 사용하는 방법도 있기 때문이다.

하지만... 굳이 처음 한 번 정렬하면 되는 정보를 조회할 때마다 정렬하고 싶지 않아서 그냥 위와 같이 필드로 두는 방향을 택했다.

리뷰어님도 다른 말씀 안 하셨기 때문에 나름 합리적인 구조가 아니었나 싶다.

private List<Station> sort(final List<Section> sections) {
    final Map<Station, Station> stationRelationShip = getStationRelationShip(sections);
    final Optional<Station> startStation = getStartStation(stationRelationShip);
    return startStation.map(station -> getSortedStations(stationRelationShip, station))
        .orElse(Collections.emptyList());
}

private Map<Station, Station> getStationRelationShip(final List<Section> sections) {
    return sections.stream()
        .collect(Collectors.toMap(Section::source, Section::target));
}

private Optional<Station> getStartStation(final Map<Station, Station> stationRelationShip) {
    return stationRelationShip.keySet().stream()
        .filter(station -> !stationRelationShip.containsValue(station))
        .findFirst();
}

private List<Station> getSortedStations(final Map<Station, Station> stationRelationShip,
                                        final Station startStation) {
    final List<Station> sortedStations = new ArrayList<>();
    Optional<Station> nextStation = Optional.ofNullable(startStation);
    while (nextStation.isPresent()) {
        sortedStations.add(nextStation.get());
        nextStation = Optional.ofNullable(stationRelationShip.get(nextStation.get()));
    }
    return sortedStations;
}

개인적으로 정렬 로직에 대해서 스트림을 적절히 활용한 것 같아 뿌듯하다 😊

 

---

 

💬 테이블 설계하기

테이블 구조는 아래와 같이 설계하였다.

CREATE TABLE IF NOT EXISTS station
(
    id bigint auto_increment NOT NULL,
    name VARCHAR(255) NOT NULL UNIQUE,
    PRIMARY KEY(id)
);

CREATE TABLE IF NOT EXISTS line
(
    id bigint auto_increment NOT NULL,
    name VARCHAR(255) NOT NULL UNIQUE,
    color VARCHAR(20) NOT NULL,
    extra_fare int NOT NULL,
    PRIMARY KEY(id)
);

CREATE TABLE IF NOT EXISTS section
(
    id bigint auto_increment NOT NULL,
    line_id bigint NOT NULL,
    source_station_id bigint NOT NULL,
    target_station_id bigint NOT NULL,
    distance int NOT NULL,
    PRIMARY KEY(id),
    FOREIGN KEY(line_id) REFERENCES line(id) ON DELETE CASCADE,
    FOREIGN KEY(source_station_id) REFERENCES station(id),
    FOREIGN KEY(target_station_id) REFERENCES station(id)
);

사실 외래키에 대해서도 정말 의견이 분분하게 나뉘는데, 개인적으로 이렇게 작은 규모라면 외래키를 걸어두는 게 더 좋을 것 같다.

물론 그만큼 테스트가 빡세지만... 테스트는 픽스처를 활용하여 어느 정도 해소할 수 있을 것 같다.

 

그리고, section 테이블의 필드에는 on delete cascade 옵션을 주었는데, 이는 부모 테이블의 row가 제거되었을 때 함께 제거되어 section의 고아 필드(?)가 남지 않도록 만들었다.

위에서도 말했던 것처럼 section의 경우 line의 정보와 연관되어 있는 정보이기 때문에, 노선이 제거된 이후에도 해당 노선의 구간 정보가 존재할 필요가 없다고 생각했기 때문이다. 

 

그리고, 원래 구현할 때는 station 쪽에도 on delete cascade를 걸어두었었는데, 생각해보니까 역 정보가 제거된다고 해서 구간 정보를 바로 제거해버리면 기존 구간에 대한 거리 업데이트가 안 될 것 같아서 회고를 작성하며 간단하게 리팩터링을 해보았다.

public int deleteById(final Long id) {
    try {
        final String sql = "DELETE FROM station WHERE id = ?";
        return jdbcTemplate.update(sql, id);
    } catch (final DataIntegrityViolationException e) {
        throw new BadRequestException(ErrorCode.STATION_DELETE_IF_EXISTS_SECTION);
    }
}

그냥 이렇게 FK에 대한 오류가 발생하면 이미 구간에 대한 정보가 있기 때문에 역을 제거할 수 없다는 예외를 반환하도록 만들었다.

실행하면 요런 식으로 제거할 수 없도록 만들었다! 😊

 

---

 

💬 인터페이스로 의존성 역전시키기 (DIP)

이번 미션에서 가장 신경쓴 부분 중에 하나이다.

저번 장바구니 미션 회고 글을 작성하면서 마지막에 다음과 같은 커멘트를 남겼었다.

지난 미션의 의존성은 다음과 같은 구조였다.

서비스에서 바로 Dao를 참조하다 보니, Dao가 반환하는 값인 Entity와 의존 관계가 생기고, 이 정보를 바탕으로 도메인 객체를 생성하다 보니 서비스가 도메인까지 알게 되는 구조가 된 것이다.

 

그래서 이번 미션에서는 다음과 같이 구조를 변경하였다.

Repository 계층을 추가해서 서비스 레이어는 도메인만 바라보고, 도메인과 엔티티를 변환하는 작업을 Repository에게 위임한 것이다.

이 덕분에 기존의 순환 관계를 없앨 수 있었다.

 

하지만, DAO와 Repository는 Persistence layer에 존재하고 있다.

application layer가 Repository를 import 하면서 여전히 persistence layer를 의존하며 domain 패키지에 존재하는 도메인 객체와 모두 상호 참조하게된다는 단점이 존재했다.

 

⭐️ 이를 위해 Repository의 인터페이스는 도메인 레이어에 두고, 이에 대한 구현체를 persistence layer에 두어 의존 관계를 역전시키는 전략을 채택하였다! 덕분에 Application Layer에서는 도메인 계층의 패키지만 의존하게 되어서 더 이상 순환 참조가 발생하지 않았다.

실제로 서비스 레이어의 import를 보면 application, domain으로만 참조 관계가 형성된 것을 확인할 수 있다.

 

실제로 그림으로 설명하자면 이런 느낌이라고 볼 수 있다.

 

---

 

💬 외부 라이브러리에 덜 의존적인 형태로 설계하기

최단 경로를 구하기 위하여 jgrapht 라이브러리를 사용했어야 했는데, 부끄럽지만 내 처음 제출 코드는 다음과 같았다.

// RouteService.java
private WeightedMultigraph<Station, DefaultWeightedEdge> getPossibleRouteGraph(
    final Map<Long, List<LineWithSectionRes>> sectionsByLineId) {
    final WeightedMultigraph<Station, DefaultWeightedEdge> routeGraph = new WeightedMultigraph<>(
        DefaultWeightedEdge.class);
    sectionsByLineId.values()
        .forEach(sectionRes -> addRouteBySectionRes(routeGraph, sectionRes));
    return routeGraph;
}

전체 코드를 가져오기 너무 길어 일부만 가져왔지만, 저런 식으로 외부 라이브러리가 제공하는 기능을 그대로 사용하고 있었다.

당연하게 리뷰어님에게도 같은 지적을 받아서 부끄러웠다 🥲

어떤 방법을 사용하면 좋을지 고민하다, 인터페이스를 활용하기로 결정하였다.

public interface GraphProvider {

    List<Station> getShortestPath(final List<SubwayLine> sections, final Station sourceStation,
                                  final Station targetStation);
}

도메인 계층에 다음과 같은 인터페이스를 만들고 (이름을 엄청 고민했었는데 나름... 괜찮나?), 인자로 출발역에서 도착역까지 갈 수 있는 모든 구간 정보와 출발역, 도착역 정보를 넘겨주었다.

 

그리고, 외부 서비스임을 나타내는 'infra' 패키지에 JgraphService를 추가하였다.

@Component
public class JgraphtService implements GraphProvider {

    @Override
    public List<Station> getShortestPath(final List<SubwayLine> sections,
                                         final Station sourceStation,
                                         final Station targetStation) {
        final WeightedMultigraph<Station, DefaultWeightedEdge> routeGraph = createRouteGraph(sections);
        validateRequestRoute(sourceStation, targetStation, routeGraph);
        return getShortestPath(sourceStation, targetStation, routeGraph);
    }
    ...
}

사실 내부는 그냥 최단 경로 구하는 코드여서 의미는 없을 것 같아 첨부하지 않았다.

인자로 넘겨주는 값이 많기는 하지만, 내부적으로 상태를 가지지 않게 하려면 최단 경로를 구하는 메서드에서 경로도 구하고, 입력값이 올바른 값인지에 대한 검증까지 다 해야 된다. 

 

이렇게 설계한 덕분에 application layer에서는 도메인 계층의 GraphProvider 인터페이스만 바라보고 있어 추후 다른 라이브러리로 교체되더라도 application layer에 대한 변경사항은 없어 영향 범위를 최소화할 수 있게 되었다. 

🥹 개인적으로 infra 패키지는 왜 항상 인터페이스를 사용하는 것일까 궁금했는데... 이렇게 직접 구성하면서 완전히 이해할 수 있었다.

 

---

 

💬 요금 정책 효율적으로 적용하기

부끄럽게도 요금 정책 역시 처음에 짠 코드가 굉장히 별로다.

public Fare calculateFare(final TotalDistance distance) {
    if (distance.lessThan(EXTRA_FARE_SECTION_TEN)) {
        return BASIC_FARE;
    }
    if (distance.lessAndEqualsThan(EXTRA_FARE_SECTION_FIFTY)) {
        final TotalDistance extraDistance = distance.subtract(EXTRA_FARE_SECTION_TEN);
        return BASIC_FARE.add(calculateByDistance(extraDistance, FARE_SECTION_UNIT_FIVE));
    }
    final TotalDistance fareSectionDistance = EXTRA_FARE_SECTION_FIFTY.subtract(EXTRA_FARE_SECTION_TEN);
    final TotalDistance extraDistance = distance.subtract(EXTRA_FARE_SECTION_FIFTY);
    final Fare extraFare = calculateByDistance(fareSectionDistance, FARE_SECTION_UNIT_FIVE)
        .add(calculateByDistance(extraDistance, FARE_SECTION_UNIT_EIGHT));
    return BASIC_FARE.add(extraFare);
}

10km, 10~50km, 50km 이상에 대해 각각 분기문을 통해 처리하고 있다 보니 확장에 매우 취약한 구조가 된 것이다.

OCP를 위반하고 있어요. 어떻게 개선해볼수 있을까요? (Hint. 해당 구간에 판단하는지 여부와 요금을 계산하는 메서드가 공통적이네요)

요기서 리뷰어님이 굉장한 힌트를 주셨다. 구간을 판단하는 메서드와 요금을 계산하는 메서드를 나누라는 것이다.

 

그래서 아예 정책별로 클래스를 나누기로 결정하였다. 

우선, 각 정책에 대해 공통적인 행위를 정의하는 인터페이스를 생성해두었다.

public interface FarePolicy {

    boolean isAvailable(final Distance distance);

    Fare calculateFare(final Distance distance);
}

isAvailable가 구간에 대해 정책 적용이 가능한지 판단하는 메서드이고, calculateFare가 실제로 요금을 계산하는 메서드이다.

 

 

그리고, 해당 인터페이스를 구현한 3개의 구체 정책 클래스를 생성하였다.

10km 이하에 대한 기본 정책은 BasicFarePolicy, 10~50km는 UnitFiveFarePolicy, 50km는 UnitEightFarePolicy로 설정했다.

이름이 좀 아쉽긴 한데... 5km당, 8km당 100원씩 추가된다는 게 더 중요한 것 같아서 저렇게 만들어두었다 ㅎㅎ

공통적으로 사용하는 필드 값은 enum을 통해서 정의해두었다. (한 번에 관리하기 위해서)

 

그리고, 중요한 점은 각 정책들을 'FarePolicyComposite'라는 클래스를 통해 한 번에 관리하고 있는 것이다.

public Fare getTotalFare(final Distance shortestDistance) {
    return farePolicies.stream()
        .filter(policy -> policy.isAvailable(shortestDistance))
        .map(policy -> policy.calculateFare(shortestDistance))
        .findFirst()
        .orElseThrow();
}

해당 컴포지트 클래스를 통해, 각 정책들을 하나씩 순회하면서 적용 가능한 정책인지 판단한 뒤, 적용 가능하다면 진행한다.

현재 설계상 구간 정보는 각 정책 중 하나의 범위에 무조건 들어가기 때문에 orElseThrow()로 던져질 위험이 없을 것이라 판단하였고, 이렇게 되면 60km 이상의 구간에서는 다른 정책을 추가해야 할 때 farePolices 리스트에 정책을 추가해주기만 하면 되기 때문에 확장에 열린 구조를 유지할 수 있었다.

 

💡 단, 이 구조를 채택하기 위해서는 isAvailable의 조건 정보가 촘촘해야 된다는 점이다. 현재 구조는 촘촘하게 설계되어 괜찮지만!

 

그렇다면, 정책 리스트는 어디에서 추가해줄까?

각 정책들은 스프링 빈으로 관리되고 있기 때문에 @Configuration을 활용하면 쉽게 주입이 가능해진다.

@Configuration
public class FarePolicyConfig {

    @Bean
    public FarePolicyComposite farePolicies(
        final FarePolicy basicFarePolicy,
        final FarePolicy unitFiveFarePolicy,
        final FarePolicy unitEightFarePolicy
    ) {
        return new FarePolicyComposite(
            List.of(basicFarePolicy, unitFiveFarePolicy, unitEightFarePolicy)
        );
    }
}

이때 파라미터로 주입되는 클래스의 이름은 꼭 빈에 등록된 클래스의 이름과 맞춰줘야 한다.

💡 스프링에서  기본적으로 파라미터의 타입을 기준으로 빈을 조회한다.
만약, 같은 타입의 빈이 여러 개 조회되면 추가적으로 필드나 파라미터 이름으로 빈을 조회한다.

위의 원리를 바탕으로 'FarePolicyComposite' 타입의 'farePolicies'라는 이름을 가진 빈을 등록했기 때문에, application layer에서도 다음과 같이 주입받아서 사용하면 된다.

@Service
public class RouteService {
    ...
    private final FarePolicyComposite farePolicies;
}

 

이를 바탕으로 추가 요금 정책도 동일하게 구현할 수 있었다 ㅎㅎ

나 같은 경우는 나이 정보를 따로 받지 않고, 그냥 할인된 가격도 함께 반환하도록 만들다 보니 가능했던 것 같다.

public DiscountFare getDiscountFares(final Fare totalFare) {
    final List<Fare> fares = discountFarePolicies.stream()
        .map(policy -> policy.calculateFare(totalFare))
        .collect(Collectors.toUnmodifiableList());
    return new DiscountFare(fares.get(0), fares.get(1));
}

단, 이때는 빈으로 등록된 순서에 따라서 영향을 받다보니까 조금 애매하다.

만약 나이를 입력받는다면 이렇게 map -> collect 대신에 요금 정책을 적용했던 것처럼 filter를 통해 받을 수 있을 것 같다.

 

---

 

쓸 내용이 별로 없을 것이라고 생각했는데 쓰다 보니까 오늘 하루가 다 가버렸다 😅

이번 미션은 정신없던 것만큼 나름 많은 걸 얻어갔던 것 같다.

내일부터 새로운 미션 시작인데... 파이팅 해야겠다! 💪