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 ~ 4 단계 방탈출 예약 결제 / 배포] 에버(손채영) 미션 제출합니다. #123

Merged
merged 15 commits into from
Jun 14, 2024

Conversation

helenason
Copy link
Member

@helenason helenason commented Jun 9, 2024

안녕하세요 스티치!

조금 늦게 리뷰 요청 드립니다. 벌써 마지막 미션이라니 믿기지 않아요 🥹

속도가 조금 늦어진 것 같아 스티치의 리뷰를 우선적으로 받고 싶어 제 코드에 완전히 만족하지 못함에도 리뷰 먼저 요청 드립니다.

테스트 계정

유저

ID: user1@user.com
PW: 1234

어드민

ID: admin@admin.com
PW: 1234

배포

주소

문서화

문서

ERD

Untitled

이번 리뷰도 잘 부탁드립니다 : )

Copy link

@lxxjn0 lxxjn0 left a comment

Choose a reason for hiding this comment

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

애버, 미션 구현하시느라 수고하셨습니다!

이번에는 코드에서는 따로 코멘트를 남길 부분이 많이 없었던 것 같습니다. 그 외로 추가로 말씀드릴 부분들이 있을 것 같은데 ERD에서 각 테이블의 연관관계를 각각의 엣지가 아니라 묶어서 작성되어있다보니 올바른 관계 표현이 안되는 것 같습니다. 구분해서 그려주시면 더 좋을 것 같아요.

그리고 연동규격서를 잘 작성하였는데 각 필드에 대한 설명이 존재하지 않습니다. 해당 연동규격서만을 보고 API를 사용할 수 있을까요? 토스페이먼츠의 API를 연동할 때 제공받았던 연동규격서를 한번 참고해보시고 어떤 부분이 부족한지 한번 확인해보시고 다시 수정해보시면 좋을 것 같아요!

src/docs/asciidoc/index.adoc Outdated Show resolved Hide resolved
Comment on lines 40 to 55
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
PaymentInfo that = (PaymentInfo) o;
return Objects.equals(paymentKey, that.paymentKey) && Objects.equals(orderId, that.orderId);
}

@Override
public int hashCode() {
return Objects.hash(paymentKey, orderId);
}
Copy link

Choose a reason for hiding this comment

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

동등성을 부여한 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

단순히 paymentKey와 orderId가 같을 경우 같은 PaymentInfo 객체라고 생각하기 때문에 동등성을 부여해주었습니다!
하지만 현재 해당 성질이 사용되는 부분은 없는 것 같아요.

+) paymentKey만으로도 동등함을 비교할 수 있기 때문에 코드를 수정해주었습니다.

src/main/java/roomescape/service/ReservationService.java Outdated Show resolved Hide resolved
@lxxjn0
Copy link

lxxjn0 commented Jun 10, 2024

추가로 제공된 서버에서 예약 페이지 등 기존에 제공되고 있던 페이지들에 접근이 안되는 경우가 발생하고 있었습니다. 해당 부분들도 올바르게 동작할 수 있도록 확인해주시면 좋을 것 같습니다 :)

@helenason
Copy link
Member Author

안녕하세요 스티치!

리뷰 반영하여 다시 요청 드립니다.

추가로 제공된 서버에서 예약 페이지 등 기존에 제공되고 있던 페이지들에 접근이 안되는 경우가 발생하고 있었습니다. 해당 부분들도 올바르게 동작할 수 있도록 확인해주시면 좋을 것 같습니다 :)

해당 부분을 해결하는 과정에서 커밋들이 많이 발생했습니다.
코드 수정 후 빌드하지 않은 채로 서버를 돌리면서 왜 안 되지.. 이러고 있었더라구요 🥲

더 코멘트 주실 부분 있으면 편하게 추가해주세요! 감사합니다 🙇‍♀️

Copy link

@lxxjn0 lxxjn0 left a comment

Choose a reason for hiding this comment

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

에버, 미션 진행하시느라 수고하셨습니다 👍🏼

문서도 훨씬 더 상세해졌고 코드는 따로 수정할만한 부분이 더는 보이지 않아서 미션은 여기서 마무리해도 될 것 같습니다. 추가로 더 궁금한 점이 있거나 하시면 언제든지 질문주셔도 됩니다. 레벨 2 진행하시느라 수고 많으셨어요 :)

@lxxjn0 lxxjn0 merged commit a5a1d8c into woowacourse:helenason Jun 14, 2024
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