[BE] issue129: 원하는 갯수만큼 작성된 후기 조회 #136
Conversation
| @RequestParam(required = false) Integer size | ||
| @RequestParam(name = "size", required = false, defaultValue = "") SizeRequest sizeRequest |
There was a problem hiding this comment.
SizeRequest 라는 DTO로 감싸주었군요..!!
기존의 @RequestParam(required = false) Integer size 와는 어떤 차이가 있는지 궁금합니다!!
| @Override | ||
| public SizeRequest convert(final String source) { | ||
| return source.isBlank() ? SizeRequest.empty() : new SizeRequest(Integer.parseInt(source)); | ||
| } |
There was a problem hiding this comment.
음..빈 값을 포함한 요청일 때마다 Converter 가 추가된다면 관리가 힘들지 않을까하는 생각이 들어요..ㅠ.ㅠ🥲
There was a problem hiding this comment.
아래에도 CategoryIdConverter가 있던데 비슷한 처리를 할 때마다 Converter를 만드는 방법 이외에 더 좋은 방법은 없을까요?? 아니면 혹시 Converter가 꼭 필요한 이유가 있을까요?
| if (sizeRequest.isEmpty() || sizeRequest.isMoreThan(allReviews.size())) { | ||
| return new ReviewsResponse(allReviews); |
There was a problem hiding this comment.
단순 Integer가 아닌 DTO로 관리하니 isMoreThan 과 같은 메소드 호라용이 가능하군요..!!
| private static final MemberData JJANGGU = new MemberData(1L, "jjanggu", "https://image", "github.com"); | ||
| private static final MemberData GREENLAWN = new MemberData(2L, "greenlawn", "https://image", "github.com"); | ||
| private static final MemberData DWOO = new MemberData(3L, "dwoo", "https://image", "github.com"); | ||
| private static final MemberData VERUS = new MemberData(4L, "verus", "https://image", "github.com"); |
There was a problem hiding this comment.
자주 쓰이는 것 같아요!! Fixture로 빼도 될 것 같습니다!!
| private static Member toMember(MemberData memberData) { | ||
| return new Member(memberData.getGithubId(), memberData.getUsername(), memberData.getImageUrl(), | ||
| memberData.getProfileUrl()); | ||
| } |
There was a problem hiding this comment.
메소드명과 parameter 를 보고 기능을 알 수 있을 것 같네요! (코드를 보지 않아도)
해당 메소드를 맨 아래로 내려도 괜찮을 것 같아요!
| private final NamedParameterJdbcTemplate namedParameterJdbcTemplate; | ||
|
|
||
| public List<ReviewData> findAllByStudyId(final Long studyId) { | ||
| String sql = "SELECT r.id, r.content, r.created_date, r.last_modified_date, " |
There was a problem hiding this comment.
r로 축약하지 않고 review로 선언해도 길이가 길 것 같지 않아서 더 명확하게 표현하는 것은 어떨까요 ?!
There was a problem hiding this comment.
저는 이 부분은 그렇게 나쁘다고 생각하지 않습니다! sql에서 review r, member m 자체가 테이블의 별칭을 지어주는건데 여기에 굳이 또 review를 하는 것은 의미가 없다고 생각해요!
| private MockMvc mockMvc; | ||
|
|
||
| @MockBean | ||
| private ReviewService reviewService; |
There was a problem hiding this comment.
요청 값 예외 확인을 위한 테스트 클래스라서, 선언해도 쓰이지 않을 빈일 것 같은데 제거해도 되지 않을까요 ?
| @Override | ||
| public SizeRequest convert(final String source) { | ||
| return source.isBlank() ? SizeRequest.empty() : new SizeRequest(Integer.parseInt(source)); | ||
| } |
There was a problem hiding this comment.
아래에도 CategoryIdConverter가 있던데 비슷한 처리를 할 때마다 Converter를 만드는 방법 이외에 더 좋은 방법은 없을까요?? 아니면 혹시 Converter가 꼭 필요한 이유가 있을까요?
| private final NamedParameterJdbcTemplate namedParameterJdbcTemplate; | ||
|
|
||
| public List<ReviewData> findAllByStudyId(final Long studyId) { | ||
| String sql = "SELECT r.id, r.content, r.created_date, r.last_modified_date, " |
There was a problem hiding this comment.
저는 이 부분은 그렇게 나쁘다고 생각하지 않습니다! sql에서 review r, member m 자체가 테이블의 별칭을 지어주는건데 여기에 굳이 또 review를 하는 것은 의미가 없다고 생각해요!
|
|
||
| public boolean isMoreThan(int value) { | ||
| if (isEmpty()) { | ||
| throw new IllegalStateException("SizeRequest에 value가 null 입니다."); |
There was a problem hiding this comment.
BadRequestException을 상속하는 예외 클래스를 만들어도 좋을 것 같아요
There was a problem hiding this comment.
500 응답을 던지도록 ControllerAdvice 수정
|
|
||
| public boolean isMoreThan(int value) { | ||
| if (isEmpty()) { | ||
| throw new IllegalStateException("SizeRequest에 value가 null 입니다."); |
There was a problem hiding this comment.
500 응답을 던지도록 ControllerAdvice 수정
| private String createdDate; | ||
| private String lastModifiedDate; | ||
| private String content; |
resolve: #129