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

[feat] 여행의 총 시간과 거리를 계산한다 (#289) #328

Open
wants to merge 12 commits into
base: develop-backend
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static dev.tripdraw.exception.member.MemberExceptionType.MEMBER_NOT_FOUND;
import static dev.tripdraw.exception.trip.TripExceptionType.TRIP_NOT_FOUND;

import dev.tripdraw.application.draw.RouteImageGenerator;
import dev.tripdraw.domain.member.Member;
import dev.tripdraw.domain.member.MemberRepository;
import dev.tripdraw.domain.trip.Point;
Expand All @@ -25,6 +26,11 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;

import static dev.tripdraw.exception.member.MemberExceptionType.MEMBER_NOT_FOUND;
import static dev.tripdraw.exception.trip.TripExceptionType.TRIP_NOT_FOUND;

Comment on lines +29 to +33
Copy link
Collaborator

Choose a reason for hiding this comment

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

제 눈에는 이미 위에 있는데, 왜 또 보이죠..? 그림자 분신술...

@RequiredArgsConstructor
@Transactional
@Service
Expand Down
17 changes: 17 additions & 0 deletions backend/src/main/java/dev/tripdraw/domain/trip/Route.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,20 @@
import jakarta.persistence.Embeddable;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.OneToMany;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.experimental.Accessors;

import static dev.tripdraw.exception.trip.TripExceptionType.POINT_NOT_FOUND;
import static dev.tripdraw.exception.trip.TripExceptionType.POINT_NOT_IN_TRIP;
import static jakarta.persistence.CascadeType.PERSIST;
import static jakarta.persistence.CascadeType.REMOVE;
import static jakarta.persistence.FetchType.LAZY;

@Accessors(fluent = true)
@Getter
@NoArgsConstructor(access = PROTECTED)
Expand Down Expand Up @@ -47,4 +54,14 @@ public void deletePointById(Long pointId) {

pointToDelete.delete();
}

public List<Point> points() {
return points.stream()
.filter(point -> !point.isDeleted())
.toList();
}

public RouteLength calculateRouteLength() {
return RouteLength.from(points);
}
}
49 changes: 49 additions & 0 deletions backend/src/main/java/dev/tripdraw/domain/trip/RouteLength.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package dev.tripdraw.domain.trip;

import dev.tripdraw.exception.trip.TripException;

import java.util.List;
import java.util.stream.IntStream;

import static dev.tripdraw.exception.trip.TripExceptionType.ONE_OR_NO_POINT;
import static java.lang.Math.*;
Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved

