Skip to content

2호선 VIP#14

Open
woo-chang wants to merge 40 commits into
woowacourse-code-review-study:mainfrom
woo-chang:step2
Open

2호선 VIP#14
woo-chang wants to merge 40 commits into
woowacourse-code-review-study:mainfrom
woo-chang:step2

Conversation

@woo-chang

Copy link
Copy Markdown

No description provided.

woo-chang and others added 30 commits May 14, 2023 10:31
* docs: 기능 요구사항 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 역 이름 길이 검증 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 역 이름 형식 검증 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 역 이름이 공백일 때 검증 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* test: 이름에 대한 생성 테스트 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 거리 검증 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 두 역과 역 사이의 거리를 가지는 구역 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* remove: 사용하지 않는 파일 정리

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 이름 검증 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 색 검증 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* docs: API 명세 작성

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 역 등록 API 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 역 정보 조회 API 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* docs: API 명세 수정

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 등록 API 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 조회 API 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 목록 조회 API 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 역 등록 API 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선에서 역 삭제 API 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선에 새로운 역 등록 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선에 존재하는 역 제거 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* test: 노선에 역 추가, 삭제 후 거리를 검증하는 테스트 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 역을 생성하는 로직 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 역을 조회하는 로직 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선을 등록하는 로직 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* refactor: 노선 목록 조회시 노선 안의 역 정보를 포함하도록 변경

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 조회 로직 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 목록 조회 로직 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선에 역 등록 로직 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노션 역 삭제 로직 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: DB에 역을 저장하는 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: DB에 노선을 저장하는 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: DB에 구간을 저장하는 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: DB에서 모든 노선 조회 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: DB에서 노선 업데이트 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: DB에서 특정 노선의 구간을 삭제하는 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: DB에 모든 구간 정보를 저장하는 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 역 ID로 역 정보 조회 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 저장 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 ID로 정보 조회 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 정보 조회 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 노선 정보 업데이트 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* refactor: 모든 노선 정보 조회 시 노선을 다시 조회하지 않도록 수정

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 예외 발생 시 로그를 남기는 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* refactor: 유효한 구간 정보가 순서대로 입력되지 않아도 정상적으로 노선을 생성하도록 변경

Co-authored-by: woo-chang <clllickme@naver.com>

* refactor: 컨벤션 통일

Co-authored-by: woo-chang <clllickme@naver.com>

* refactor: Service에 Transactional 적용

Co-authored-by: woo-chang <clllickme@naver.com>

* test: Controller 통합테스트 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* feat: 내부 예외처리 기능 추가

Co-authored-by: woo-chang <clllickme@naver.com>

* docs: README 업데이트

Co-authored-by: woo-chang <clllickme@naver.com>

* fix: 역간 거리의 최대길이 제한 삭제

Co-authored-by: woo-chang <clllickme@naver.com>

* refactor: 예외 메시지 수정

Co-authored-by: woo-chang <clllickme@naver.com>

* test: 거리에 대한 테스트 수정

Co-authored-by: woo-chang <clllickme@naver.com>

* docs: 수정된 기능 반영

* chore: 코드 컨벤션 수정

* feat: Swagger를 이용해 API 문서화

* refactor: 예상할 수 없는 예외에 대한 로그 레벨 수정

* refactor: 노선 색에 대한 예외 세분화

* test: 의도와 다른 테스트명 수정

* refactor: 예외에서 상태 코드 관리 책임 분리

* refactor: request 기본 생성자 접근 제어자 수정

* refactor: 도메인 검증 통일성 있게 수정

* refactor: service에서 repository를 사용하도록 수정

* refactor: 도메인 정적 팩토리 메서드 수정

* refactor: 코드 컨벤션 수정

---------

Co-authored-by: bjk1649 <qhwns1649@naver.com>

@greeng00se greeng00se left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다즐 고수시네요 👍 잘 보고갑니다.
도메인이 외부 라이브러리에 많이 의존하고 있는데 이 부분을 인터페이스를 통해 결합도를 낮춰보는건 어떨까?

Comment on lines +16 to +23
public FareStrategy fareStrategy() {
return new FareStrategyComposite(
List.of(
new DistanceFareStrategy(),
new RouteFareStrategy(),
new AgeFareStrategy()
)
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Composite 👍

Comment on lines +12 to +19
@Bean
public OpenAPI openAPI() {
final Info info = new Info()
.title("지하철 API Document")
.version("v0.0.1")
.description("지하철 API 명세서입니다.");
return new OpenAPI()
.components(new Components())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swagger 👍


@Operation(summary = "경로 정보 조회 API", description = "출발역에서 도착역까지의 경로 정보를 조회합니다.")
@GetMapping("/shortest-path")
public ResponseEntity<ShortestPathResponse> findShortestPath(@Valid @RequestBody PassengerRequest request) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쿼리 스트링으로 받아보는건 어떨까?


public enum AgePolicy {

BABY(age -> age < 6, fare -> fare),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

애기는 꼬맹이랑 청소년보다 돈을 많이 받나요?

Comment on lines +7 to +14
BASE_FIFTY(50, distance -> {
final long additionDistance = distance - 50;
return calculateDistanceFarePerStep(additionDistance, 8);
}),
BASE_TEN(10, distance -> {
final long additionDistance = Math.min(BASE_FIFTY.base, distance) - 10;
return calculateDistanceFarePerStep(additionDistance, 5);
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

거리별 정책도 enum으로 만들어줬넹

Comment on lines +49 to +62
if (upwardPosition == ADDITIONAL_INDEX) {
if (isAddAtFront(downwardPosition)) {
addSectionEndPoints(true, upward, downward, distance);
return;
}
addUpwardSectionBetweenStations(upward, downward, distance, downwardPosition);
}

if (downwardPosition == ADDITIONAL_INDEX) {
if (isAddAtEnd(upwardPosition)) {
addSectionEndPoints(false, upward, downward, distance);
return;
}
addDownwardSectionBetweenStations(upward, downward, distance, upwardPosition);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전략 패턴으로 분리해보는건 어떨까요?

Comment on lines +7 to +20
private static final int MINIMUM_VALUE = 0;

private final int value;

public Distance(final int value) {
validate(value);
this.value = value;
}

private void validate(final int value) {
if (value < MINIMUM_VALUE) {
throw new InvalidDistanceException("역 사이의 거리는 0이상이어야합니다.");
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요구사항에 따르면 1이상이어야 되는거 아닐까요?

Comment on lines +52 to +74
final List<StationResponse> stationsOfLineTwo = List.of(
new StationResponse(1L, "잠실역"),
new StationResponse(2L, "잠실새내역")
);
final List<StationResponse> stationsOfLineFour = List.of(
new StationResponse(3L, "이수역"),
new StationResponse(4L, "서울역")
);
final List<LineResponse> lines = List.of(
new LineResponse(1L, "2호선", "초록색", 500, stationsOfLineTwo),
new LineResponse(2L, "4호선", "하늘색", 1000, stationsOfLineFour));
final LinesResponse response = new LinesResponse(lines);

given(lineService.findLines()).willReturn(response);

final MvcResult mvcResult = mockMvc.perform(get("/lines"))
.andDo(print())
.andExpect(status().isOk())
.andReturn();

final String jsonResponse = mvcResult.getResponse().getContentAsString(Charset.forName("UTF-8"));
final LinesResponse result = objectMapper.readValue(jsonResponse, LinesResponse.class);
assertThat(result).usingRecursiveComparison().isEqualTo(response);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 코드가 길어지는데 given-when-then 패턴을 사용해보는건 어떨까요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants