본문 바로가기
Memo/우테코 4기

[우테코] 지하철 경로 미션 1~2 단계 학습 로그

by 연로그 2022. 5. 29.
반응형

목차

1. GitHub 저장소

2. 요구사항

3. 피드백

4. 셀프 회고


 


1. GitHub 저장소 🐱‍💻

 

1단계 Repository: https://github.com/yeon-06/atdd-subway-path/tree/step1

1단계 Pull Request: https://github.com/woowacourse/atdd-subway-path/pull/176

2단계 Repository: https://github.com/yeon-06/atdd-subway-path/tree/step2

2단계 Pull Request: https://github.com/woowacourse/atdd-subway-path/pull/258

 


2. 요구사항 🚀

 

img: flaticon

 

1단계 요구사항

  • 경로 조회 API 구현
    • 최단 경로 라이브러리 JGraphT 이용
    • 요금 계산
      • 기본 금액 1250원 (10km까지 적용)
      • 5km마다 100원 추가 (50km까지 적용)
      • 8km마다 100원 추가
  • 기존 API 수정
    • 지하철 노선에 extraFare 정보 추가

 

2단계 요구사항

  • 요금 계산시 환승 고려
    • 가장 비싼 지하철의 추가 요금을 적용
    • 연령별 요금 할인 적용
      • 13세 이상~19세 미만: 운임에서 350원을 공제한 금액의 20%할인
      • 6세 이상~13세 미만: 운임에서 350원을 공제한 금액의 50%할인

 


3. 피드백 📚

😵: 본인(연로그), 🦅: 구구(리뷰어)

 

 

3-1. 주생성자와 부생성자

AS-IS 코드에서는 부생성자가 추가로 생길 경우 validate가 잘 되었는지 확인하기 어렵다

// AS-IS
public User(String name, int age) {
    validate(name);
    this.name = name;
    this.age = age;
}
public User(Long id, String name, int age) {
    this(name, age);
    this.id = id;
}
// TO-BE
public User(String name, int age) {
    this(null, name, age);
}
public User(Long id, String name, int age) {
    validate(name);
    this.id = id;
    this.name = name;
    this.age = age;
}

 

 

3-2. request, response는 로그로 남겨야할까?

 이번에 Spring 스터디에서 AOP에 대해 공부하며 미션에서도 적용해보았다. Controller에서 받는 파라미터들과 반환하는 데이터를 콘솔 로그에 찍어줬다. 

 

🦅: 로그로 꼭 남겨야할 이유가 있을까요? 데이터가 너무 길거나 로그로 남기면 안되는 데이터인 경우가 있진 않을까요?

 

😵: request, response 값을 로그로 남긴 후에 오류가 발생하면 아 이거 오류 났네... 이 API는 request 값들을 받았구나 어? 이 값은 빈 값이 들어오면 안되는데 빈 값이 들어왔었네? 하면서 로그 확인하며 에러 파악하는게 쉬울거라 생각했습니다! 로그로 남기면 안되는 데이터가 존재하면 따로 지정해줄 수 있을 것 같아요

 

🦅: 해당 오류가 나는 부분에서 해당 데이터만 로그로 찍으면 되지 않을까요?

 

 👉 납득하고 AOP를 통해 로그 찍은 부분들 전부 삭제

 

 

3-3. 상수가 많다면 enum으로 관리하자

 거리별로 요금을 다르게 계산하는 부분이 있었다. 이 부분에 대해서 마땅한 상수명이 생각나지 않아 DISTANCE_UNIT_1, DISTANCE_UNIT_2 이런 식으로 이름 지었다가 바로 구초리가 날라왔다. Distance라는 Enum을 만드니까 아래와 같이 의미 있는 이름을 지을 수 있게 되었다.

public enum Distance {

    BASIC(0, 0, 9),
    MIDDLE(5, 10, 49),
    FAR(8, 50, Integer.MAX_VALUE);

    private final int unit;
    private final int startPoint;
    private final int endPoint;

    // ...
}

 

 

3-4. 제공되는 메서드 사용하기

 코딩 테스트에서는 1초라도 성능을 우선시 하다보니 불필요한 형변환을 자제하기 위해 메서드를 따로 만들고는 했다. 별 생각 없이 AS-IS 같은 코드를 작성했다가 이미 잘 만들어진 함수를 사용하라는 리뷰를 받았다. (직접 짠 코드보다 오류가 발생할 확률이 적어지므로)

// AS-IS
private int floor(int number) {
    return number - number % 10;
}
// TO-DO
private int floor(int number) {
    return (int) Math.floor(number);
}

 

 

3-5. 메서드 설명을 위한 주석 쓰지 않기

 로직을 짜다보니 너무 복잡한 구조로 짜여진 메서드가 있었다. 메서드명도 findUpdateWhenAdd 이런 식으로 모호했다. 이를 설명하기 위해 구간이 새로 추가될 때 기존 구간 중에서 변경되는 값이 있으면 해당 구간을 반환한다. 이런 식으로 구구절절 썼다. 해당 부분에 대해 당연히 리뷰가 들어왔고 과감하게 메서드를 삭제하기로 결심했다. 대신 명확한 의미를 가진 메서드 add()를 생성해 사용했다.

 

 

