[7주차/빈] 워크북 제출합니다

[2u6in]

✅ 실습 체크리스트

• 이론 학습을 완료하셨나요?
• 미션 요구사항을 모두 이해하셨나요?
• 실습을 수행하기 위한 공부를 완료하셨나요?
• [] 실습 요구사항을 모두 완료하셨나요?

✅ 컨벤션 체크리스트

• 디렉토리 구조 컨벤션을 잘 지켰나요?
• pr 제목을 컨벤션에 맞게 작성하였나요?
• pr에 해당되는 이슈를 연결하였나요?(중요)
• 적절한 라벨을 설정하였나요?
• 파트장에게 code review를 요청하기 위해 reviewer를 등록하였나요?
• 닉네임/main 브랜치의 최신 상태를 반영하고 있는지 확인했나요?(매우 중요!)

📌 주안점

저번 리뷰 때 언급해주신 것들 중에 BaseEntity를 선택적으로 상속받은 게 BaseEntity의 모든 필드값을 필요로 하지 않고 일부만 사용하는 경우에 상속을 받지 않고 필요한 부분만 선언해주는 식으로 작성을 했는데 이렇게 해도 괜찮을지 궁금합니다!

[YoungJJun]

우선 주안점에 대해 설명드릴게요! 필요한 필드만 직접 선언하는 방식에 대해서 사실 정답이 있는건 아니지만.. 아래 사항들을 참고해서 한번 생각 해보시고 결정해주시면 될 것 같아요.

1. 생성,수정,삭제 중 일부만 필요한게 실제로 나중에도 그렇게 유지될지 먼저 고민해봐야 할 것 같아요. 지금은 필요없다고 생각했는데 나중에 필요하게 될 수도 있으니까요.

2. 생성, 수정, 삭제 중 1개만 필요한데 3개를 모두 상속받으면 안 쓰는 필드가 있어서 비효율적이지 않나 생각하실 수도 있는데, 이에 대한 비용은 엄청 작아요. 이 비용을 줄이는 것보다는 모두 상속받고 안쓰는건 그냥 두는 방향이 더 안전할 것 같아요.

3. 만약 엔티티마다 필요한 범위가 명확하게 다르고 그게 의미있는 구분이라면 상속 구조로 만들어서 계층화를 시키는 방법도 있습니다.

   @Getter
   @MappedSuperclass
   @EntityListeners(AuditingEntityListener.class)
   public abstract class BaseTimeEntity {
   
       @CreatedDate
       @Column(updatable = false)
       private LocalDateTime createdAt;
   
       @LastModifiedDate
       private LocalDateTime updatedAt;
   }
   
   @Getter
   @MappedSuperclass
   public abstract class BaseEntity extends BaseTimeEntity {
   
       private LocalDateTime deletedAt;
   
       public void softDelete() {
           this.deletedAt = LocalDateTime.now();
       }
   
       public boolean isDeleted() {
           return this.deletedAt != null;
       }
   }

   → 생성, 수정만을 기록하는 경우 BaseTimeEntity를 상속받고, soft delete가 필요한 엔티티는 BaseEntity를 상속받는 구조

저는 생성, 수정, 삭제 모두 추가하는게 큰 비용이 드는게 아니니까 BaseEntity 하나 두고 그냥 상속 받는게 깔끔하고 실수할 일 없다고 생각하기는 합니다!

[YoungJJun]

7주차 피드백

1. 이번주 미션에 대해서 오프셋, 커서 기반 페이지네이션 모두 잘 적용해 주셨습니다.

2. 여러 ReqDTO에 검증 어노테이션 추가 및 GeneralExceptionAdvice 구현도 잘 된 것 같아요!

3. ReviewSuccessCode 에 REVIEW_FOUND

   HttpStatus.FOUND 로 설정되어 있는데 이게 단어는 FOUND인데 저희가 생각하는 찾았다는 의미의 FOUND가 아니라 302 Redirect를 의미합니다.

   따라서 조회 성공 응답은 HttpStatus.OK (200) 이 적절한 응답 입니다!

4. ReviewRepository.findAllByMemberIdOrderByRatingDesc

   @Query 부분에 :memeberId 라고 오타 있어요.

5. ReviewService.getMyReviews()

   중간에 if (!req.cursor().equals(”-1”)) 에서 cursor가 null일 경우 NPE

   바로 아래 switch (req.sort().toLowerCase()) 에서 sort가 null일 경우 .toLowerCase() 호출 시 NPE

   두 값에 null이 들어올 수 있는지 체크 해주시고 검증로직 추가해주시면 좋을 것 같아요.

   그리고 1번 상황 같은 경우는 "-1".equals(req.cursor()) 이런식으로 순서를 바꾸면 cursor가 null이어도 해결됩니다.

   이렇게 순서 바꾸는 패턴 엄청 자주 보게 되는것 같아서 알아두시면 좋을 것 같아요.

6. 생일이 LocalDate에서 LocalDateTime으로 변경하셨는데 저는 생일이면 시간은 포함하지 않는게 더 자연스러운 것 같아요. 특별히 시간까지 기록하는 이유가 있으신가요??

7. HomeController

    public ApiResponse<HomeResDTO.HomeDTO> getHome(
               @RequestParam(name = "region") @NotBlank String region,
               @PathVariable @NotNull Long memberId
       )

   검증 어노테이션 붙어있는데 컨트롤러에 @Validated가 없습니다. @RequesBody는 @Validated 없이 각 항목에 @Valid 추가만으로 동작하지만 @RequestParam, @PathVariable 는 @Validated 가 붙어있는 상황에서만 동작합니다. 추가해주셔야 @NotNull, @NotBlank 동작할 것 같네요.

---

빈 7주차 수고하셨습니다! 담주도 화이팅해주세요~