[BE-REFACTOR] 바이옴 포켓몬 티어 정렬 #363
Conversation
| } | ||
| return DESCENDING_COMPARATOR; |
There was a problem hiding this comment.
아래 compare과 개행 컨벤션이 안 맞는 것 같아요! 통일 부탁드려요
| public class NativePokemonComparator implements Comparator<NativePokemon> { | ||
|
|
||
| public static final String ASCENDING = "asc"; | ||
| public static final String DESCENDING = "desc"; | ||
| private static final NativePokemonComparator ASCENDING_COMPARATOR = new NativePokemonComparator(ASCENDING); | ||
| private static final NativePokemonComparator DESCENDING_COMPARATOR = new NativePokemonComparator(DESCENDING); |
There was a problem hiding this comment.
Comparator 라는 이름과 다르게 차순을 포함하는 상수도 관리하고 있는 듯 합니다!
차순은 해당 클래스의 상수로 관리하지 않고
BiomeService 에서 차순 관련 상수가 필요하다면 그곳에만 정의하는 것은 어떨까요?! 🤔
There was a problem hiding this comment.
현재 BiomeService에서는 상수를 사용하지 않고 Comparator의 compare 메소드와 BiomeController에서만 사용하고 있습니다! 비교하는 역할을 하는 클래스인 Comparator가 비교 기준을 가지고 있는게 더 낫지 않을까요?!
backend/pokerogue/src/main/java/com/pokerogue/helper/biome/service/NativePokemonComparator.java
Outdated
Show resolved
Hide resolved
|
/noti @jinchiim 폴라!!! 코멘트(이지만 질문) 달았어요 😄 |
backend/pokerogue/src/main/java/com/pokerogue/helper/biome/service/NativePokemonComparator.java
Outdated
Show resolved
Hide resolved
|
/noti 여러분 정렬 기준 문자열을 enum으로 관리하게 되면서 컨버터 등등이 들어났는데요 😅 천천히 봐주세요.. 변경 사항이 꽤 있어서 다시 리뷰 요청합니다 🙇🏻♀️ 제가 맡은 부분 외에 건들인게 있어서 이것만 추가 설명 적겠습니다 ConverterConfig 변경
의견주세요!! |
|
|
||
| @Configuration | ||
| public class ConverterConfig { | ||
| @EnableMongoRepositories(basePackages = {"com.pokerogue"}) | ||
| public class DataMongoDbConfig { |
| public ApiResponse<BiomeDetailResponse> biomeDetails(@PathVariable("id") String id, | ||
| @RequestParam(value = "boss", defaultValue = "desc") SortingCriteria bossPokemonOrder, | ||
| @RequestParam(value = "wild", defaultValue = "asc") SortingCriteria wildPokemonOrder) { |
There was a problem hiding this comment.
지금은 /api/v1/biome/grass?boss=asc&wild=desc 요런 api구조가 될 거 같은데
/api/v1/biome/grass?sort=boss-asc,wild-desc 이런 스타일은 어떠신지..!
| } | ||
|
|
||
| @Override | ||
| public int compare(NativePokemon firstPokemon, NativePokemon secondPokemon) { |
|
|
||
| @Configuration | ||
| public class ConverterConfig { | ||
| @EnableMongoRepositories(basePackages = {"com.pokerogue"}) |
There was a problem hiding this comment.
패키지가 조금 광범위한 것 같은데, repoitory가 따로 있는 것이 아니라서 쩔수 없긴 하지만
helper 까지만이라도 줄여볼까요?? 🤔
🍄 PR 확인 사항
PR이 다음 요구 사항을 충족하는지 확인하세요. :
현재 작업은 어떤 이슈를 해결한 것인지 설명해주세요.
기존 코드에서 변경된 점이 있다면 설명해주세요. (추가 X)
이건 제안인데요! 쿼리 스트링으로 보스 포켓몬과 야생 포켓몬들 티어 정렬 기준을 받도록 api를 수정하면 어떨까요? 사용자 활동 로그를 바탕으로 정렬 순서도 고치기로 했던 걸로 기억하는데 정렬 같은 조건문은 요구사항에 따라 바뀌는 것보다 이렇게 설계하는게 더 유연해보여서 제안합니다 :)
보스 포켓몬들에 대해 티어 높은 순으로 정렬만 요구사항이었기 때문에 쿼리스트링 default 값을 보스 포켓몬은 desc, 야생 포켓몬은 asc로 설정해보았습니다. (하위 호환성 O)별로라면 철회하겠읍니다..