Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2단계 - 경로 조회 기능] 테오(최우성) 미션 제출합니다. #161

Merged
merged 49 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
2ae9c7f
refactor: 테이블 이름 변경
woosung1223 May 17, 2023
deaa0b7
refactor: 외래키 제약조건 추가
woosung1223 May 17, 2023
07236eb
refactor: 데이터베이스 예외 발생 시 문구 반환
woosung1223 May 17, 2023
865285c
chore: 코드 리포맷팅
woosung1223 May 17, 2023
ba2fe46
refactor: 동등성 비교조건 변경
woosung1223 May 18, 2023
e88520c
refactor: URI에서 컬렉션이 드러나도록 변경
woosung1223 May 18, 2023
2a074b6
test: 컨트롤러 테스트 보완
woosung1223 May 18, 2023
358c719
refactor: logback-access가 아닌 인터셉터로 로깅 처리
woosung1223 May 18, 2023
3d57ba3
refactor: 서비스 입력 모델 생성
woosung1223 May 18, 2023
85cf081
refactor: 서비스 출력 모델 생성
woosung1223 May 18, 2023
355093a
chore: 패키지 구조 변경
woosung1223 May 18, 2023
d9a0a43
chore: 패키지 이름 변경
woosung1223 May 18, 2023
a5846d3
feat: 경로 조회 및 요금 계산 비즈니스 로직 구현
woosung1223 May 18, 2023
8cb28a2
refactor: RouteMap 객체를 재사용 가능하게 Section List으로 구성
woosung1223 May 18, 2023
0a8c562
feat: 요금 계산용 값 객체 생성
woosung1223 May 18, 2023
5755cce
refactor: 경로 조회 및 요금 계산 로직 수정
woosung1223 May 18, 2023
deb3904
feat: 경로탐색 구현체 생성
woosung1223 May 18, 2023
ff7a588
feat: 경로 조회 및 요금 계산 API 구현
woosung1223 May 18, 2023
436df91
test: 통합 테스트용 http 파일 생성
woosung1223 May 18, 2023
87fba10
chore: 테스트 패키지 구조 프로덕션과 동일하게 변경
woosung1223 May 18, 2023
8462216
fix: 몇몇 입력 모델에서 검증을 하지 않던 문제 수정
woosung1223 May 18, 2023
a1841b4
refactor: 메소드 추가 분리
woosung1223 May 19, 2023
c3cc929
refactor: 경로 추적 시 두 번의 연산을 수행하지 않도록 DTO 생성
woosung1223 May 19, 2023
18406a6
chore: 사용하지 않는 메소드 제거
woosung1223 May 19, 2023
4b4ba7d
test: jgrapht 구현체 테스트 생성
woosung1223 May 19, 2023
b065df8
chore: 메소드 분리, 가독성 개선
woosung1223 May 19, 2023
fab2161
test: service 테스트 보완, dummy.sql 생성
woosung1223 May 19, 2023
55ebc2a
test: 최단 경로 관련 통합테스트 생성
woosung1223 May 19, 2023
24f3558
chore: sql 파일 위치 변경
woosung1223 May 19, 2023
74ae079
feat: 프로덕션 환경에서는 MySQL, 테스트 환경에서는 H2를 사용하도록 변경
woosung1223 May 20, 2023
07ab84f
fix: 더미 데이터 사용에 따른 테스트 코드 수정
woosung1223 May 20, 2023
3ff06bc
chore: .http 파일 삭제
woosung1223 May 20, 2023
a55a7cc
feat: swagger를 통한 API 명세 자동화
woosung1223 May 20, 2023
8697f4a
chore: 메소드명 변경
woosung1223 May 20, 2023
77cef32
docs: 고민사항 기재
woosung1223 May 20, 2023
c2d164e
chore: 클래스명 변경
woosung1223 May 21, 2023
2c6abba
refactor: `@Validated` 어노테이션을 통한 검증
woosung1223 May 21, 2023
caf3f04
chore: 구현체가 있으므로 인터페이스에서 `@Component` 제거
woosung1223 May 21, 2023
9f3601e
chore: 메소드 인자명 변경
woosung1223 May 21, 2023
2ff9d56
chore: 불필요한 `support` 패키지 삭제
woosung1223 May 21, 2023
17a5215
refactor: repository의 insert가 도메인 엔티티를 반환하도록 수정
woosung1223 May 21, 2023
9126f5d
test: Repository 테스트 보완
woosung1223 May 22, 2023
b0d4709
refactor: 출력 모델에서 도메인 엔티티 제거
woosung1223 May 22, 2023
13ee83e
refactor: 중복된 의미를 가지는 예외객체 제거
woosung1223 May 22, 2023
4fae785
refactor: 요금 계산 로직을 enum으로 수행
woosung1223 May 22, 2023
05d3183
refactor: 입력 모델 규칙 위반에 대한 예외처리 생성
woosung1223 May 22, 2023
eb7e1f0
chore: 불필요한 `@Valid` 제거
woosung1223 May 22, 2023
c294bd5
docs: 리팩토링 기록 작성 및 아키텍쳐 이미지 변경
woosung1223 May 22, 2023
4ffbe88
refactor: Repository가 H2와 관련이 없기에 네이밍 수정
woosung1223 May 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ build/
*.iws
*.iml
*.ipr
out/
/out/