3-6. 객체지향적인 코드 만들기

 Section은 상행역, 하행역을 가지고 있다. 해당 부분에 대해 DB와 똑같이 상행역의 id, 하행역의 id를 갖게 만들었다. 이는 테이블을 그대로 가져갔을 뿐이고 객체지향적이지 않다. Section이 id가 아닌, Station 자체를 갖도록 수정했다. 로직을 수정하다보니 DB에서 호출하는 과정은 더 까다로웠지만 훨씬 더 간편해진 로직이 많았다.

 

 

3-7. 쿼리는 공통적으로 쓰지 마라

 select 절이 너무 복잡해서 select ... from ...는 공통 상수로 빼고 where절만 따로 덧붙였다. 그러자 selecAll이 변경된다면 모든 find 관련 메서드에 영향이 가는 치명적인 문제가 발생했다. 조금 복잡해보이더라도 쿼리문은 따로 작성하도록 수정했다.

private String selectAll = "select * from Section ";

public List<Section> findByLineId(Long id) {
    String sql = selectAll + "where id = :id";
    // ...
}

 

 

3-8. 한 줄에 120칸

코드가 너무 길어지면 한 화면에 보기 힘들어진다. 줄바꿈은 120칸 정도에서 하는 것으로 권장된다. IntelliJ에서는 이를 회색 줄로 표시해주고 있다.

 

 

3-9. 정적 팩토리 메서드 네이밍 컨벤션

  • from: 매개변수 하나 -> 해당 타입의 인스턴스 반환
  • of: 매개변수 여러개 -> 해당 타입의 인스턴스 반환
  • valueOf: from과 of의 더 자세한 버전
  • instance 또는 getInstance: 매개변수로 명시한 인스턴스 반환 (같은 인스턴스 보장 X)
  • create 또는 newInstance: 매개변수로 명시한 인스턴스 반환 (매번 새로운 인스턴스)
  • getType: getInstance와 같으나 생성할 클래스가 아닌 다른 클래스에 팩토리 메서드 정의할 때 사용
  • newType: newInstance와 같으나 생성할 클래스가 아닌 다른 클래스에 팩토리 메서드 정의할 때 사용
  • type: getType, newType의 간결한 버전

 

 

3-10. 정렬 기준 명시적으로 만들기

Comparator를 직접 정의해서 사용하는 부분이 있었다. 내림차순으로 정렬해야 했기에 다음과 같이 코드를 작성했다.

Comparator.comparingInt(it -> -it.distanceUnit)

 

🦅: 이런 코드는 다른 개발자가 놓치기 쉽습니다. 읽기 쉽게 만들 방법 없을까요? Comparator 클래스가 제공하는 다른 기능은 없을지 찾아봅시다.

 

👉 reversed() 활용

Comparator.comparingInt(Distance::getDistanceUnit).reversed()

 

 


4. 셀프 회고 🙃

 

 평소보다 학습 로그를 늦게 올렸다. 글을 미루려던건 아니고 미션이 바로 얼마전에야 머지되었기 때문에... 원래는 최대한 일찍 끝내려고 하는 타입이라 마지막 날에는 머지될 수 있었는데 요번에는 잘 안됐다. 요번 담당 리뷰어가 너무 바쁘셨던 것도 있고 미션 기간이 다른때보다 짧긴 했다. 그리고 잘못된 설계로 인해 프로젝트의 대부분을 수정하는 일이 너무너무 많았다.

 

 이번 미션을 진행하며 너무 많은 후회를 했던 부분이 이 설계와 관련되어 있다. 이전 미션에서 MVC에서는 TDD를 어떻게 하지? 대충 컨트롤러 테스트부터 작성하고 컨트롤러 -> 서비스 -> 도메인 순으로 로직을 작성하면 되나? 싶어서 되는대로 개발을 진행했었다. 그 결과 도메인 = 엔티티 = 테이블... 객체지향은 개나 줘버린 코드가 되었다... 이런 내 코드를 선택해준 내 페어... 조시에게 너무 감사하고 미안하다^ㅡT... 2단계에서 구초리 리뷰를 받으며 걍 되엎어버리고 지하철 노선도 미션부터 다시 할까도 고민했다. 구초리 받은 시점이 이미 미션 마지막 날이이기도 하고 문제를 회피하는 것처럼 느껴져서 기존 코드를 최대한 리팩터링 했지만... 여전히 이도저도 아닌 코드로 존재한다. 누가 내 코드를 보면 레거시 냄새난다고 할듯... 방학 때 (시간적 여유가 된다면) 다시 해보고싶은 미션 1위가 되지 않을까 싶다...

 

 항상 즐겁게만 지내오던 우테코 생활 중에 이번 미션이 가장 힘들었던 것 같다. 미션의 난이도가 어려웠다기보다 번아웃이 오려하는데 한 것도 없는데 번아웃이 와도 되나.. 라는 의문이 들던 시기같다. 나도 사람이다보니 스트레스가 안 쌓일 수가 없다. 테스트 코드만 돌려도 터지려고 하는 노트북, 한시간 내내 서있어야하는 출퇴근, 아무리 고쳐도 마음에 들지 않는 내 코드, 부족한 생활비, 부족한 시간... 옛날에는 그림과 게임이라는 취미를 통해 스트레스를 해소했다. 이제는 그림을 그리는건 흥미가 떨어졌고 게임을 하기엔 시간이 없다. 적절한 스트레소 해소법이 사라진 것 같다. 운동이라는 새로운 취미를 가져볼까 했는데 약속이 없으면 나가질 않으니 운동도 안하게 된다... 다음 미션이 끝나고 나면 방학인데 그때 새로운 취미를 찾아볼까라는 생각을 하고 있다.

반응형