Conversation
PicturePark1101
approved these changes
Feb 17, 2025
| import org.hankki.hankkiserver.api.store.parameter.SortOption; | ||
| import org.hankki.hankkiserver.common.code.StoreErrorCode; | ||
| import org.hankki.hankkiserver.common.exception.BadRequestException; | ||
| import org.springframework.core.convert.converter.Converter; |
Contributor
There was a problem hiding this comment.
enum class에서 변환하지 않고 별도의 컨버터 클래스를 구현하신 이유가 궁금합니다 (순수 궁금증)
제 생각으론 domain과 서비스 로직을 분리하고자 이렇게 하신 것 같은데 혹시 맞으실까요??
Contributor
Author
There was a problem hiding this comment.
더 세부적인 예외메시지 처리를 위해 했습니다.
wldks1008
reviewed
Feb 17, 2025
Comment on lines
45
to
47
| public List<Store> findAllByDynamicQueryWithPaging(StoreCategory storeCategory, PriceCategory priceCategory, SortOption sortOption, CustomCursor cursor, int PAGE_SIZE) { | ||
| return storeRepository.findAllByCategoryAndLowestPriceAndUniversityIdAndIsDeletedFalseOrderBySortOptionsWithPaging(storeCategory, priceCategory, sortOption, cursor, PAGE_SIZE); | ||
|
|
Member
There was a problem hiding this comment.
상수인 것을 강조하기 위해 파라미터 변수명을 대문자로 사용하신걸까요??
사소한 것이지만 제 생각에는 자바 코딩 컨벤션에 따라 변수명을 카멜 케이스로 작성하는 것이 더 맞다고 생각해요. 어짜피 해당 함수를 구현하는 로직에서 상수를 이용한다는 것을 알 수 있을 텐데, 굳이 파라미터 변수명까지 상수처럼 표기하는 것은 불필요하다고 생각합니다. 오히려 저는 해당 클래스에 상수가 존재하지 않는데 왜 대문자로 표기한 것인지 처음볼 때는 헷갈렸어요. 어떻게 생각하시나요?
|
|
||
| } | ||
|
|
||
| public static class CustomCursorBuilder { |
Member
There was a problem hiding this comment.
외부에서는 사용하지 않는 내부 클래스이니 private으로 접근 제어자를 변경하는 것은 어떨까요?
Comment on lines
+44
to
+52
| private SortOption getSortOptionFromRequest() { | ||
| ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes(); | ||
| if (attributes != null) { | ||
| HttpServletRequest request = attributes.getRequest(); | ||
| String sortOption = request.getParameter("sortOption"); | ||
| return sortOption != null ? SortOption.valueOf(sortOption.toUpperCase()) : null; | ||
| } | ||
| return null; | ||
| } |
Member
There was a problem hiding this comment.
이미 String을 SortOption enum으로 변경해주는 converter를 만들어 두었는데 이 로직이 이중으로 필요한가요? 위에서 그냥 cursor.getSortOption()을 호출해주면 될 것 같은데.. 저도 잘 몰라서 질문 드립니다.
Contributor
Author
There was a problem hiding this comment.
현재 requestparam이 required = false입니다.
그래서 클라이언트에서 sortoption을 아예 보내지 않은 경우, converter의 convert로직을 타지 않습니다.
-> customcursorvalidator 수행시 sortoption은 여전히 null일 수 있습니다.
그래서 null인 경우도 추가했습니다.
Parkjyun
added a commit
that referenced
this pull request
Mar 6, 2025
* [feat] 족보 공유하기 기능 구현 (#227) * [feat] add request, command dto * [feat] imple repository logic * [feat] imple service, refac origin functions * [feat] imple controller * [refac] Applying feedback * [feat] Applying feedback * [feat] 족보 주인인지 여부 리턴하는 API 구현 (#229) * [feat] make response, command dto * [feat] implementing repository func * [feat] implementing controller func * [feat] Add Exception Handling * [refac] Remove Unnecessary Code * [feat] 족보 주인 닉네임 조회 API (#231) * [feat] Implement controller * [feat] Implement service * [refac] Refacotr dto naming * [feat] 족보 공유 관련 API 분리, 시큐리티 whiteList에 추가 (#233) * [refac] Refactor api path, Add shared favaroite GET api * [feat] Add shared favaroite GET api * [feat] Add businessLogicWhileList path * [feat] Add response dto * [feat] 전체 카테고리 추가 (#236) * [feat] 전체 카테고리 추가 * [chore] add All category image url to yml * [feat] 식당 상세조회 반환 스펙 변경 (#237) * [fix] fix FavoriteImage update logic (#239) * [feat] 가격대 전체 카테고리 추가 (#242) * [feat] change store category sequence * [feat] delete unnecessary code * [feat] add PriceCategory Converter * [refac] move webConfig and related class * [feat] delete unnecessary log * [refac] 메뉴 추가 시 발생 쿼리문 개선 (#244) * [refac] optimize query execution by reducing select queries * [refac] improve nested loop * [refac] delete unnecessary query * [refac] minimize interdependence between methods * [feat] 식당 카테고리 api 버저닝 (#251) * [fix] add versioning in store category api * [fix] fix typo * [fix] 좋아요 수 동시성 이슈 처리 (#249) * [feat] apply optimistic lock * [chore] add aop dependency * [fix] set default version * [feat] make custom retry annotation * [feat] logic for retry * [feat] apply retry logic * [chore] change error message * [refac] move to other package * [refac] change throwing exception class * [refac] simplify exception handling * [refac] change error message * [refac] 로그인 로직 리팩토링 (#252) * [refac] move package * [refac] change expiration date * [refac] make OAuthProvider interface * [refac] implement interface * [refac] use factory method pattern * [refac] adhere to the Dependency Inversion Principle * [refac] change to primitive type * [refac] delete unused method * [refac] set default user name * [refac] extract hardcoded strings into constants * [refac] use TypeReference for safe deserialization * [refac] change access modifier from public to protected * [fix] replace to @RequestParam for parameter binding * [refac] change package name * [refac] divide method * [refac] add final keyword * [refac] delete blank for adhere convention * [refac] use transaction annotation on method * [refac] use bean name to find class * [refac] change package * [feat] add auto creation of release (#255) * [feat] add auto creation of release * [refac] add blank Co-authored-by: kgy1008 <libra001008@gmail.com> * [feat] 데이터베이스를 MySQL로 이전 (#256) * [feat] add mysql and h2 dependency * [chore] update submodule Co-authored-by: kgy1008 <libra001008@gmail.com> * [chore] update submodule Co-authored-by: kgy1008 <libra001008@gmail.com> * [chore] changes images of README.md --------- Co-authored-by: kgy1008 <libra001008@gmail.com> * [chore] changes yml (#258) * [refac] 인증 비즈니스 로직 리팩토링 (#253) * [refac] make user info dto * [refac] extract external service from transaction * [refac] handle feign exception * [refac] rename variable * [refac] apply facade pattern * [refac] use final keyword * [refac] use final keyword * [refac] delete unnecessary transaction * [refac] delete unnecessary logout logic in validation * [refac] add transactional annotation * [refac] extract external network from transactional * [refac] rename class * [refac] move package * [docs] update README.md * [refac] delete static method * [fix] validate null name * [refac] separate responsibilities by creating dedicated classes * [refac] delete AuthValidator.java * [refac] summarize parameter * [refac] refactor to use string constants * [refac] move location of validation logic * [refac] divide method * [fix] Revoke refresh token renewal on access token reissuance * [fix] change access modifier to public for apply transactional annotation * [feat] 홈뷰 페이지네이션 (#262) * [feat] create custom universitystore repository interface * [refactor] rename method of custom store repository * [feat] change service layer for pagination * [feat] create custom cursor * [refactor] extract methods related with orderspecifier * [feat] create cursor provider * [feat] create store page response * [feat] add method signature related with paging * [feat] implement interface to provide paging * [feat] create logic for store pagination without university filter * [feat] add sortoption converter * [feat] create dynamicQueryProvider * [feat] create CustomUniversityStoreRepositoryImpl * [refactor] rename method used for dynamic query * [feat] create CustomUniversityStoreRepository * [feat] create logic for store pagination * [feat] create store retrieval api with pagination * [refactor] delete magic number * [fix] delete lazy initialization * [refactor] resolve conflict * [chore] delete unnecessary dependency * [refactor] refactor duplication in webconfig * [chore] add gitignore * [feat] create custom validator for cursor * [feat] add enum converter * [refactor] refactor enum converters * [feat] add whitelist * [refactor] refactor logic * [feat] add handling exception type * [feat] fix logic to ignore price category All * [feat] create test utils * [feat] create test fixtures * [feat] create service slice test * [feat] create PriceCategoryConverter test * [feat] create StoreControllerTest * [feat] add profile to test * [refactor] optimize import * [refactor] delete validated * [refactor] change test to assert appropriate values * [refactor] delete redundant logic * [refactor] fix typo * [refactor] change method to non-static * [feat] add test code * [fix] fix cursor validation logic * [refactor] change access level * [refactor] change parameter naming * [refactor] add model attribute for readability * [feat] apply refresh Token rotation (#264) --------- Co-authored-by: 박서진 <109809242+PicturePark1101@users.noreply.github.com> Co-authored-by: kgy1008 <gayeon1008@g.skku.edu> Co-authored-by: kgy1008 <libra001008@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue 📌
close #240
Description ✔️
To Reviewers