[feat] 이미 등록된 식당인지 판별하는 API #72
Merged
Conversation
This file contains 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
|
|
||
| public record StoreDuplicateValidationRequest( | ||
| Long id, | ||
| Double latitude, |
There was a problem hiding this comment.
Wrapper class 보단 null을 허용하지 않는 double을 사용하는게 어떨까요? 그리고 현재 Point.java의 자료형도 double이라 통일성을 위해 double을 사용하는게 더 적절할 것 같습니다.
| @Transactional(readOnly = true) | ||
| public StoreDuplicateValidationResponse validateDuplicatedStore(final StoreDuplicateValidationRequest request) { | ||
| Long storeId = universityStoreFinder.findUniversityStoreWithLatitudeAndLongitude(request.id(), request.latitude(), request.longitude()) | ||
| .map(UniversityStore->UniversityStore.getStore().getId()) |
There was a problem hiding this comment.
람다의 매개변수는 카멜케이스로 네이밍 하는게 좋지 않을까요? 클래스나 레코드 타입과 혼동될 수 있을 것 같습니다.
|
|
||
| public record UserLoginRequest( | ||
| @Nullable | ||
| String name, | ||
| @NotNull |
There was a problem hiding this comment.
현재 받아야하는 문자열이 KAKAO 혹은 APPLE이니 null과 더불어 빈 문자열을 검색하는 @notblank로 하는게 어떨까요?
|
|
||
| private final UniversityStoreRepository universityStoreRepository; | ||
|
|
||
| public Optional<UniversityStore> findUniversityStoreWithLatitudeAndLongitude(Long id, Double latitude, Double longitude) { |
There was a problem hiding this comment.
이 부분도 Point와 통일하여 double로 바꾸는게 통일성을 위해 적절할 것 같습니다.
| @@ -6,9 +6,16 @@ | |||
| @Getter | |||
| public class ConflictException extends RuntimeException { | |||
There was a problem hiding this comment.
HankkiResponse의 코드를 참고해서 제네릭 타입으로 바꾸는게 어떨까요?
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.
Related Issue 📌
close #70
Description ✔️
To Reviewers
일단 문자열인 도로명 주소나 가게 이름을 비교하는 것은 좋지 않다고 생각하였고, 위도와 경도 조합으로도 고유한 가게가 식별되는지 찾아보았습니다.
https://navermaps.github.io/maps.js.ncp/docs/tutorial-Projection.html
저희가 쓰고 있는 네이버 지도 api 문서를 확인해보았는데, '실세계의 고유한 지점을 가리키는 좌표입니다. 일반적으로 위도와 경도로 표시하는 WGS84 표준을 사용합니다.' 라고 적혀있는 것을 보아 위도와 경도 조합으로 검증하는 것이 괜찮다고 생각했습니다.
응답 결과
식당이 이미 등록되어 있을 경우
식당이 등록되어 있지 않을 경우