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

다이어리 조회 #56

Closed
wants to merge 8 commits into from
Closed

다이어리 조회 #56

wants to merge 8 commits into from

Conversation

koo995
Copy link
Collaborator

@koo995 koo995 commented Sep 15, 2024

  • 조회용 커스텀리포지토리를 분리하고 JdbcTemplate 을 이용해서 작성했습니다.
  • DiaryRecord 엔티티 안에 있는 product_id을 이용해서 제품의 이름과 제조사명을 가져와야하기에 join 을 이용한 별도의 쿼리로 작성했습니다.

close #34

@koo995 koo995 added the dev label Sep 15, 2024
@koo995 koo995 self-assigned this Sep 15, 2024
Comment on lines +25 to +29
public class DiaryRetrievalQueryDtoExtractor implements ResultSetExtractor<DiaryRetrievalQueryDto> {
private final ObjectMapper objectMapper;

@Override
public DiaryRetrievalQueryDto extractData(ResultSet rs) throws SQLException, DataAccessException {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 클래스는 조회용 dto 를 매핑하기 위함이라.. 변경을 고려하기는 어려울 것 같아서 절차지향적으로 작성했는데 괜찮나요??

Copy link
Collaborator

Choose a reason for hiding this comment

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

이걸 한번에 다 조회할 필요가 있는건가요?
조회 페이지상 아침,점심,저녁 다 따로 클릭해서 조회하는 형태던데 왜 한번에 다 조회하는건가요?

Copy link
Collaborator Author

@koo995 koo995 Sep 22, 2024

Choose a reason for hiding this comment

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

다이어리 조회 페이지상 아침, 점심, 저녁, 간식 모두 한번에 보여주기 위해서 한번에 조회를 하였습니다. 요구사항 이미지에서는 아침 식사만 나타나있어서 오해의 소지가 있었네요!

private final DiaryCrudRepository diaryCrudRepository;
private final DiaryRetrievalCustomRepository diaryRetrievalCustomRepository;
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 인터페이스를 추상화한 이유가 뭔가요?

Copy link
Collaborator Author

@koo995 koo995 Sep 22, 2024

Choose a reason for hiding this comment

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

조회로직이 jdbc 템플릿을 이용하는 리포지토리에 의존하고 있어서 인터페이스로 추상화를 하였습니다. 그리고 의존성 방향을 고려해서 query 패키지가 상위에 있는 repository 패키지를 의존하도록 했습니다. 혹시 인터페이스를 이용하기보단 그냥 클래스로 조합하는 것이 더 나았을까요??

@@ -10,8 +11,8 @@
@Repository
@RequiredArgsConstructor
public class DiaryRepositoryImpl implements DiaryRepository{
Copy link
Collaborator

Choose a reason for hiding this comment

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

조회용 repository를 따로 만들어 보는건 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DiaryRetrievalCustomRepositoryImpl 클래스가 조회용 리포지토리가 아닌가요..??

Comment on lines +25 to +29
public class DiaryRetrievalQueryDtoExtractor implements ResultSetExtractor<DiaryRetrievalQueryDto> {
private final ObjectMapper objectMapper;

@Override
public DiaryRetrievalQueryDto extractData(ResultSet rs) throws SQLException, DataAccessException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이걸 한번에 다 조회할 필요가 있는건가요?
조회 페이지상 아침,점심,저녁 다 따로 클릭해서 조회하는 형태던데 왜 한번에 다 조회하는건가요?

@@ -29,4 +31,10 @@ public ApiResponse<DiarySavedResponse> addDiaryRecord(@Valid @RequestBody AddDia
@PathVariable Long diaryId) {
return ApiResponse.success(addDiaryRecordService.addDiaryRecord(addDiaryRecordRequest, diaryId));
}

@GetMapping("/diary/{memberId}/{diaryDate}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

rest api 규칙에 어울리지 않는 방식이네요
diary 뒤에는 해당 리소스의 id가 오는게 일반적입니다.
https://learn.microsoft.com/ko-kr/azure/architecture/best-practices/api-design
요 문서보고 다시 디자인해보세요!

@koo995 koo995 deleted the branch main2 September 22, 2024 16:40
@koo995 koo995 closed this Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

다이어리 조회
2 participants