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 #21] : 답변 등록/조회 API #22

Merged
merged 29 commits into from
Aug 7, 2024
Merged

Conversation

hyun2371
Copy link
Member

@hyun2371 hyun2371 commented Aug 7, 2024

관련 이슈

💫 작업 요약

  • 답변 등록, 조회 API 개발/테스트
  • 답변 등록 시 크레딧 검증 로직 개발/테스트

🔍 중점적으로 리뷰 할 부분

  • 테스트 코드 가독성
  • questionPostFixture, AnswerFixture 내부에서 MemberFixture를 넣었는데, 테스트 내에서 명시적으로 파라미터로 넣을지 고민이 됩니다
  • AnswerControllerTest 내에서 QuestionPost 생성하는 부분을 beforeEach를 통해 공통적으로 추출할까 했는데, 가독성을 위해 각각 작성했습니다.

hyun2371 added 29 commits August 6, 2024 13:26
@hyun2371 hyun2371 added the ✨ feat 기능 추가 label Aug 7, 2024
@hyun2371 hyun2371 requested a review from dudxo August 7, 2024 06:18
@hyun2371 hyun2371 self-assigned this Aug 7, 2024
@hyun2371 hyun2371 linked an issue Aug 7, 2024 that may be closed by this pull request
4 tasks
Copy link

github-actions bot commented Aug 7, 2024

Test Results

24 tests   10 ✅  2s ⏱️
 9 suites  14 💤
 9 files     0 ❌

Results for commit 6ed1a9b.

@hyun2371 hyun2371 merged commit 5c096d6 into dev Aug 7, 2024
3 checks passed
@hyun2371 hyun2371 deleted the feat/#21/answer-post-and-get branch August 7, 2024 11:47
@dudxo
Copy link
Collaborator

dudxo commented Aug 7, 2024

  • 테스트 코드 가독성

    Assertions.assertThat()을 static import하는게 더 깔끔해보입니다!

  • questionPostFixture, AnswerFixture 내부에서 MemberFixture를 넣었는데, 테스트 내에서 명시적으로 파라미터로 넣을지 고민이 됩니다

    명시적으로 파라미터를 넣지 않으면 Member가 어디서 주입되는지 찾기 힘들어져 가독성이 안좋은 코드라고 느껴집니다. 파라미터로 Member를 주입해주는게 좋겠어요!

  • AnswerControllerTest 내에서 QuestionPost 생성하는 부분을 beforeEach를 통해 공통적으로 추출할까 했는데, 가독성을 위해 각각 작성했습니다.

    가현님도 아시겠지만, beforeEach를 통해 공통적으로 추출하면 또 다른 테스트를 추가했을 때 영향을 받을 수 있다보니 가독성뿐만 아니라 추후 문제가 생기는걸 방지하기 위해 각각 작성한 방식이 너무 좋았다고 생각됩니다ㅎㅎ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feat 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ 답변 등록/조회 API
2 participants