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

Merged
merged 5 commits into from
Sep 20, 2024

Conversation

dani820
Copy link
Collaborator

@dani820 dani820 commented Sep 8, 2024

이 PR을 통해 해결하려는 문제

  • 독서록 단 건 조회를 위한 조회 기능 컨트롤러 로직 작성

추가 및 변경사항

  • 독서록 조회 기능 컨트롤러 로직 작성
  • 테스트 코드 작성

참고(옵션)

  • 테이블 설계 변경에 따른 Member 클래스 필드 정리 필요

체크리스트

  • 리뷰어를 지정하였는가
  • 코드가 오류나 경고없이 빌드되는가
  • 추가 및 변경된 사항에 대해 충분히 테스트 하였는가

컨트롤러 로직 작성 및 테스트 코드 작성
@dani820 dani820 self-assigned this Sep 8, 2024
public record ContentResponse(
String title,
String content,
String fromDate,
Copy link
Collaborator

Choose a reason for hiding this comment

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

fromDate, toDate 가 무슨 뜻인가요???

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

독서 기간에 대한 변수로 각각 시작일, 종료일을 뜻하는데요. startDate, endDate로 변경하는 쪽이 더 의미가 맞다고 생각되어서 수정하려 합니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

변수명이 명확하지 않아요.

import com.inmybook.application.port.in.RegisterPostCommand;
import com.inmybook.application.service.PostDetailsOutput;

@Component
public class PostMapper {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mapper 는 왜 component 로 주입 하나요???

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

작성 당시 단순히 빈 등록을 위해 @component를 사용했던 것 같습니다; 주입 방식을 달리 하는게 좋을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

static 함수로 만드는 방법도 고려해 보시면 좋을것 같아요.

@@ -31,7 +31,7 @@ public class PostController {

private final RegisterPostUseCase registerPostUseCase;
private final ReadPostUseCase readPostUseCase;
private final PostMapper postMapper;
private static final PostMapper postMapper = new PostMapper();
Copy link
Collaborator

Choose a reason for hiding this comment

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

객체 자체를 스태틱으로 선언 하는것 외에도 PostMapper 에서 제공하는 함수를 static 함수로 만드는걸 고려해 봐도 좋을 것 같아요.
객체로 따로 만들 필요가 없어 보이는 클래스처럼 보이긴 해서요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

static 함수로 변경했습니다~

- PostMapper 클래스 내 함수들을 static으로 변경하여 불필요한 객체 생성 없이 간단히 접근 가능하도록 수정
- 조회 기능 Controller 테스트 코드 내에서 PostMapper 함수 모킹한 부분 수정(mockStatic 사용)
Copy link
Collaborator

@f-lab-bradley f-lab-bradley left a comment

Choose a reason for hiding this comment

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

구현 완료 된건가요??

@dani820
Copy link
Collaborator Author

dani820 commented Sep 19, 2024

넵 피드백 주신 부분 반영 완료했습니다

Copy link
Collaborator

@f-lab-bradley f-lab-bradley left a comment

Choose a reason for hiding this comment

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

수고 하셨습니당

@dani820 dani820 merged commit f15d005 into develop Sep 20, 2024
1 check passed
@dani820 dani820 deleted the feature/17 branch September 20, 2024 13:03
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