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

2단계 - 인수 테스트 리팩토링 #207

Merged
merged 7 commits into from
Jan 24, 2022

Conversation

loop-study
Copy link

안녕하세요 박현철입니다.
이번에 진행하면서 여러 고민사항이 있습니다.

  1. @ExceptionHandler를 찾아서 사용해봤는데 알맞게 사용했는지 모르겠습니다. ErrorResponse를 반환하는데 여기에 에러문구말고 또 무엇을 담아야할까요?

  2. @RestControllerAdvice 경우에 exception 패키지에 위치시켰는데 ErrorResponse DTO도 해당 패키지에 위치하는게 맞는지 궁금합니다.

그 외에도 의견부탁드립니다. 🙇🏻‍♀️

2022. 01. 23.
step1 리팩토링
- 중복 제거
- 의미없이 노출되는 public 메서드 -> private 로 숨김
- 수정 후 반환하는 로직 -> void 로 변경
- 기타 등등

Resolves : loop-study
See also : parkeeseul
2022. 01. 23.
- 지하철역 관리 인수 조건 추가
- 노선 관리 인수 조건 추가

Resolves : loop-study
See also : parkeeseul
2022. 01. 23.
- LineSteps로 중복 분리

Resolves : loop-study
See also : parkeeseul
2022. 01. 23.
- 지하철 노선 이름 중복 로직 추가
- 지하철역 이름 중복 로직 추가
- 에러 공통처리를 위한 ExceptionAdvice 추가
- 관련 테스트 추가

Resolves : loop-study
See also : parkeeseul
2022. 01. 23.
- 각 테스트 클래스마다 중복되는 RestAssured를 CommonRestAssured로 분리하여 수정
- 메서드명 수정

Resolves : loop-study
See also : parkeeseul
2022. 01. 23.
- 불필요한 import 제거

Resolves : loop-study
See also : parkeeseul
2022. 01. 23.
- 테스트 패키지 분리
- 기타 등등

Resolves : loop-study
See also : parkeeseul
Copy link

@parkeeseul parkeeseul left a comment

Choose a reason for hiding this comment

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

2단계 미션 잘 진행해주셨네요! 👍
몇 가지 코멘트와 질문 주신 사항에 대한 저의 의견 남겨두었으니
확인 부탁드리고 추가적인 수정사항은 다음 단계와 함께 반영 부탁드립니다! 😄
즐거운 저녁 되세요! 🔥

Comment on lines +8 to +15
@RestControllerAdvice
public class ExceptionAdvice {

@ExceptionHandler(IllegalArgumentException.class)
public ResponseEntity<ErrorResponse> argumentException(IllegalArgumentException ex) {
return ResponseEntity.badRequest().body(new ErrorResponse(ex.getMessage()));
}
}

Choose a reason for hiding this comment

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

Q1. @ExceptionHandler를 찾아서 사용해봤는데 알맞게 사용했는지 모르겠습니다. ErrorResponse를 반환하는데 여기에 에러문구말고 또 무엇을 담아야할까요?

A1. 저는 실무에서 상황에 맞는 status 코드와 body 는 에러 코드 또는 메세지 정도만 제공하고 있는데요,
때문에 작업자들(프론트엔드 개발자)과 커뮤니케이션을 통해 해결하고 있습니다.
저는 개인적으로 간단하게 제공하고 있지만
아래와 같은 상세 정보들을 함께 제공하면 커뮤니케이션 비용이 줄어들 수도 있겠다는 생각도 드네요 😄

{
    "timestamp": "2019-01-17T16:12:45.977+0000",
    "status": 500,
    "error": "Internal Server Error",
    "message": "Error processing the request!",
    "path": "/my-endpoint-with-exceptions"
}

Q2. @RestControllerAdvice 경우에 exception 패키지에 위치시켰는데 ErrorResponse DTO도 해당 패키지에 위치하는게 맞는지 궁금합니다.

A2. 정답이 있는 문제라기 보다는 회사의 컨벤션이나 설계에 따라 다를 수 있을 것 같은데요,
개인적으로는 exception 패키지를 분리하는 것을 선호합니다.
지금의 작성해주신 구조도 좋을 것 같아요! 👍

@parkeeseul parkeeseul merged commit ccf8b13 into next-step:loop-study Jan 24, 2022
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.

None yet

2 participants