public class RouteLength {

public static final double AVERAGE_DISTANCE_FOR_ONE_DEGREE_DIFFERENCE_IN_LATITUDE_ON_KOREA_REGION = 111.1;
Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved

private final Double length;
Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved

private RouteLength(List<Point> points) {
this.length = calculateLength(points);
Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved
}

public static RouteLength from(List<Point> points) {
return new RouteLength(points);
}

private Double calculateLength(List<Point> points) {
if (points.isEmpty() || points.size() == 1) {
throw new TripException(ONE_OR_NO_POINT);
}
Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved

return IntStream.range(0, points.size() - 1)
.mapToDouble(i -> distanceBetween(points.get(i), points.get(i + 1)))
.sum();
}

private Double distanceBetween(Point startPoint, Point endPoint) {
double theta = startPoint.longitude() - endPoint.longitude();
Double latitude1 = startPoint.latitude();
Double latitude2 = endPoint.latitude();

double unitDistance = sin(toRadians(latitude1)) * sin(toRadians(latitude2))
+ cos(toRadians(latitude1)) * cos(toRadians(latitude2)) * cos(toRadians(theta));
Comment on lines +37 to +42
Copy link
Member

Choose a reason for hiding this comment

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

다음과 같이 메서드를 분리해보면 어떨까요?

private double calculateUnitDistance(Point startPoint, Point endPoint) {
    double start = startPoint.latitude();
    double end = endPoint.latitude();
    double theta = start - end;
    return sin(toRadians(start)) * sin(toRadians(end)) + 
            cos(toRadians(start)) * cos(toRadians(end)) * cos(toRadians(theta));
}

Copy link
Member

Choose a reason for hiding this comment

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

추가로 double startRadians = toRadians(startPoint.latitude()); 와 같이 radians로 변수 추출 한다면 조금 더 최적화 할 수 있겠네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

theta는 경도를 사용해서 구하기 때문에 제시해주신 방법은 오류가 있네요!

저도 변수명을 많이 고민했는데,
오히려 이런 부분은 수학적인 도메인이기 때문에 startLatitude, endLatitude 보다 1,2를 뒤에 붙이는 네이밍을 선택했습니다
이 부분은 어떻게 생각하실까요?

Copy link
Member

Choose a reason for hiding this comment

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

근거만 있다면 취사선택 하시면 될 것 같습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 startLatitude, endLatitude 를 더 선호합니다만, 리오의 선택에 맡기겠습니다!


return toDegrees(acos(unitDistance)) * AVERAGE_DISTANCE_FOR_ONE_DEGREE_DIFFERENCE_IN_LATITUDE_ON_KOREA_REGION;
}

public String lengthInKm() {
return String.format("%.2f" + "km", length);
Copy link
Member

Choose a reason for hiding this comment

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

km도 view의 영역 아닐까 싶은데 어떻게 생각하시나요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

근데 이 부분 그러면 어떤 단위로 날아가는지는 api에 담는 것이 좋을까요 아니면 팀 내부 회의에서 정하는 것이 좋을까요?

Copy link
Member

Choose a reason for hiding this comment

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

단위는 변수명으로 정해도 되고, 팀 내부 회의에서 정해도 좋을 것 같습니다~

}
}
18 changes: 15 additions & 3 deletions backend/src/main/java/dev/tripdraw/domain/trip/Trip.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static dev.tripdraw.domain.trip.TripStatus.ONGOING;
import static dev.tripdraw.exception.trip.TripExceptionType.NOT_AUTHORIZED_TO_TRIP;
import static dev.tripdraw.exception.trip.TripExceptionType.ONE_OR_NO_POINT;
import static dev.tripdraw.exception.trip.TripExceptionType.TRIP_INVALID_STATUS;
import static jakarta.persistence.FetchType.LAZY;
import static jakarta.persistence.GenerationType.IDENTITY;
Expand All @@ -17,6 +18,7 @@
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import java.time.Duration;
import java.util.List;
import lombok.Getter;
import lombok.NoArgsConstructor;
Expand Down Expand Up @@ -80,6 +82,18 @@ public void validateAuthorization(Member member) {
}
}

public TripDuration calculateTripDuration() {
List<Point> points = points();
if (points.size() == 1 || points.isEmpty()) {
throw new TripException(ONE_OR_NO_POINT);
Copy link
Member

Choose a reason for hiding this comment

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

여기서도 사용해서 ONE_OR_NO_POINT로 정하신 거였군요!
저는 각각의 예외가 따로 분리되는 것도 좋다고 생각하지만 ONE_OR_NO_POINT으로 할 지 예외를 따로 따로 구분할지 취사 선택 해주시면 될 것 같습니다! 👍

}
Point startingPoint = points.get(0);
Point arrivalPoint = points.get(points.size() - 1);

Duration between = Duration.between(startingPoint.recordedAt(), arrivalPoint.recordedAt());
return TripDuration.of(between);
Copy link
Member

Choose a reason for hiding this comment

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

계산 로직이 Duration 내부로 들어가는건 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Duration이 java 내장 클래스입니다
TripDuration이 Point 2개를 받는 방향으로 리팩터링 할게요~

Copy link
Member

Choose a reason for hiding this comment

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

네네 RouteLength와 유사한 형태로 TripDuration 정적팩터리 메서드로 넣어보면 어떨까 하는 리뷰였습니다!

}

