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/get-traffics (2) #17

Merged
merged 1 commit into from
May 28, 2024
Merged

Feat/get-traffics (2) #17

merged 1 commit into from
May 28, 2024

Conversation

nove1080
Copy link
Collaborator

controller 구현 후 푸쉬를 까먹어서 PR 합니다 ㅜㅜ

@GetMapping("/{trafficId}")
@Transactional
public ApiResponse<ApiResponse.SuccessBody<BrowseTrafficsResponse>> browseTraffic(
@PathVariable Long trafficId) {
// todo implement

Choose a reason for hiding this comment

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

주요 변경 사항 및 개선 제안:

  1. ViewPointParam 클래스의 도입이 충분히 유익하며, TrafficController 클래스에서 이를 적극 활용하고 있음.
  2. readTrafficService의 사용은 좋으나, TrafficDetailConverter에 대한 구현 누락으로 해당 매서드의 구체적인 동작 방식 명확하지 않음. 필요에 따라 추가 설명 필요.
  3. @Transactional 어노테이션을 통해 트랜잭션 관리를 위한 설정이 주석 처리되어 있는 것은 부적절.
  4. 주석에 있는 '호출 시 Authentication 획득'에 대한 Todo 작업 진행 필요.
  5. 메서드 생성 및 수정 시의 주석(todo)을 올바르게 처리하고, 작업 완료 시 반영 확인 필요.
  6. 예외 처리에 대한 로직이 누락됨 - 입력 값 유효성 검증 및 예상치 못한 상황 대비하여 적절한 예외 처리 고려 요망.

버그 위험:

  1. PatchFavoriteTrafficNameBodyFavoriteTrafficBody와 같은 Request DTO 클래스의 사용 여부에 대한 확인 필요.
  2. trafficId에 대한 browseTraffic 메서드 내부 로직 구현 필요.
  3. 미완료된 '최종적으로 교통 정보 조회 구현'과 같은 주석 내 임시 작업 지정에 대한 완료 여부 검토 필요.

이외에는 코드 스타일, 네이밍 컨벤션, 리팩토링 등의 좀 더 세부적인 사항은 보다 심층적인 리뷰가 필요할 수 있습니다.

@nove1080 nove1080 merged commit 8669972 into main May 28, 2024
2 checks passed
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.

1 participant