### NetBeans ###
/nbproject/private/
Expand All @@ -30,4 +30,4 @@ out/
/.nb-gradle/

### VS Code ###
.vscode/
.vscode/
57 changes: 55 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,25 @@
- [x] 노선 목록 조회 API 구현
- [x] 각 노선에 포함된 역을 순서대로 보여주도록 응답을 개선한다.

- [x] 경로 조회 및 요금 조회 API 구현
- [x] 출발역과 도착역 사이의 최단 거리 경로, 총 거리 정보를 응답한다.
- [x] 여러 노선의 환승을 고려한다.
- [x] 경로 조회 시 요금 정보를 포함하여 응답한다.
- [x] 기본운임(10KM 이내)은 1,250원이다.
- [x] 이용 거리 초과 시 추가운임을 부과한다.
- [x] 10KM부터 50KM 까지는 5KM마다 100원이 추가된다.
- [x] 50KM를 초과하면 8KM마다 100원이 추가된다.

---
# step 1 기록
### Architecture
![img.png](img.png)

---

<details>
<summary> step 1 기록 </summary>
<div>

- [x] `Line` 도메인 엔티티를 CRUD용으로도 사용하기도 하고, 비즈니스 로직을 수행할 때 사용하기도 해서 문제가 많았음.
- [x] 문제점 1: CRUD 시점에는 완전한 도메인 객체가 아님.
- [x] 문제점 2: 하나의 도메인 엔티티가 여러 개념을 내포하고 있음.
Expand All @@ -91,4 +107,41 @@
- [x] N번 쿼리를 보내야 했던 문제를 1번으로 해결
- [x] 데이터베이스 레벨에서 조인을 하는 방법도 있을 듯 한데.. 관련 DAO나 Row를 또 한번 만들어줘야 하는 문제점
- [x] 조인할 때마다 새로운 DAO를 만들 것인가?
-

</div>
</details>

<details>
<summary> step 2 기록 </summary>
<div>

# step 2 기록
- [x] `ON DELETE RESTRICT` 를 통해 외래키 제약조건 부여
- [x] `SECTION` 의 참조 필드는 `STATION` 행이 삭제되었다고 해서 같이 삭제가 되거나 NULL 처리를 할 수 없음
- [x] 도메인 제약조건이 깨지기 때문.
- [x] 따라서 서비스 로직에서 삭제 방어를 하거나, 외래키에 제약조건을 부여하는 방법이 있을 듯 함.
- [x] 이번에는 외래키 제약조건을 통해 무결성 보장.
- [x] 현재 `동일한 이름을 가진 STATION을 두 번 생성하려는 경우` 등은 테이블 제약조건에 의해 불가능하다.
- [x] 즉, 영속성 레벨에서 예외가 발생한다.
- [x] 이는 다르게 말하면 불필요한 데이터가 영속성 레이어까지 침투한다는 것이다.
- [x] 영속성 레이어까지 신뢰하지 못하는 데이터를 침투시킬것인가? 그렇다면 서비스에서 모든 무결성 검사를 진행해야 할까? 고민해보기.
- [x] 기본적으로 제공된 코드에 `logback-access.xml`이 존재했고, 이에 대한 의존성도 설정되어 있었음.
- [x] 찾아보니 컨테이너 레벨에서 로깅 기능을 제공해주는 듯 함.
- [x] 프로젝트 할 때 도입 고려해보면 좋을 듯.
- [x] 서비스 레이어에서 입력/출력 모델 생성
- [x] 표현 계층과 비즈니스 계층의 격리를 하기 위함
- [x] `RouteMap` 객체를 재사용 가능하게 변경
- [x] `Station`을 들고 있는게 아닌, `Section`을 들고 있게 한다면 그래프 탐색에도 재사용 가능
- [x] 경로를 구하는 로직이나, 요금을 계산하는 로직은 우리 시스템의 일부인데 도메인 엔티티 어디에서도 이런 기능은 존재하지 않는다.
- [x] 즉, `경로 및 요금 로직`을 외부 모듈로 다루니까 이런 문제가 발생함.
- [x] 하지만 시스템의 핵심 로직이라고 하더라도 무조건 다 도메인 엔티티에 정의되어야 하는 건 아니지 않을까?
- [x] 예를 들어, 인증도 핵심 로직이지만 도메인 엔티티에 인증 절차를 직접 정의하지는 않음.
- [x] service 테스트에 대한 고민
- [x] `LineService`는 `LineProperty`, `Station` 데이터가 존재한다는 가정 하에 동작할 수 있다.
- [x] 그러면 Mocking을 할 것인가? 테스트 코드가 너무 더러워짐.
- [x] Mocking을 하지 않을 것인가? 그러면 더미 데이터를 매번 생성해줘야 함.
- [x] 아니면 Fake 객체를 만들 것인가?
- [x] swagger 사용, API 명세 자동화했음.
- [x] 정말 기본적인 기능만 사용했는데, 추후 프로젝트 시 적절하게 사용한다면 협업이 용이해질 듯 함.
</div>
</details>
8 changes: 5 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@ repositories {
dependencies {
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'org.springframework.boot:spring-boot-starter-jdbc'

implementation 'net.rakugakibox.spring.boot:logback-access-spring-boot-starter:2.7.1'
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.jgrapht:jgrapht-core:1.0.1'
implementation 'org.springdoc:springdoc-openapi-ui:1.6.6'

testImplementation 'io.rest-assured:rest-assured:4.4.0'
testImplementation 'org.springframework.boot:spring-boot-starter-test'

runtimeOnly 'com.h2database:h2'
runtimeOnly 'com.mysql:mysql-connector-j'
}

test {
useJUnitPlatform()
}
}
Binary file added img.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package subway.application.domain;
package subway.application.core.domain;

