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

[Step3] 즐겨찾기 기능 구현 리뷰 요청드립니다. #238

Open
wants to merge 6 commits into
base: misudev
Choose a base branch
from

Conversation

misudev
Copy link

@misudev misudev commented Feb 22, 2022

안녕하세요 이슬님~
마지막 단계 리뷰 부탁드립니다! 🙇🏼‍♀️

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.

3단계 미션 잘 진행해 주셨네요! 👍
몇 가지 코멘트 남겨두었으니 확인 부탁드립니다 😄
마지막까지 화이팅 이에요! 🔥

Comment on lines +63 to +98
@DisplayName("즐겨찾기 추가 하기")
@Test
public void addFavorite() {
// when
ExtractableResponse<Response> response = 즐겨찾기_생성_요청(로그인토큰, 강남역, 양재역);

// then
즐겨찾기_생성됨(response);
}

@DisplayName("즐겨찾기 조회 하기")
@Test
public void getFavorites() {
// given
즐겨찾기_생성_요청(로그인토큰, 강남역, 양재역);
즐겨찾기_생성_요청(로그인토큰, 양재역, 교대역);

// when
ExtractableResponse<Response> response = 즐겨찾기_목록_조회_요청(로그인토큰);

// then
즐겨찾기_정보_조회됨(response, new Long[]{강남역, 양재역}, new Long[]{양재역, 교대역});
}

@DisplayName("즐겨찾기 삭제 하기")
@Test
public void deleteFavorites() {
// given
ExtractableResponse<Response> createResponse = 즐겨찾기_생성_요청(로그인토큰, 강남역, 양재역);

// when
ExtractableResponse<Response> response = 즐겨찾기_삭제_요청(로그인토큰, createResponse);

// then
즐겨찾기_삭제됨(response);
}

Choose a reason for hiding this comment

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

즐겨찾기 목록을 관리한다 에서 시나리오 형식으로 잘 작성해주셔서
해당 테스트들은 작성해주지 않아도 괜찮을 것 같아요 😄

public class FavoriteService {
private final StationService stationService;
private final PathService pathService;
private final MemberRepository memberRepository;

Choose a reason for hiding this comment

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

memberRepository 를 사용하고 있지 않는데요,
즐겨찾기 생성, 조회, 삭제 시 유요한 memberId 인지 확인하지 않아도 될까요? 😄


return PathResponse.of(path);
}

Path findPath(Station upStation, Station downStation) {

Choose a reason for hiding this comment

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

접근제어자를 생략하신 이유가 있으신가요? 😄


public class FavoriteResponse {
private Long id;
private Long memberId;

Choose a reason for hiding this comment

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

요구사항에 memberId 는 없네요!
요구사항을 확인해주세요 😄

}

@PostMapping
public ResponseEntity<FavoriteResponse> createFavorite(

Choose a reason for hiding this comment

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

즐겨찾기 생성 후 반환값에 대한 요구사항을 확인해주세요! 😄
스크린샷 2022-02-22 오후 7 58 34

Comment on lines +26 to +30
Optional.ofNullable(result)
.orElseThrow(() -> {
throw new IllegalArgumentException("출발역과 도착역이 연결되어 있지 않습니다.");
});

Choose a reason for hiding this comment

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

Optional 을 사용하신 이유가 있으신가요?
어떠한 이점이 있나요? 😄

Suggested change
Optional.ofNullable(result)
.orElseThrow(() -> {
throw new IllegalArgumentException("출발역과 도착역이 연결되어 있지 않습니다.");
});
if (Objects.isNull(result)) {
throw new IllegalArgumentException("출발역과 도착역이 연결되어 있지 않습니다.");
}

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