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] naver maps api fix #11

Merged
merged 7 commits into from
Sep 9, 2023
Merged

[FEAT] naver maps api fix #11

merged 7 commits into from
Sep 9, 2023

Conversation

jinjoo-lab
Copy link
Collaborator

@jinjoo-lab jinjoo-lab commented Aug 31, 2023

추가/수정한 기능 설명

  1. Config , Object Mapper Kakao , Naver 별로 분리 -> 의존성 주입 확인
  2. 경로 ResultDto 생성 및 구현 완료 -> 테스트 완료

특이사항

check list

  • issue number를 브랜치 앞에 추가 했나요?
  • 모든 단위 테스트를 돌려보고 기존에 작동하던 테스트에 영향이 없는 것을 확인했나요?
  • 추가/수정사항을 설명했나요?

Copy link
Member

@ohksj77 ohksj77 left a comment

Choose a reason for hiding this comment

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

Good!


@Primary
@Bean
public ObjectMapper KakaoObjectMapper() {
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 Author

Choose a reason for hiding this comment

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

수정 완료

@Bean
public ObjectMapper objectMapper() {
public ObjectMapper NaverObjectMapper() {
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 Author

Choose a reason for hiding this comment

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

수정 완료


/*response 변경*/
@PostMapping("/search")
SearchPathResponse searchPath(@RequestBody SearchPathRequest request) {
Copy link
Member

Choose a reason for hiding this comment

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

ResponseEntity랑 접근지정자 써주면 될듯!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아하 이걸 수정 안했네

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정 완료

@AllArgsConstructor
public class RouteUnitEnt {
Summary summary;
List<Double[]> path;
Copy link
Member

Choose a reason for hiding this comment

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

dto들에 전반적으로 private이 없는 부분들 확인해줘!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

private final String toSmall;

public String toSmallFuel() {
return this.toSmall;
Copy link
Member

Choose a reason for hiding this comment

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

이거 근데 this.toString().toLowerCase(); 하면 toSmall 없어도 되는거 아닌가? 아래 enum에도 그렇구
다른 뜻이 있었으면 알려조

TRAAVOIDCARONLY("자동차 전용도로 회피 우선", "traavoidcaronly");

private final String toKorean;
private final String toSmall;
Copy link
Member

Choose a reason for hiding this comment

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

요거 말한거야

import org.springframework.stereotype.Service;

@Service
public class PathService {
private final PathClient<SearchPathRequest, SearchPathResponse> client;

PathService(PathClient<SearchPathRequest, SearchPathResponse> client) {
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 Author

Choose a reason for hiding this comment

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

수정 완료

@ohksj77 ohksj77 changed the title Feat/naver maps api fix [FEAT] naver maps api fix Aug 31, 2023
@jinjoo-lab jinjoo-lab merged commit 61ca56a into master Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants