kuk329 / reservation

0 stars 0 forks source link

FeedService #4

Open smilejakdu opened 6 months ago

smilejakdu commented 6 months ago

아까 밑에도 말했던건데 행동을 나누게 되면 코드가 깔끔해지고 가독성이 올라가게 됩니다. 그리고 다른곳에 필요한것과 필요하지않는것에 대한 접근제한자로 나눠서 분리시킨다. 를 기억하시면 될것같아요 .

예를 들게되면요

FeedService

보시면

 List<Follow> follows = followRepository.findAllByFromUser(user);
        List<User> users = follows.stream().map(Follow::getToUser).toList(); // 내가 팔로우한 유저 목록

로 되어있잖아요 이거 한묶음 인것같은데 하나의 행동으로 뺄 수 있을것 같아요 follows 가 다른곳에 쓰이지 않고 단순 내가 팔로우한 유저 목록을 만들기위해 follows 를 하신거죠? 그러면 저거는 따로 행동으로 뺄 수 있을것 같아요 .

    private List<User> getFollowedUsersList(User user) {
        return followRepository.findAllByFromUser(user).stream()
                .map(Follow::getToUser)
                .collect(Collectors.toList());
    }

그리고 밑에는 뭐하는 행위 인지는 잘 모르겠다만....

     if(users.size()>0){
            List<UserLog> allByUserIn = feedRepository.findAllByActorIn(users);
            for (UserLog userLog : allByUserIn) {
                result.add(new GetFeedRes(userLog.getLog()));
            }
            // todo : paging 처리 + 정렬 순서

        }

list 찾아가지고 존재한다면 로그를 쌓는것 같은데요. 이것도 그럼 메서드로 네이밍 정확하게 해줘서 따로 행동으로 빼주는게 좋을것 같아요. 그리고 이 코드 다른곳에 쓰이나요 ?? 아니라면 접근제한자 private 로 두시고요.

그리고 알고리즘 수업때 얘기하긴 했었는데 , 빈값에 대한 이라던지 그런 return 들은 상단에 두는게 보는 사람이 편하다 라고 했었잖아요ㅎㅎ 그렇게 적용하시면 됩니다.

private 너가 원하는 ResponseDto CreateUserLog(List<User> users)  {
   if (user.isEmpty()) {  // -> 이거 size 에서 isEmpty() 로 변경해도 되지않나요 ??
       return 어떤 .. NotFound 라던지 등등
   }

   List<UserLog> allByUserIn = feedRepository.findAllByActorIn(Users);
   return allByUserIn.stream()
                  .map(GetFeedRes::new).collect(Collectors.toList)
}

이런식으로 변경하면 좋지않을까??


    public List<GetFeedRes> getFeedList(String userEmail) {
        User user = userRepository.findByEmail(userEmail)
                .orElseThrow(() -> new BaseException(USERS_INVALID_EMAIL));

        List<User> followedUsers = getFollowedUsersList(user);
        return createUserLog(followedUsers);
    }

  private List<User> getFollowedUsersList(User user) {
        return followRepository.findAllByFromUser(user).stream()
                .map(Follow::getToUser)
                .collect(Collectors.toList());
 }

private 너가 원하는 ResponseDto createUserLog(List<User> users)  {
   if (user.isEmpty()) {  // -> 이거 size 에서 isEmpty() 로 변경해도 되지않나요 ??
       return 어떤 .. NotFound 라던지 등등
   }

   List<UserLog> allByUserIn = feedRepository.findAllByActorIn(Users);
   return allByUserIn.stream()
                  .map(GetFeedRes::new).collect(Collectors.toList)
}

코딩테스트 수업때 얘기했던거 별반 다를거 없습니다. 그때 제가 말씀드렸던거 프로젝트에서도 그대로 하시면 돼요

이런식으로 지금 프로젝트 모두 리팩토링 해주시고 pr 날려주세요