Conversation
Summary of ChangesHello @ParkJiYeoung8297, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 풀 리퀘스트는 사용자가 찜한 브랜드와 캠페인 목록을 조회할 수 있는 기능을 추가하여 사용자 경험을 향상시키는 것을 목표로 합니다. 사용자는 매칭률, 인기순, 최신순 등 다양한 기준으로 찜 목록을 정렬하여 개인화된 정보를 얻을 수 있습니다. 이를 통해 사용자는 관심 있는 브랜드와 캠페인을 더욱 쉽게 탐색하고 관리할 수 있게 됩니다. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 사용자가 찜한 브랜드와 캠페인 목록을 조회하는 새로운 API를 추가하는군요. 전반적으로 N+1 문제를 피하기 위해 IN 절을 사용하고, QueryDSL을 활용하여 정렬 기능을 구현한 점이 좋습니다. 몇 가지 개선점을 제안합니다. DTO를 불변(immutable)하게 만들고, 컨트롤러에서 기본 파라미터 처리를 개선하며, QueryDSL 표현식을 더 타입-세이프하게 작성하는 것을 고려해볼 수 있습니다. 자세한 내용은 각 파일에 남긴 코멘트를 참고해주세요.
| brands.forEach(dto -> | ||
| dto.setTags(tagMap.getOrDefault(dto.getBrandId(), List.of())) | ||
| ); | ||
|
|
||
| return FavoriteBrandListResponseDto.builder() | ||
| .count(brands.size()) | ||
| .brands(brands) | ||
| .build(); |
There was a problem hiding this comment.
FavoriteBrandDto를 불변으로 변경하는 것과 연관하여, forEach를 사용해 DTO 리스트를 직접 수정하는 대신, stream().map()을 사용하여 태그 정보가 포함된 새로운 DTO 객체를 생성하는 것이 좋습니다. 이는 코드의 불변성을 유지하고 가독성을 높이는 데 도움이 됩니다.
| brands.forEach(dto -> | |
| dto.setTags(tagMap.getOrDefault(dto.getBrandId(), List.of())) | |
| ); | |
| return FavoriteBrandListResponseDto.builder() | |
| .count(brands.size()) | |
| .brands(brands) | |
| .build(); | |
| final List<FavoriteBrandDto> brandsWithTags = brands.stream() | |
| .map(dto -> new FavoriteBrandDto( | |
| dto.getBrandId(), | |
| dto.getBrandName(), | |
| dto.getBrandLogoUrl(), | |
| dto.getMatchingRatio(), | |
| dto.getLikeCount(), | |
| tagMap.getOrDefault(dto.getBrandId(), List.of()) | |
| )) | |
| .toList(); | |
| return FavoriteBrandListResponseDto.builder() | |
| .count(brandsWithTags.size()) | |
| .brands(brandsWithTags) | |
| .build(); |
References
- Avoid using @Setter on domain or DTO objects to prevent uncontrolled external modifications. If mutation is needed, provide specific methods for it.
| private Expression<Long> brandMatchingRatioExpression() { | ||
| return Expressions.numberTemplate( | ||
| Long.class, | ||
| "COALESCE({0}, 0)", | ||
| MATCH_BRAND_HISTORY.matchingRatio | ||
| ); | ||
| } |
There was a problem hiding this comment.
Expressions.numberTemplate를 사용하여 COALESCE 함수를 문자열로 직접 사용하고 있습니다. QueryDSL이 제공하는 타입-세이프한 coalesce() 메소드를 사용하면 코드의 가독성과 데이터베이스 이식성을 높일 수 있습니다.
private Expression<Long> brandMatchingRatioExpression() {
return MATCH_BRAND_HISTORY.matchingRatio.coalesce(0L);
}| private Expression<Long> campaignMatchingRatioExpression() { | ||
| return Expressions.numberTemplate( | ||
| Long.class, | ||
| "COALESCE({0}, 0)", | ||
| MATCH_CAMPAIGN_HISTORY.matchingRatio | ||
| ); | ||
| } |
There was a problem hiding this comment.
brandMatchingRatioExpression와 마찬가지로, COALESCE 함수를 문자열 템플릿 대신 타입-세이프한 coalesce() 메소드로 변경하는 것이 좋습니다.
private Expression<Long> campaignMatchingRatioExpression() {
return MATCH_CAMPAIGN_HISTORY.matchingRatio.coalesce(0L);
}| @RequestParam(required = false) BrandSortType sort | ||
| ) { | ||
| Long userId = userDetails.getUserId(); | ||
| return CustomResponse.ok( | ||
| userFavoriteService.getMyFavoriteBrands( | ||
| userId, | ||
| sort == null ? BrandSortType.MATCH_SCORE : sort | ||
| ) |
There was a problem hiding this comment.
sort 파라미터의 기본값을 삼항 연산자로 처리하고 있습니다. @RequestParam의 defaultValue 속성을 사용하면 코드를 더 간결하게 만들 수 있습니다. Spring이 문자열을 BrandSortType enum으로 자동 변환해줍니다. 이렇게 변경하면 서비스나 리포지토리 레이어에서 불필요한 null 체크 로직을 제거할 수도 있습니다.
| @RequestParam(required = false) BrandSortType sort | |
| ) { | |
| Long userId = userDetails.getUserId(); | |
| return CustomResponse.ok( | |
| userFavoriteService.getMyFavoriteBrands( | |
| userId, | |
| sort == null ? BrandSortType.MATCH_SCORE : sort | |
| ) | |
| @RequestParam(defaultValue = "MATCH_SCORE") BrandSortType sort | |
| ) { | |
| Long userId = userDetails.getUserId(); | |
| return CustomResponse.ok( | |
| userFavoriteService.getMyFavoriteBrands( | |
| userId, | |
| sort | |
| ) |
| @RequestParam(required = false) CampaignSortType sort | ||
| ) { | ||
| Long userId = userDetails.getUserId(); | ||
|
|
||
| return CustomResponse.ok( | ||
| userFavoriteService.getMyFavoriteCampaigns( | ||
| userId, | ||
| sort == null ? CampaignSortType.MATCH_SCORE : sort | ||
| ) |
There was a problem hiding this comment.
getMyFavoriteBrands 메소드와 마찬가지로, sort 파라미터의 기본값 처리를 @RequestParam의 defaultValue 속성을 사용하여 개선할 수 있습니다.
| @RequestParam(required = false) CampaignSortType sort | |
| ) { | |
| Long userId = userDetails.getUserId(); | |
| return CustomResponse.ok( | |
| userFavoriteService.getMyFavoriteCampaigns( | |
| userId, | |
| sort == null ? CampaignSortType.MATCH_SCORE : sort | |
| ) | |
| @RequestParam(defaultValue = "MATCH_SCORE") CampaignSortType sort | |
| ) { | |
| Long userId = userDetails.getUserId(); | |
| return CustomResponse.ok( | |
| userFavoriteService.getMyFavoriteCampaigns( | |
| userId, | |
| sort | |
| ) |
| public void setTags(List<String> tags) { | ||
| this.tags = tags; | ||
| } |
There was a problem hiding this comment.
DTO 객체는 가능하면 불변(immutable)으로 유지하는 것이 좋습니다. setTags 메소드는 FavoriteBrandDto를 가변(mutable) 상태로 만듭니다. 이 setter를 제거하고, 모든 필드를 생성자를 통해 초기화하도록 하는 것을 고려해보세요. 이렇게 하면 객체의 상태를 예측하기 쉬워지고 부수효과(side effect)를 줄일 수 있습니다.
References
- Avoid using @Setter on domain or DTO objects to prevent uncontrolled external modifications. If mutation is needed, provide specific methods for it.
1 participant
Summary
내가 찜한 브랜드 및 캠페인 조회 추가
Changes
Type of Change
Related Issues
#345
참고 사항