feat(profile): 프로필 기능 구현 #30
Conversation
| private BooleanExpression majorFilter(List<Major> majors) { | ||
| if (majors == null) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
List의 null 여부를 검사할 때 List.isEmpty()를 사용하는게 어떨까요??
There was a problem hiding this comment.
list에 아무것도 넣지 않은 경우는 null이 들어와서 isEmpty 검사를 하면 NPE가 발생하더라구요. 그래서 저렇게 했습니다!
| @PutMapping | ||
| @ResponseStatus(HttpStatus.CREATED) | ||
| public void update(@Valid @RequestBody ProfileUpdateRequest requestDto) { |
There was a problem hiding this comment.
update를 하는 로직이니깐 CREATED보다는 NO_CONTENT가 낫지 않을까요??
| @GetMapping("/{profileId}") | ||
| @ResponseStatus(HttpStatus.OK) | ||
| public ProfileResponse readOne(@PathVariable(name = "profileId") Long profileId) { | ||
| return ProfileResponse.from(queryService.readOne(profileId)); | ||
| } |
There was a problem hiding this comment.
URL에서 공백을 대문자로 구분하기 보다는 -을 사용하게 더 좋아보여요!!
| public static ProfileResponse from(Profile profile) { | ||
| return new ProfileResponse( | ||
| profile.getWriter().getId(), | ||
| profile.getWriter().getName(), | ||
| profile.getInformation().getImageUrl(), | ||
| profile.getInformation().getMajor(), | ||
| profile.getOnlineProfile().getGithubUrl(), | ||
| profile.getOnlineProfile().getResume(), | ||
| profile.getOnlineProfile().getPortfolio() | ||
| ); | ||
| } |
There was a problem hiding this comment.
저는 DTO에서도 생성자의 매걔변수가 많을 때는 빌더 패턴을 주로 사용하는 데 창보님을 어떻게 생각하시나요?
There was a problem hiding this comment.
저는 Builder 패턴을 좋아하지 않아요. builder는 값을 빼먹을 가능성이 너무 높다고 느껴져요. 그래서 저는 값을 강제하는 생성자를 선호합니다
| public ProfileNotFoundException(User writer) { | ||
| super(HttpStatus.NOT_FOUND, String.format("%s이 생성한 profile이 없습니다.", writer.getId())); | ||
| } | ||
|
|
||
| public ProfileNotFoundException(Long id) { | ||
| super(HttpStatus.NOT_FOUND, String.format("%s의 아이디를 가진 profile이 없습니다.", id)); | ||
| } |
There was a problem hiding this comment.
예외에서 매걔변수를 받아 같이 처리하는 방법이 되게 좋은 거 같아요!!
| public interface ProfileRepository extends JpaRepository<Profile, Long> { | ||
| boolean existsProfileByWriter(User writer); |
There was a problem hiding this comment.
여기서 ProfileQueryRepository까지 다중 상속 받아 사용할 때 ProfileRepository만 선언하여 쓰는 게 가독성 면에서 좋을 거 같아요.
There was a problem hiding this comment.
저도 그게 좋다고 생각을 했는데, ProfileRepository가 ProfileQueryRepository의 함수까지 읽어서 자동으로 쿼리를 생성하려고 하더라구요... 아마 새로 생긴 기능인 것 같습니다. 그래서 이렇게 가야할 것 같아요...😿
There was a problem hiding this comment.
아이고... 네이밍을 조금 바꾸면 되지 않을까요??
🎫 관련 이슈
close #27
📄 개요
🏁 확인 사항
🙋🏻 덧붙일 말
비즈니스의 흐름을 파악하기 쉬운지, 더 좋은 어노테이션이나 방법이 없는지, 발생할 수 있는 문제는 없는지, QueryDSL로 동적쿼리 만든 부분은 괜찮은지를 집중적으로 리뷰해주셨으면 합니다!