[Review] ATDD 4주차 2차 PR

최홍준 리뷰어 님께 감사의 말을 전하고 싶다!!

 

1. 리뷰 정리

1-1) 최단 경로 구하는 로직

원래 Service에서 최단 경로를 구하는 로직은 다음과 같았습니다.

public PathResponse findShortestPath(Long source, Long target, PathCondition pathCondition) {
    Station upStation = stationService.findById(source);
    Station downStation = stationService.findById(target);
    List<Line> lines = lineService.findLines();
    SubwayMap subwayMap = new SubwayMap(lines);

    Path path = subwayMap.findPath(upStation, downStation, pathCondition.getEdgeInitiator());
    int shortestDistance = subwayMap.findShortestPathDistance(upStation, downStation);
    int fare = calculateFare(shortestDistance);

    return PathResponse.of(path, fare);
}

위 로직에서 Service에서 shortestDistance를 구한 후, 이에 대한 fare를 구합니다.

이를 PathResponse.of에 전달해서 반환하고 있습니다.

 

하지만 'fare를 구해야 할 책임은 누구한태 있을까?' 라고 생각해봤을때, 해당 행동을 책임질 주체는 Path가 아닐까? 라는 생각을 하게 되었습니다.

 

찾아낸 최단 경로인 path 객체 스스로가 자신의 가격을 알고 있어야 한다라고 생각하였습니다.

또한 리뷰어도 다음과 같이 리뷰해주셨습니다.

Path에 최단거리 필드를 추가하고, 이를 이용하도록 하자!

public class Path {
    private Sections sections;
    private int shortestDistance;

    public Path(Sections sections) {
        this.sections = sections;
        this.shortestDistance = sections.totalDistance();
    }
    // 생략...

    public void setShortestDistance(int shortestDistance) {
        this.shortestDistance = shortestDistance;
    }

    public int extractFare() {
        FareStrategy fareStrategy = FareType.findStrategy(shortestDistance);
        return fareStrategy.calculate(shortestDistance);
    }
}

Path 자체에 최단거리 필드가 있고, 최단 거리 기준으로 요금을 구하는 책임을 가지고 있다.

 

변경된 코드는 다음과 같다.

public PathResponse findShortestPath(Long source, Long target, PathCondition pathCondition) {
    Station upStation = stationService.findById(source);
    Station downStation = stationService.findById(target);
    List<Line> lines = lineService.findLines();
    SubwayMap subwayMap = new SubwayMap(lines);

    Path path = subwayMap.findPath(upStation, downStation, pathCondition.getEdgeInitiator());
    if (pathCondition == PathCondition.DURATION) {
        int shortestDistance = subwayMap.findShortestPathDistance(upStation, downStation);
        path.setShortestDistance(shortestDistance);
    }

    return PathResponse.of(path);
}

path에 최단 거리를 구해서 set해준다. 사실 setter를 사용하는 점이 매우 마음에들지는 않다.

이는 이후 다시 리팩터링 해야할것 같다.

 

1-2) var를 사용한 이유

var를 사용한 이유는 인수테스트는 가독성이 우선시 된다 생각하기 떄문입니다.
또한 개발자가 아닌, 기획자나 디자이너 분들도 볼 수 있기 때문에 인수테스트 자체의 가독성을 높혔습니다 ㅎㅎ
ExtractableResponse 이라는 타입을 개발자 외의 분들이 알필요는 없다 생각했기 때문입니다.

 

1-3) 문서화 작성시 실제 객체 사용하기

나 :

음 우선 추천해주신 방법에 대하여 문서의 좋은 관리방법이라 생각됩니다.

 

다만, 저희의 미션 요구사항에서 ATDD 사이클은 "인수테스트 작성 -> 문서작성 -> TDD로 개발" 사이클 순 입니다.

따라서 내부 구현이 되있지 않은 상황에서 문서화를 먼저 해야 하는데, 이때 mock없이 실제 객체의 사용을 힘들것 같다 생각됩니다.

물론 TDD로 기능 개발을 다 한 후, 문서화를 할수도 있지만, 먼저 문서화를 하자는 취지에 어긋난다 생각됩니다 ㅠ,ㅠ

 

답변:

 

1-4) 문서 분리하기

문서 부분 또한 위와 같이 함수로 뽑으면, 다른 API문서에서도 공통적으로 사용할 수 있다.

 

2. 다른 분들 리뷰

이번에는 다른 분들의 리뷰중 도음이 될만한 것들 또한 조금 살펴보았다!

 

2-1) 정수 표현 방식

정수를 표현할때 처의 4의 배수 단위로 _ 를 사용하면 가독성을 높힐 수 있다.

이에 대한 내용을 들어본 기억은 있는데, 자주 사용하지 않던 방식이라 어색했다.

다음 리뷰 수정때 나도 적용시켜야 겠다

 

2-2) Fare 를 계산할 책임은?

위에서도 작성했는데, 나는 Path에 Fare를 계산할 책임이 있다 생각하였다.

다행이 나와 비슷하게 생각하신 리뷰어가 있으셨다 ㅎㅎ

 

2-3) 정적 팩토리 메서드의 네이밍 규칙

대부분 from, of를 정적 팩토리 메서드의 이름으로 사용했었는데, 난 여지껏

변환의 방향성의 기준으로 네이밍을 하는 줄 알았다.

 

예를 들어 Dto -> domain 이면 Domain.from(dto)를 하면 dto 에서 domain으로 변환한다는 의미가 전달된다 생각했기 때문이다.

하지만 이러한 이유가 아닌, 인자의 개수로 네이밍이 정해진다는 점을 기억해야겠다!

 

2-4) 거리와 시간의 일급객체화

거리와 시간 또한 일급 객체로 포장하면 매우 재미있을것 같다.

Object Value로써 활용할 수 있을것 같다.