import subway.application.exception.DistanceExceedException;
import subway.application.core.exception.DistanceExceedException;

import java.util.Objects;

Expand Down
53 changes: 53 additions & 0 deletions src/main/java/subway/application/core/domain/Fare.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package subway.application.core.domain;

import subway.application.core.exception.DistanceNotPositiveException;

public class Fare {

private static final int BASE_FARE = 1_250;
private static final double BASE_DISTANCE = 10.0;
private static final double REFERENCE_DISTANCE_UNDER_FIFTY = 5.0;
private static final double REFERENCE_DISTANCE_OVER_FIFTY = 8.0;

private final int value;
pjy1368 marked this conversation as resolved.
Show resolved Hide resolved

public Fare(double distance) {
validate(distance);
this.value = chargeOf((int) distance);
}

private void validate(double distance) {
if (distance < 0.0) {
throw new DistanceNotPositiveException();
}
}

private int chargeOf(double distance) {
if (Double.compare(distance, BASE_DISTANCE) < 0) {
return BASE_FARE;
}
return BASE_FARE + calculateOverFare(distance);
}

private int calculateOverFare(double distance) {
if (isDistanceBetweenTenAndFifty(distance)) {
distance -= 10.0;
return calculateOverFareForEveryDistance(distance, REFERENCE_DISTANCE_UNDER_FIFTY);
}
distance -= 50.0;
return calculateOverFareForEveryDistance(40, REFERENCE_DISTANCE_UNDER_FIFTY) +
calculateOverFareForEveryDistance(distance, REFERENCE_DISTANCE_OVER_FIFTY);
}

private static boolean isDistanceBetweenTenAndFifty(double distance) {
return Double.compare(distance, 10.0) >= 0 && Double.compare(distance, 50.0) <= 0;
}

private int calculateOverFareForEveryDistance(double distance, double referenceDistance) {
return (int) ((Math.ceil(((int) distance - 1) / (int) referenceDistance) + 1) * 100);
Copy link

Choose a reason for hiding this comment

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

((int) distance -1) / (int) referenceDistance)는 int / int 인데 나눗셈 과정에서 발생할 수 있는 문제는 없을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

리뷰 남겨주신거 보긴 했는데 테코톡 + 미션으로 바빠 이제 코멘트를 남기네요! 🥲

int / int 나눗셈으로 인해 distance가 0.9이든, 0.0이든 동일시되는 문제가 발생할 것 같습니다.
요금 정책이 해당 계산식이라 저렇게 구성한 것이긴 한데, 거리가 X.5KM 이상이면 반올림해서 계산한다 등의 비즈니스 규칙을 따로 정의하고 수식을 바꿔도 괜찮을 것 같네요.

}

