[BE] 가까운 지하철 역을 2개 보여준다. #575

tsulocalize · 2024-09-10T02:45:35Z

❗ Issue

[BE] 가까운 지하철 역을 2개 보여준다. #574

✨ 구현한 기능

지하철 역을 2개 보여주는 기능
환승역을 하나로 보여주는 기능

📢 논의하고 싶은 내용

🎸 기타

tkdgur0906

고생많으셨습니다!
리뷰 한번 확인해주세요~

tkdgur0906 · 2024-09-17T16:02:46Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/service/SubwayStationService.java

-    private static final double AVERAGE_WALKING_SPEED = 1.3 * 60; // meter per minute
+    // meter per second * minute unit * decreasing speed on open street
+    private static final double AVERAGE_WALKING_SPEED = 1.3 * 60 * 0.4;
+    private static final int MAX_NESTING_STATION_NUMBER = 4;


중첩된 지하철역 수가 4인 이유가 있나요?

대한민국 내 환승역 최대 중첩수가 4 입니다!

tkdgur0906 · 2024-09-17T16:04:17Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/service/SubwayStationService.java

@@ -25,7 +28,17 @@ public SubwayStationResponse readNearestStation(double latitude, double longitud
                    double distance = Math.sqrt(dx * dx + dy * dy);
                    return SubwayStationResponse.of(station, (int) Math.round(distance / AVERAGE_WALKING_SPEED));


거리 계산 로직을 메서드 분리하면 좋을 것 같아요!

반영했습니다!

tkdgur0906 · 2024-09-17T16:05:01Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/service/SubwayStationService.java

    private static final List<SubwayStation> SUBWAY_STATIONS = SubwayReader.readSubwayStationData();

-    public SubwayStationResponse readNearestStation(double latitude, double longitude) {
+    public List<SubwayStationResponse> readNearestStation(double latitude, double longitude) {


스트림이 길어서 가독성이 좀 떨어진다고 생각하는데, 인지 단위로 한번씩 끊는 것은 어떤가요?

반영했습니다!

tkdgur0906 · 2024-09-17T16:08:02Z

backend/bang-ggood/src/test/java/com/bang_ggood/station/SubwayStationServiceTest.java

    @ParameterizedTest
    @MethodSource("provideStationData")
-    void readNearestStation(double latitude, double longitude, String nearestStationName) {
+    void readNearestStation(double latitude, double longitude, Station nearestStation, Station nextNearestStation) {


테스트 코드를 맨 위로 올리는 것은 어떤가요?

반영했습니다!

JINU-CHANG · 2024-09-19T01:56:44Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/dto/SubwayStationResponse.java

+
+    public SubwayStationResponse merge(SubwayStationResponse response) {
+        if (!stationName.equals(response.stationName)) {
+            throw new BangggoodException(ExceptionCode.STATION_NOT_SAME);


Suggested change

throw new BangggoodException(ExceptionCode.STATION_NOT_SAME);

throw new BangggoodException(ExceptionCode.STATION_NAME_NOT_SAME);

예외 메시지도 변경되면 좋겠어요!

반영했습니다!

JINU-CHANG · 2024-09-19T02:05:49Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/controller/SubwayStationController.java

+    public ResponseEntity<List<SubwayStationResponse>> readNearestStation(@RequestParam("latitude") double latitude,
+                                                                          @RequestParam("longitude") double longitude) {


Double로 바꿔야 될 것 같아요!

반영했습니다!

JINU-CHANG · 2024-09-19T02:09:15Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/dto/SubwayStationResponse.java


    public static SubwayStationResponse of(SubwayStation station, Integer walkingTime) {
-        return new SubwayStationResponse(station.getName(), station.getLine(), walkingTime);
+        List<String> stationLine = new ArrayList<>();
+        stationLine.add(station.getLine());


어차피 1개만 받는 것 같은데 왜 List를 선택하셨나요??

merge() 메서드를 통해 다른 Response와 합치기 위함입니다.

잠실역 2호선.merge(잠실역 8호선) = 잠실역 2호선,8호선

shin-jisong

간단한 리뷰만 남겼습니다! 수고하셨습니다~

shin-jisong · 2024-09-20T00:33:10Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/domain/SubwayStation.java

@@ -16,6 +18,13 @@ public SubwayStation(Integer id, String name, String line, double latitude, doub
        this.longitude = longitude;
    }

+    public double getDistance(double latitude, double longitude) {


getXX의 명명은 getter와 혼동되는 것 같은데 함수의 명명을 바꿔보는 것 어떨까요?

calculateDistance로 바꿨습니다!

tkdgur0906

고생하셨습니다~

JINU-CHANG · 2024-09-20T11:02:25Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/service/SubwayStationService.java

-                .min(Comparator.comparing(SubwayStationResponse::walkingTime))
-                .orElseThrow(() -> new BangggoodException(ExceptionCode.STATION_NOT_FOUND));
+                .sorted(Comparator.comparing(SubwayStationResponse::getWalkingTime))
+                .limit(MAX_NESTING_STATION_NUMBER * REQUESTED_STATION_NUMBER)


이 부분은 어떤 의미의 코드인가요??

JINU-CHANG · 2024-09-20T11:06:27Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/service/SubwayStationService.java

+                    double distance = station.calculateDistance(latitude, longitude);
                    return SubwayStationResponse.of(station, (int) Math.round(distance / AVERAGE_WALKING_SPEED));


SubwayStation 클래스에 walkingTime 계산을 요청할 수도 있지 않을까요?
뒤에 (int) Math.round(distance / AVERAGE_WALKING_SPEED)); 로직과 따로 있지 않아도 될 것 같아요

JINU-CHANG · 2024-09-20T11:07:04Z

backend/bang-ggood/src/main/java/com/bang_ggood/station/service/SubwayStationService.java

+                .collect(Collectors.groupingBy(
+                        SubwayStationResponse::getStationName,
+                        Collectors.reducing(SubwayStationResponse::merge)
+                ));


이 부분만 조금 더 개선하면 이해하기 쉬울 것 같아요!

shin-jisong

수고하셨습니다!

tsulocalize added 2 commits September 10, 2024 10:37

refactor: 도보 걷는 속도 조정

2746434

feat: 가까운 지하철 역을 2개까지 보여주는 기능 구현

1e66dcd

tsulocalize added 🔒 BE 😊 Feature labels Sep 10, 2024

tsulocalize requested review from tkdgur0906, shin-jisong and JINU-CHANG September 10, 2024 02:45

tsulocalize self-assigned this Sep 10, 2024

tkdgur0906 requested changes Sep 17, 2024

View reviewed changes

JINU-CHANG requested changes Sep 19, 2024

View reviewed changes

tsulocalize added 5 commits September 19, 2024 12:56

refactor: 인자 타입 변경

616f925

refactor: 커스텀 예외를 통한 예외 처리

f587db4

style: 테스트 메서드 위치 변경

b71bcd0

refactor: 지하철 역 거리 구하는 메서드 분리

9ceab17

refactor: stream 분리

34b5532

shin-jisong requested changes Sep 20, 2024

View reviewed changes

refactor: 메서드 명 명확하게 변경

fc33c05

tkdgur0906 approved these changes Sep 20, 2024

View reviewed changes

JINU-CHANG requested changes Sep 20, 2024

View reviewed changes

shin-jisong approved these changes Sep 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] 가까운 지하철 역을 2개 보여준다. #575

[BE] 가까운 지하철 역을 2개 보여준다. #575

tsulocalize commented Sep 10, 2024

tkdgur0906 left a comment

tkdgur0906 Sep 17, 2024

tsulocalize Sep 19, 2024

tkdgur0906 Sep 17, 2024

tsulocalize Sep 19, 2024

tkdgur0906 Sep 17, 2024

tsulocalize Sep 19, 2024

tkdgur0906 Sep 17, 2024

tsulocalize Sep 19, 2024

JINU-CHANG Sep 19, 2024

tsulocalize Sep 19, 2024

JINU-CHANG Sep 19, 2024

tsulocalize Sep 19, 2024

JINU-CHANG Sep 19, 2024

tsulocalize Sep 19, 2024

shin-jisong left a comment

shin-jisong Sep 20, 2024

tsulocalize Sep 20, 2024

tkdgur0906 left a comment

JINU-CHANG Sep 20, 2024

JINU-CHANG Sep 20, 2024

JINU-CHANG Sep 20, 2024

shin-jisong left a comment

		@@ -25,7 +28,17 @@ public SubwayStationResponse readNearestStation(double latitude, double longitud
		double distance = Math.sqrt(dx * dx + dy * dy);
		return SubwayStationResponse.of(station, (int) Math.round(distance / AVERAGE_WALKING_SPEED));

	throw new BangggoodException(ExceptionCode.STATION_NOT_SAME);
	throw new BangggoodException(ExceptionCode.STATION_NAME_NOT_SAME);

		public ResponseEntity<List<SubwayStationResponse>> readNearestStation(@RequestParam("latitude") double latitude,
		@RequestParam("longitude") double longitude) {

		double distance = station.calculateDistance(latitude, longitude);
		return SubwayStationResponse.of(station, (int) Math.round(distance / AVERAGE_WALKING_SPEED));

[BE] 가까운 지하철 역을 2개 보여준다. #575

Are you sure you want to change the base?

[BE] 가까운 지하철 역을 2개 보여준다. #575

Conversation

tsulocalize commented Sep 10, 2024

❗ Issue

✨ 구현한 기능

📢 논의하고 싶은 내용

🎸 기타

tkdgur0906 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shin-jisong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkdgur0906 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shin-jisong left a comment

Choose a reason for hiding this comment