public void changeStatus(TripStatus status) {
validateStatus(status);
this.status = status;
Expand Down Expand Up @@ -138,9 +152,7 @@ public void deletePointById(Long pointId) {
}

public List<Point> points() {
return route.points().stream()
.filter(point -> !point.isDeleted())
.toList();
return route.points();
}

public String nameValue() {
Expand Down
41 changes: 41 additions & 0 deletions backend/src/main/java/dev/tripdraw/domain/trip/TripDuration.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package dev.tripdraw.domain.trip;

import java.time.Duration;

public class TripDuration {

private static final String MINUTE = "분";
private static final String HOUR = "시간";
private static final String DAY = "일";
private static final String WHITE_SPACE = " ";
Comment on lines +7 to +10
Copy link
Member

Choose a reason for hiding this comment

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

이 부분은 View의 영역이라고 생각하는데 어떻게 생각하시나요?


private final Duration duration;

private TripDuration(Duration duration) {
this.duration = duration;
}

public static TripDuration of(Duration duration) {
return new TripDuration(duration);
}

public String durationInMinutes() {
long minutes = duration.toMinutes();
return minutes + MINUTE;
}

public String durationInHoursAndMinutes() {
long hours = duration.toHours();
long minutes = duration.toMinutes() - (hours * 60);

return hours + HOUR + WHITE_SPACE + minutes + MINUTE;
}

public String durationInDaysAndHoursAndMinutes() {
long days = duration.toDays();
long hours = duration.toHours() - (days * 24);
long minutes = duration.toMinutes() - (days * 1440) - (hours * 60);

return days + DAY + WHITE_SPACE + hours + HOUR + WHITE_SPACE + minutes + MINUTE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public enum TripExceptionType implements ExceptionType {
TRIP_INVALID_STATUS(BAD_REQUEST, "잘못된 여행 상태입니다."),
POINT_ALREADY_HAS_POST(CONFLICT, "이미 감상이 등록된 위치입니다."),
TRIP_ALREADY_DELETED(CONFLICT, "이미 삭제된 여행입니다."),
ONE_OR_NO_POINT(BAD_REQUEST, ""),
;

private final HttpStatus httpStatus;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,10 @@ void setUp() {
tripService.deletePoint(loginUser, response.pointId(), trip.id());

// then
Point deletedPoint = trip.route().points()
assertThat(trip.route().points()
.stream()
.filter(point -> Objects.equals(point.id(), response.pointId()))
.findFirst()
.get();

assertThat(deletedPoint.isDeleted()).isTrue();
.anyMatch(point -> Objects.equals(point.id(), response.pointId())))
.isFalse();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package dev.tripdraw.domain.trip;

import dev.tripdraw.exception.trip.TripException;
import dev.tripdraw.exception.trip.TripExceptionType;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;

@SuppressWarnings("NonAsciiCharacters")
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class RouteLengthTest {

@Test
void 경로의_길이를_계산한다() {
// given
List<Point> points = List.of(new Point(1.1, 1.1, LocalDateTime.now()), new Point(2.1, 1.1, LocalDateTime.now()), new Point(2.1, 2.1, LocalDateTime.now()));

// when
RouteLength length = RouteLength.from(points);

// then
Assertions.assertThat(length.lengthInKm()).isEqualTo("222.13km");
Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved
}

@ParameterizedTest
@MethodSource("generateData")
void 경로에_위치정보가_하나이거나_없는_경우_예외를_발생시킨다(List<Point> points) {
// expect
Assertions.assertThatThrownBy(() -> RouteLength.from(points))
.isInstanceOf(TripException.class)
.hasMessage(TripExceptionType.ONE_OR_NO_POINT.message());
}

static Stream<List<Point>> generateData() {
return Stream.of(
new ArrayList<>(),
List.of(new Point(1.1, 1.1, LocalDateTime.now()))
);
}


Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved
}
67 changes: 60 additions & 7 deletions backend/src/test/java/dev/tripdraw/domain/trip/RouteTest.java
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
package dev.tripdraw.domain.trip;

import static dev.tripdraw.exception.trip.TripExceptionType.POINT_ALREADY_DELETED;
import static dev.tripdraw.exception.trip.TripExceptionType.POINT_NOT_FOUND;
import static dev.tripdraw.exception.trip.TripExceptionType.POINT_NOT_IN_TRIP;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import dev.tripdraw.exception.trip.TripException;
import java.time.LocalDateTime;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator.ReplaceUnderscores;
import org.junit.jupiter.api.Test;

import java.time.LocalDateTime;

import static dev.tripdraw.exception.trip.TripExceptionType.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

@SuppressWarnings("NonAsciiCharacters")
@DisplayNameGeneration(ReplaceUnderscores.class)
class RouteTest {
Expand Down Expand Up @@ -102,5 +101,59 @@ class RouteTest {
.isInstanceOf(TripException.class)
.hasMessage(POINT_NOT_FOUND.message());
}

@Test
void 삭제된_위치정보는_반환하지_않는다() {
// given
Route route = new Route();
Point point1 = new Point(1L, 1.1, 2.1, false, LocalDateTime.now());
Point point2 = new Point(2L, 3.1, 4.1, false, LocalDateTime.now());
route.add(point1);
route.add(point2);

// when
route.deletePointById(1L);

// then
assertThat(route.points()).containsExactly(point2);
}

@Test
void 경로의_거리를_계산한다() {
// given
Route route = new Route();
route.add(new Point(1.1, 1.1, LocalDateTime.now()));
route.add(new Point(1.1, 2.1, LocalDateTime.now()));
route.add(new Point(2.1, 2.1, LocalDateTime.now()));

// when
RouteLength routeLength = route.calculateRouteLength();

// then
assertThat(routeLength.lengthInKm()).isEqualTo("222.18km");
}

@Test
void 경로에_위치정보가_존재하지_않으면_거리를_계산할_때_예외를_발생시킨다() {
// given
Route route = new Route();

// expect
assertThatThrownBy(route::calculateRouteLength)
.isInstanceOf(TripException.class)
.hasMessage(ONE_OR_NO_POINT.message());
}

@Test
void 경로에_위치정보가_하나만_존재하면_거리를_계산할_때_예외를_발생시킨다() {
Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved
// given
Route route = new Route();
route.add(new Point(1.1, 1.1, LocalDateTime.now()));

// expect
assertThatThrownBy(route::calculateRouteLength)
.isInstanceOf(TripException.class)
.hasMessage(ONE_OR_NO_POINT.message());
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package dev.tripdraw.domain.trip;

import static org.assertj.core.api.Assertions.assertThat;

import java.time.Duration;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Test;

@SuppressWarnings("NonAsciiCharacters")
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class TripDurationTest {

private final TripDuration tripDuration = TripDuration.of(Duration.ofMinutes(3248));

@Test
void 여행기간을_분_단위로_나타낸다() {
// given & when
String durationInMinutes = tripDuration.durationInMinutes();

// then
assertThat(durationInMinutes).isEqualTo("3248분");
}

@Test
void 여행기간을_시간과_분_단위로_나타낸다() {
// given & when
String durationInHoursAndMinutes = tripDuration.durationInHoursAndMinutes();

// then
assertThat(durationInHoursAndMinutes).isEqualTo("54시간 8분");
}

@Test
void 여행기간을_일과_시간과_분_단위로_나타낸다() {
// given & when
String durationInDaysAndHoursAndMinutes = tripDuration.durationInDaysAndHoursAndMinutes();
// then
assertThat(durationInDaysAndHoursAndMinutes).isEqualTo("2일 6시간 8분");
Jaeyoung22 marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading