refactor: 상황에 맞는 페이지네이션 검증 #8
Conversation
yungic
requested review from
GyungA,
GiJungPark,
wambatcodeeee,
jaewon0926 and
bellra-jin
| @@ -32,6 +32,7 @@ | |||
| @RequiredArgsConstructor | |||
| public class BookmarkController { | |||
|
|
|||
| private final PageableValidation pageableValidation; | |||
There was a problem hiding this comment.
의존성을 주입할 필요가 없습니다.
PageableValidation의 메서드를 static하게 작성해서 가져오면 됩니다.
| } else if (size < 1){ | ||
| throw new ValidationException("사이즈는 1 이상이어야 합니다."); | ||
| } | ||
| pageableValidation.pageValidate(page, size); |
There was a problem hiding this comment.
팩토리 메서드 PageableValidation.pageValidate(page, size); 로 수정되겠네요.
There was a problem hiding this comment.
해당사항은 pageableVaildation.pageVaildate(page, size)을 사용한 모든곳에 일괄로 수정하셔야 할 것 같습니다.
| * Pageable 까지 만들어서 넘기는 버전, 그냥 검증한 하는 버전 두 가지 버전의 메서드를 구현 | ||
| * 하나로 만들면 좋았겠다. 하지만 서비스단까지 바꿔야 하는 관계로 패스 | ||
| */ | ||
| public void pageValidate(int page, int size) { |
There was a problem hiding this comment.
public static void pageValidate(int page, int size) 로 변경하시면 될 것 같습니다.
| @@ -66,7 +66,7 @@ public ResponseEntity<List<BookmarkListResponse>> getBookmarkedPapersByUserId( | |||
| @RequestParam(name = "page", defaultValue = "1") int page, | |||
| @RequestParam(name = "size", defaultValue = "20") int size) { | |||
|
|
|||
| pageableValidation.pageValidate(page, size); | |||
| pageValidate(page, size); | |||
There was a problem hiding this comment.
PageableValidation 팩토리 메서드로 변경한점 확인했습니다.
| @@ -26,13 +25,14 @@ | |||
|
|
|||
| import java.util.List; | |||
|
|
|||
| import static com.elice.ustory.global.Validation.PageableValidation.pageValidate; | |||
There was a problem hiding this comment.
현재는 정적으로 Import 했는데, 이에 대한 장점과 단점을 고민해보시면서 사용하시면 좋을것 같아요.
지금은 팩토리 메서드 명 중복이 생길수 있는 위험성이나, 유추가 불가능한 메서드가 아니기 때문에 좋은것 같습니다.
There was a problem hiding this comment.
확인했습니다. 안 그래도 고민 했었는데 말 그대로 유추가 불가능하지 않아서 그대로 썼습니다.
| private NoticeService noticeService; | ||
|
|
||
| public NoticeController(NoticeService noticeService, PageableValidation pageableValidation) { | ||
| this.pageableValidation = pageableValidation; | ||
| public NoticeController(NoticeService noticeService) { | ||
| this.noticeService = noticeService; | ||
| } |
There was a problem hiding this comment.
다른곳에서는 RequiredArgsConstructor 애너테이션을 사용했는데, 여기에서는 직접 생성자 주입 코드를 작성하신 이유가 있을까요?
There was a problem hiding this comment.
Notice 컨트롤러를 구현해주신 진희님께서 생성자 주입 코드로 진행해 주셔서 애너테이션 대신 주입 코드를 작성 했습니다.
There was a problem hiding this comment.
음.. notice 부분을 애너테이션으로 고쳐놓는게 더 좋을까요???
기중님이 구현한 코드에서는 pageable (pagerequest)이 서비스 에어리어에서 만들어지고
재원, 진희님이 구현한 코드에서는 pagealbe이 컨트롤러 에어리어에서 만들어지기 때문에, 각각 다른 코드로 만들어서
리팩터 진행했습니다.
세 분 어사인으로 걸어놨고, 다 확인하시면 어사인 하시고 머지 되게 해주시면 됩니다 :)