Open
Conversation
moongua404
reviewed
Aug 17, 2025
| import mission.application.port.input.Input; | ||
|
|
||
| public class LogisticsInput implements Input { | ||
| public String region(){ |
Contributor
There was a problem hiding this comment.
개인적으로 함수 네이밍이 좀 더 읽기 편하려면 동사로 짓는게 함수는 행동을 표현하기 때문에 더 적절해 보여요.
Author
There was a problem hiding this comment.
아 void로 선언할 때만 동사로 지어야하는 줄 알았네요. 수정해보겠습니다.
| } | ||
| return places; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
AddressLoader, RouteLoader랑 코드가 반복되는 부분이 많은 것 같아요. 공통되는 부분은 분리해도 좋을 것 같아요
| private static final double EARTH_RADIUS_KM = 6371.0; | ||
|
|
||
| private List<Place> places = PlaceLoader.loadFromCsv("src/main/resources/place.csv"); | ||
| private List<Address> addresses = AddressLoader.loadFromCsv("src/main/resources/position.csv"); |
Contributor
There was a problem hiding this comment.
일반적으로 service는 상태를 갖지 않습니다.
특히 외부에 대한 의존성이 없어야 하는 헥사고날 패턴 상의 서비스에서 외부 기술에 종속성을 만드는건 부적절한 것 같아요
| String arrivalInput = input.region(); | ||
|
|
||
| Address departure = departureAddress(departureInput); | ||
| Address arrival = arrivalAddress(arrivalInput); |
Contributor
There was a problem hiding this comment.
검증 로직을 뒤로 빼면 사용자의 입장에서 문제를 파악하기 어려울 것 같아요.
ex)
출발지를 입력해주세요.
> 잘못된 장소
도착지를 입력해주세요.
> 숭실대학교 정보과학관
위치 정보를 찾을 수 없습니다.위 경우에는 처음 입력한 잘못된 장소가 문제인지 직관적으로 와닿지 않을 것 같아요
| int hourPart = (int) time; | ||
| int minutePart = (int) Math.round((time - hourPart) * 60); | ||
| return LocalTime.of(hourPart % 24, minutePart); | ||
| } |
Contributor
There was a problem hiding this comment.
메서드를 대부분 public으로 선언한 이유가 있나요?
Author
There was a problem hiding this comment.
저도 굳이 public으로 선언할 필요가 없었던 것 같아서 이번 미션 수행할 때 수정해보려했습니다.
kokkumong
added a commit
to kokkumong/mission-logistics
that referenced
this pull request
Aug 18, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allrounder-Study-Mission-PR
[운송 I (배송)] 미션 구현 (정규호)
💡 Summary
구현한 기능
✅ CheckList
...
신경써서 구현한 점 / 어려웠던 점
📚 Retrospective (YWT)
What I did
What I Learned
What I will do next
✨ Impression
이번 미션은 헥사고날 아키텍쳐 사용 연습에 목표를 두고, 저번 미션에서 "의존성 분리"에 대한 피드백을 반영해보려고 노력했다. 인터페이스 사용법에 익숙해질 수 있어서 좋았고, loader 구현과 사용과정에서 stream().filter()문법 사용이 좀 더 익숙해져서 좋았다. 아쉬운 점은 예외처리에 큰 신경을 못쓴 점과 dto의 활용을 하지 못한 점, 단위테스트를 사용하지 못한 점이다.