[우아한 테크코스 3기] LEVEL 2 회고 - 지하철 노선도 관리 1, 2단계 미션 1차 피드백을 받아보다 (98일차)

안녕하세요? 제이온입니다.

 

주말동안 중간곰과 온라인 페어 프로그래밍을 하고, 오늘도 마찬가지로 저녁까지 온라인으로 페어를 진행했습니다.

 

 

지하철 노선도 관리 미션 1, 2단계 1차 피드백

저보다 중간곰의 리뷰가 더 빨리 와서 같이 테스트 코드 위주로 리팩토링을 진행하였습니다. 아무래도 1차적으로 리뷰가 거친 코드다보니까 제 리뷰어인 김고래도 한 번에 merge를 해 주셨습니다. 그래도 몇가지 생각해 볼만한 피드백이 있어서 공유하려고 합니다.

 

 

 

 

첫 번째 피드백은 DataAccessUtils.singleResult() 메소드를 사용하여 리팩토링하는 것입니다. 위 코드의 원본부터 보겠습니다.

 

 

    public Optional<Line> findById(final Long id) {
        final String sql = "SELECT * FROM line WHERE id = ?";
        final List<Line> lines = jdbcTemplate.query(sql, lineRowMapper, id);
        if (lines.isEmpty()) {
            return Optional.empty();
        }
        return Optional.ofNullable(lines.get(0));
    }

 

 

우선, jdbcTemplate의 queryForObject()가 아닌 query()를 쓴 이유는 간단합니다. queryForObject()는 원하는 데이터를 찾지 못할 경우 익셉션을 발생시키는데, try~catch로 잡는 것은 성능이 좋지 않으므로 사용하지 않는 것입니다. 물론, 최선의 방법이 try ~ catch라면 괜찮지만, queryForObject() 대신 query()를 사용하면 익셉션 없이도 비교적 손쉽게 값을 얻어올 수 있습니다.

 

query()는 쿼리에 맞는 데이터들을 모아서 List로 반환하는데, 만약 비어있다면 Optional.empty()를, 그렇지 않다면 첫 번째 요소를 반환하는 식으로 소스 코드를 작성할 수 있습니다. 이때, 비어있는지 여부를 확인하는 if문까지도 줄이기 위하여 DataAccessUtils.singleResult()를 사용합니다.

 

 

    public Optional<Line> findById(final Long id) {
        final String sql = "SELECT * FROM line WHERE id = ?";
        final List<Line> lines = jdbcTemplate.query(sql, lineRowMapper, id);
        return Optional.ofNullable(DataAccessUtils.singleResult(lines));
    }

 

 

DataAccessUtils.singleResult()는 해당하는 요소가 없을 경우 null을, 그렇지 않다면 첫 번쨰 요소를 반환합니다. 이때, Optional.ofNullable()로 묶어서 반환하게 되면, 인자가 null일 때는 Optional.empty()로 변환하여 반환됩니다.

 

 

 

 

두 번째는 Dto에게 약간의 변환 작업을 주라는 것입니다. 해당 코드의 원본을 보겠습니다.

 

 

    public LineResponse createLine(final LineRequest lineRequest) {
        final String name = lineRequest.getName();
        final String color = lineRequest.getColor();

        final Line line = lineDao.save(new Line(name, color));
        return LineResponse.from(line);
    }

 

 

LineRequest 객체에서 name과 color를 받아와서 new 키워드를 통해 Line 객체를 생성하는 것을 알 수 있습니다. 이 변환 책임을 LineRequest에게 주면 코드를 간결하게 할 수 있습니다.

 

 

public class LineRequest {

    private String name;
    private String color;
    private Long upStationId;
    private Long downStationId;
    private int distance;
    private int extraFare;

    public LineRequest() {
    }

    public LineRequest(String name, String color) {
        this(name, color, null, null, 0, 0);
    }

    public LineRequest(String name, String color, Long upStationId, Long downStationId, int distance, int extraFare) {
        this.name = name;
        this.color = color;
        this.upStationId = upStationId;
        this.downStationId = downStationId;
        this.distance = distance;
        this.extraFare = extraFare;
    }

    public Line toEntity() {
        return new Line(name, color);
    }