public int value() {
return value;
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package subway.application.domain;
package subway.application.core.domain;

import subway.application.exception.StationAlreadyExistsException;
import subway.application.exception.StationConnectException;
import subway.application.exception.StationNotExistsException;
import subway.application.exception.StationTooFarException;
import subway.application.core.exception.StationAlreadyExistsException;
import subway.application.core.exception.StationNotExistsException;
import subway.application.core.exception.StationTooFarException;
import subway.application.core.exception.StationConnectException;

import java.util.List;
import java.util.Objects;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.domain;
package subway.application.core.domain;

import java.util.Objects;

Expand Down
102 changes: 102 additions & 0 deletions src/main/java/subway/application/core/domain/RouteMap.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package subway.application.core.domain;

import subway.application.core.exception.CircularRouteException;
import subway.application.core.exception.RouteNotConnectedException;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public class RouteMap {

private final List<Section> routeMap;

public RouteMap(List<Section> sections) {
this.routeMap = alignedRouteOf(sections);
}

private List<Section> alignedRouteOf(List<Section> sections) {
if (sections.isEmpty()) {
return Collections.emptyList();
}
return alignSections(sections);
}

private List<Section> alignSections(List<Section> sections) {
List<Section> temporarySections = new ArrayList<>();
Section indexSection = findFirstSection(sections);
while (canMove(sections, indexSection) && !isInnerCircle(sections, temporarySections)) {
temporarySections.add(indexSection);
indexSection = findNext(sections, indexSection);
}
temporarySections.add(indexSection);
validate(sections, temporarySections);
return temporarySections;
}

private Section findFirstSection(List<Section> sections) {
Station firstStation = getEndPoints(sections).stream()
.findAny()
.orElseThrow(CircularRouteException::new);

return sections.stream()
.filter(section -> section.hasUpBound(firstStation))
.findAny()
.orElseThrow();
}

private List<Station> getEndPoints(List<Section> sections) {
List<Station> allUpBounds = sections.stream()
.map(Section::getUpBound)
.collect(Collectors.toList());

List<Station> allDownBounds = sections.stream()
.map(Section::getDownBound)
.collect(Collectors.toList());

allUpBounds.removeAll(allDownBounds);
return allUpBounds;
}

private boolean canMove(List<Section> sections, Section targetSection) {
return sections.stream()
.anyMatch(section -> section.getUpBound().equals(targetSection.getDownBound()));
}

private boolean isInnerCircle(List<Section> originalSections, List<Section> temporarySections) {
return originalSections.size() < temporarySections.size();
}

private Section findNext(List<Section> sections, Section targetSection) {
return sections.stream()
.filter(section -> section.getUpBound().equals(targetSection.getDownBound()))
.findAny()
.orElseThrow();
}

private void validate(List<Section> originalSections, List<Section> temporarySections) {
if (originalSections.size() < temporarySections.size()) {
throw new CircularRouteException();
}
if (temporarySections.size() < originalSections.size()) {
throw new RouteNotConnectedException();
}
}

public List<Station> stations() {
Set<Station> alignedStations = new HashSet<>();
routeMap.forEach(section -> {
alignedStations.add(section.getUpBound());
alignedStations.add(section.getDownBound());
});
return alignedStations.stream()
.collect(Collectors.toUnmodifiableList());
}

public List<Section> values() {
return Collections.unmodifiableList(routeMap);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package subway.application.domain;
package subway.application.core.domain;

import subway.application.exception.SectionConnectException;
import subway.application.core.exception.SectionConnectException;

import java.util.Objects;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.domain;
package subway.application.core.domain;

import java.util.Objects;

Expand All @@ -25,11 +25,11 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Station station = (Station) o;
return Objects.equals(name, station.name);
return Objects.equals(id, station.id) && Objects.equals(name, station.name);
}

@Override
public int hashCode() {
return Objects.hash(name);
return Objects.hash(id, name);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class CircularRouteException extends ExpectedException {
Comment on lines +1 to 3
Copy link

Choose a reason for hiding this comment

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

시간이 되신다면 전에 언급주신 것처럼 Exception도 �어느 정도 계층화를 시도해보면 좋겠습니다~
DatabaseException, NetworkException을 두는 방법도 있고 각 계층마다 exception 패키지를 따로 두는 방법도 있습니다. 연습할 때는 전자를 추천드립니다.


Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class DistanceExceedException extends ExpectedException {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package subway.application.core.exception;

public class DistanceNotPositiveException extends ExpectedException {

private static final String MESSAGE = "거리는 음수일 수 없습니다.";

public DistanceNotPositiveException() {
super(MESSAGE);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class ExpectedException extends RuntimeException {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class RouteNotConnectedException extends ExpectedException {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class SectionConnectException extends ExpectedException {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class StationAlreadyExistsException extends ExpectedException {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class StationConnectException extends ExpectedException {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class StationNotExistsException extends ExpectedException {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package subway.application.exception;
package subway.application.core.exception;

public class StationTooFarException extends ExpectedException {

Expand Down
Loading