    /// 추가 기능
}

 

 

이런 식으로 toEntity() 메소드를 만들고 Line을 반환하는 것입니다.

 

 

 

 

마지막으로, 익셉션의 계층을 나누라는 것입니다. 저는 처음에 이 피드백을 보고, 궁금한 점이 있어서 김고래에게 추가로 질문을 하였습니다.

 

 

 

 

저의 생각은 이러했습니다. 현재 dao에서 에러를 발생시키는 것은 맞지만, 원래 있던 에러가 아니고 저의 커스텀 에러로 교체하여 throw를 날리고, 그것에 대해 ControllerAdvice가 처리하니까 괜찮지 않냐는 것이었죠.

 

 

 

 

하지만, 이는 제가 잘못 이해한 것이었습니다. 커스텀 익셉션을 만든 것은 좋은데, 그 익셉션은 dao에서 사용하는 것이고, dao에서 사용하는 익센셥이 상태 코드를 갖는 것은 어색한 것이었죠. 그래서 해당 익셉션은 ClientException으로 묶어서 처리할 것이 아니라, 따로 계층을 두는 것이 바람직하다고 합니다. 이 부분은 ControllerAdvice에 또다른 메소드를 두어 처리하려고 합니다만, 중간곰과 내일 이야기를 나누어 볼 계획입니다.

 

 

페어 프로그래밍

오전에는 김고래의 피드백을 같이 반영해 보았고, 데일리 이후에는 전에 만들어 두었던 3단계 코드를 기능 별로 가져오면서 commit을 해 나갔습니다. 그리고 생각보다 테스트 부분에서 많은 어려움을 느꼈고, 그 안에서도 얻어간 개념이 많았습니다.

 

 

(1) 특정 필드를 제외하고 비교하는 방법

우리는 아마 assertj 문법으로 isEqualTo()를 가장 많이 사용할 것입니다. 아무래도 기대하는 값이 잘 나왔나 비교할 때가 흔하기 때문이죠. 하지만, 만약에 객체와 객체를 비교하려고 하는데 특정 필드를 제외하고 비교하려면 어떻게 해야 할까요?

 

 

    @DisplayName("구간을 생성한다.")
    @Test
    void save() {
        final Line createdLine = lineDao.save(new Line("2호선", "black"));

        final Section section = new Section(createdLine.getId(), 2L, 4L, 10);
        final Section createdSection = sectionDao.save(section);

        assertThat(createdSection.getUpStationId()).isEqualTo(section.getUpstationId());
        assertThat(createdSection.getDownStationId()).isEqualTo(section.getDownStationId());
        assertThat(createdSection.getDistance()).isEqualTo(section.getDistance());
    }

 

 

이렇게 Id를 제외한 나머지 필드만을 비교하고 싶다면, 하나 하나 getter를 통해 불러와서 비교할 수 있습니다. 하지만, 극단적으로 필드가 10개인데 Id만 제외하고 싶다면 어떻게 할까요? 그것을 제외하고 9개를 비교하기 위하여 9개의 isEqualTo()문을 써야할 수 밖에 없습니다.

 

이러한 불편함을 해결해 주는 assertj 문법이 있습니다. 

 

 

    @DisplayName("구간을 생성한다.")
    @Test
    void save() {
        final Line createdLine = lineDao.save(new Line("2호선", "black"));

        final Section section = new Section(createdLine.getId(), 2L, 4L, 10);
        final Section createdSection = sectionDao.save(section);

        assertThat(createdSection)
            .usingRecursiveComparison()
            .ignoringFields("id")
            .isEqualTo(section);
    }

 

 

바로 usingRecursiveComparison()과 ignoringFields()를 사용하는 것이죠. 이 중 전자의 메소드는 정확히 무엇을 하는 지는 모르겠으나, 핵심은 ignoringFields()입니다. 비교의 대상에서 무시할 필드를 설정한 다음 isEqualTo()를 사용하면 객체끼리의 비교를 할 수 있습니다.

 

 

비슷한 방식으로 여러 개의 객체를 비교하되, 특정 필드를 제외하고 싶을 수 있습니다. 이럴 때는 usingElementComparatorIgnoringFields()를 사용합니다.

 

 

    @DisplayName("동일한 노선의 전체 구간을 조회한다.")
    @Test
    void findByLineId() {
        final Line createdLine = lineDao.save(new Line("2호선", "black"));
        final List<Section> sections = new ArrayList<>(Arrays.asList(
            new Section(createdLine.getId(), 2L, 4L, 10),
            new Section(createdLine.getId(), 4L, 6L, 20)
        ));
        sections.forEach(section -> sectionDao.save(section));

        assertThat(sectionDao.findByLineId(createdLine.getId()))
            .usingElementComparatorIgnoringFields("id")
            .isEqualTo(sections);
    }

 

 

이번에도 마찬가지로 id만 제외하고 싶어서 필드에 id만 적었습니다. 만약, 해당 메소드가 없다면 어떻게 코드를 짜야할 지.. 끔찍합니다.

 

 

(2) Mock에서 인자를 임의로 넣는 방법

    @DisplayName("노선을 생성한다.")
    @Test
    void createLine() {
        final String name = "2호선";
        final String color = "black";
        final Long upStationId = 2L;
        final Long downStationId = 4L;
        final int distance = 10;
        final int extraFare = 1000;

        final Line line = new Line(name, color);
        final LineRequest lineRequest = spy(new LineRequest(name, color, upStationId, downStationId, distance, extraFare));
        given(lineDao.save(line)).willReturn(new Line(1L, name, color));
        given(sectionService.createSection(eq(1L), any(SectionRequest.class))).willReturn(new SectionResponse());
        final LineResponse createdLine = lineService.createLine(lineRequest);

        verify(lineRequest, times(1)).toEntity();
        verify(lineDao, times(1)).save(line);
        verify(sectionService, times(1)).createSection(eq(1L), any(SectionRequest.class));
        assertThat(createdLine.getId()).isEqualTo(1L);
    }

 

 

해당 코드는 정답은 아니지만, 일단 테스트는 돌아가는 코드입니다. 위 코드를 유심히 보시면 given() 안의 eq()나 any()라는 요상한 메소드가 붙어있는 것을 알 수 있습니다. any()는 매개 변수가 어떤 것이 들어가도 상관 없을 때 사용하면, eq()는 비슷하지만 특정 값을 넣어야 할 때 사용합니다. eq()를 사용하게 된 이유를 구체적으로 설명드리자면, 해당 createSection()의 인자로 line_id를 넣는데, 1L을 지정해 주어야 해서 그냥 (1L, any(SectionRequest.class)를 사용했더니 에러가 발생했습니다. 자세한 이유는 모르겠으나, any()와 함께 쓰는 매개변수들에 특정 값을 넣고 싶으면 eq()를 사용해야 한다고 합니다.

 

 

    @Transactional
    public LineResponse createLine(final LineRequest lineRequest) {
        final Line line = lineDao.save(lineRequest.toEntity());

        final Long upStationId = lineRequest.getUpStationId();
        final Long downStationId = lineRequest.getDownStationId();
        final int distance = lineRequest.getDistance();

        sectionService.createSection(line.getId(), new SectionRequest(upStationId, downStationId, distance));
        return LineResponse.from(line);
    }

 

 

그리고 위 코드의 createSection()을 보면 아시다시피 리턴 값이 크게 중요하지가 않습니다. 즉, 어떤 값이 반환되든 상관 없다는 의미라서 Mock 코드의 willReturn()으로 any(LineResponse.class)를 넣어주었더니 Matcher의 개수가 맞지 않다는 에러가 발생했습니다. 아무래도 저와 중간곰 모두 Mock의 이해가 부족한 편이라서 이 부분은 적당히 아무 값이나 반환하게 만들고 넘어갔습니다. 천천히 시간을 들여서 Mock을 공부해 보려고 합니다..

 

 

정리

그 외에는 자질구레한 버그를 고치기 위해서 시간을 쓰기도 했고, 구간 등록 관련하여 리팩토링하는데 꽤나 많은 시간을 들였습니다. 하지만, 생각보다 구간 등록의 코드를 줄이기 어려워서 이 부분과 구간 제거 기능을 내일 구현할 예정입니다. 이번 미션의 마감이 내일이므로 부지런히 해야겠습니다.