[커뮤니티 #14] 기존 커뮤니티 프로젝트 클린코드 리팩토링 (Comment, Report)

최근에 바빠서 레거시 프로젝트 리팩토링을 진행하지 못했습니다.

​

지난 번에 이어서 오늘도 클린코드로 리팩토링 해보겠습니다~

 

---

Comment API 리팩토링

오랜만에 댓글 API를 보니 바꿀게 한 두가지가 아니었습니다.

​

변수, 메서드명부터 시작해서 잘못된 stream 사용, 그리고 도메인 메서드로 분리할 수 있는 것들도 보였습니다.

오늘은 이것들을 바꿔볼 예정입니다.

​

 

기존 CommentService 먼저 보겠습니다.

@RequiredArgsConstructor
@Service
@Slf4j
public class CommentService {
    private final CommentRepository commentRepository;
    private final BoardRepository boardRepository;

    @Transactional(readOnly = true)
    public List<CommentDto> findAll(CommentReadCondition condition) {
        List<Comment> commentList = commentRepository.findByBoardId(condition.getBoardId());
        List<CommentDto> commentDtoList = new ArrayList<>();
        commentList.stream().forEach(i -> commentDtoList.add(new CommentDto().toDto(i)));
        return commentDtoList;
    }

    @Transactional
    public CommentDto create(CommentCreateRequest req, User user) {
        Board board = boardRepository.findById(req.getBoardId()).orElseThrow(BoardNotFoundException::new);
        Comment comment = new Comment(req.getContent(), user, board);
        commentRepository.save(comment);
        return new CommentDto().toDto(comment);
    }

    @Transactional
    public void delete(int id, User user) {
        Comment comment = commentRepository.findById(id).orElseThrow(CommentNotFoundException::new);
        if (comment.getUser().equals(user)) {
            commentRepository.delete(comment);
        } else {
            throw new MemberNotEqualsException();
        }
    }
}

사실 매개변수로 넘어오는 id 값도 모두 Long 타입으로 바꿔주고 싶은데, 이거는 추후에 진행하고

​

먼저 변수, 메서드 명을 바꾸고 / 도메인 함수로 분리 / 메서드 분리 / Stream 사용 변경을 진행해보겠습니다.

 

 

 

리팩토링 진행 후 CommentService 코드

@RequiredArgsConstructor
@Service
@Slf4j
public class CommentService {
    private final CommentRepository commentRepository;
    private final BoardRepository boardRepository;

    @Transactional(readOnly = true)
    public List<CommentDto> findAllComments(CommentReadCondition condition) {
        List<Comment> comments = commentRepository.findByBoardId(condition.getBoardId());
        List<CommentDto> commentsDto = comments.stream()
                .map(comment -> new CommentDto().toDto(comment))
                .collect(Collectors.toList());
        return commentsDto;
    }

    @Transactional
    public CommentDto createComment(CommentCreateRequest req, User user) {
        Board board = boardRepository.findById(req.getBoardId()).orElseThrow(BoardNotFoundException::new);
        Comment comment = new Comment(req.getContent(), user, board);
        commentRepository.save(comment);
        return new CommentDto().toDto(comment);
    }

    @Transactional
    public void deleteComment(int id, User user) {
        Comment comment = commentRepository.findById(id).orElseThrow(CommentNotFoundException::new);
        validateDeleteComment(comment, user);
        commentRepository.delete(comment);
    }

    private void validateDeleteComment(Comment comment, User user) {
        if (!comment.isOwnComment(user)) {
            throw new MemberNotEqualsException();
        }
    }
}

어떤가요? 조금 더 깔끔해진 것 같나요?

Comment 도메인 메서드 추가와, 기존 서비스 메서드 분리를 통해 코드 리팩토링을 진행하고, 기존 테스트 코드 또한 리팩토링을 진행해주었습니다.

 

 

 

Report API 리팩토링

Report API는 오랜만에 보니 더욱 가관이었습니다.

​

먼저 Controller에서 공통적으로 사용하는 유저 세션 조회에 대한 코드는 따로 내부 메서드로 분리해주었습니다.

세션 부분은 추후에 어노테이션으로 다시 리팩토링을 할 예정입니다.

 

@Api(value = "Report Controller", tags = "Report")
@RequiredArgsConstructor
@RestController
@RequestMapping("/api")
public class ReportController {
    private final ReportService reportService;
    private final UserRepository userRepository;

    @ApiOperation(value = "유저 신고", notes = "유저를 신고합니다.")
    @ResponseStatus(HttpStatus.OK)
    @PostMapping("/reports/users")
    public Response reportUser(@Valid @RequestBody UserReportRequest userReportRequest) {
        return Response.success(reportService.reportUser(getPrincipal(), userReportRequest));
    }

    @ApiOperation(value = "게시글 신고", notes = "게시글을 신고합니다.")
    @ResponseStatus(HttpStatus.OK)
    @PostMapping("/reports/boards")
    public Response reportBoard(@Valid @RequestBody BoardReportRequest boardReportRequest) {
        return Response.success(reportService.reportBoard(getPrincipal(), boardReportRequest));
    }


    private User getPrincipal() {
        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
        User user = userRepository.findByUsername(authentication.getName()).orElseThrow(MemberNotFoundException::new);
        return user;
    }
}

컨트롤러는 이렇게 냅두고, 정말 개판 오분 전인 서비스 코드를 시원하게 고쳐보겠습니다.

 

 

 

 

기존 ReportService 코드

@RequiredArgsConstructor
@Service
public class ReportService {
    public final BoardReportRepository boardReportRepository;
    public final UserReportRepository userReportRepository;
    public final UserRepository userRepository;
    public final BoardRepository boardRepository;

    @Transactional
    public UserReportResponse reportUser(User reporter, UserReportRequest req) {
        User reportedUser = userRepository.findById(req.getReportedUserId()).orElseThrow(MemberNotFoundException::new);

        if (reporter.getId() == req.getReportedUserId()) {
            // 자기 자신을 신고한 경우
            throw new NotSelfReportException();
        }

        if (userReportRepository.findByReporterIdAndReportedUserId(reporter.getId(), req.getReportedUserId()) == null) {
            // 신고 한 적이 없다면, 테이블 생성 후 신고 처리 (ReportedUser의 User테이블 boolean 값 true 변경 ==> 신고처리)
            UserReport userReport = new UserReport(reporter.getId(), reportedUser.getId(), req.getContent());
            userReportRepository.save(userReport);

            if (userReportRepository.findByReportedUserId(req.getReportedUserId()).size() >= 3) {
                // 신고 수 10 이상일 시 true 설정
                reportedUser.setReported(true);
            }

            UserReportResponse res = new UserReportResponse(userReport.getId(), UserEditRequestDto.toDto(reportedUser), req.getContent(), userReport.getCreatedAt());
            return res;
        } else {
            // 이미 신고를 했다면 리턴
            throw new AlreadyReportException();
        }
    }

    @Transactional
    public BoardReportResponse reportBoard(User reporter, BoardReportRequest req) {
        Board reportedBoard = boardRepository.findById(req.getReportedBoardId()).orElseThrow(BoardNotFoundException::new);

        if (reporter.getId() == reportedBoard.getUser().getId()) {
            throw new NotSelfReportException();
        }

        if (boardReportRepository.findByReporterIdAndReportedBoardId(reporter.getId(), req.getReportedBoardId()) == null) {
            // 신고 한 적이 없다면, 테이블 생성 후 신고 처리
            BoardReport boardReport = new BoardReport(reporter.getId(), reportedBoard.getId(), req.getContent());
            boardReportRepository.save(boardReport);


            if (boardReportRepository.findByReportedBoardId(req.getReportedBoardId()).size() >= 10) {
                // 신고 수 10 이상일 시 true 설정
                reportedBoard.setReported(true);
            }

            BoardReportResponse res = new BoardReportResponse(boardReport.getId(), req.getReportedBoardId(), req.getContent(), boardReport.getCreatedAt());
            return res;
        } else {
            throw new AlreadyReportException();
        }
    }
}

기존 코드는 조건문과 Getter, Setter를 남발했습니다.

별로 좋지 않은 코드 같네요.

​

리팩토링을 진행해보겠습니다.

 

 

 

@RequiredArgsConstructor
@Service
public class ReportService {
    private final static int NORMAL_USER_REPORT_LIMIT_FOR_BEING_REPORTED = 3;
    private final static int NORMAL_BOARD_REPORT_LIMIT_FOR_BEING_REPORTED = 10;
    public final BoardReportRepository boardReportHistoryRepository;
    public final UserReportRepository userReportHistoryRepository;
    public final UserRepository userRepository;
    public final BoardRepository boardRepository;

    @Transactional
    public UserReportResponse reportUser(User reporter, UserReportRequest req) {
        validateUserReportRequest(reporter, req);
        User reportedUser = userRepository.findById(req.getReportedUserId()).orElseThrow(MemberNotFoundException::new);
        UserReportHistory userReportHistory = createUserReportHistory(reporter, reportedUser, req);
        checkUserStatusIsBeingReported(reportedUser, req);
        return new UserReportResponse(userReportHistory.getId(), UserEditRequestDto.toDto(reportedUser),
                req.getContent(),
                userReportHistory.getCreatedAt());
    }

    private void checkUserStatusIsBeingReported(User reportedUser, UserReportRequest req) {
        if (userReportHistoryRepository.findByReportedUserId(req.getReportedUserId()).size()
                >= NORMAL_USER_REPORT_LIMIT_FOR_BEING_REPORTED) {
            reportedUser.setStatusIsBeingReported();
        }
    }

    private UserReportHistory createUserReportHistory(User reporter, User reportedUser, UserReportRequest req) {
        UserReportHistory userReportHistory = new UserReportHistory(reporter.getId(), reportedUser.getId(),
                req.getContent());
        userReportHistoryRepository.save(userReportHistory);
        return userReportHistory;
    }

    private void validateUserReportRequest(User reporter, UserReportRequest req) {
        if (reporter.isReportMySelf(req.getReportedUserId())) {
            throw new NotSelfReportException();
        }

        if (userReportHistoryRepository.existsByReporterIdAndReportedUserId(reporter.getId(),
                req.getReportedUserId())) {
            throw new AlreadyReportException();
        }
    }

    @Transactional
    public BoardReportResponse reportBoard(User reporter, BoardReportRequest req) {
        Board reportedBoard = boardRepository.findById(req.getReportedBoardId())
                .orElseThrow(BoardNotFoundException::new);
        validateBoard(reporter, reportedBoard, req);
        BoardReportHistory boardReportHistory = createBoardReportHistory(reporter, reportedBoard, req);
        checkBoardStatusIsBeingReported(reportedBoard, req);
        return new BoardReportResponse(boardReportHistory.getId(), req.getReportedBoardId(),
                req.getContent(), boardReportHistory.getCreatedAt());
    }

    private void checkBoardStatusIsBeingReported(Board reportedBoard, BoardReportRequest req) {
        if (boardReportHistoryRepository.findByReportedBoardId(req.getReportedBoardId()).size()
                >= NORMAL_BOARD_REPORT_LIMIT_FOR_BEING_REPORTED) {
            reportedBoard.setStatusIsBeingReported();
        }
    }

    private BoardReportHistory createBoardReportHistory(User reporter, Board reportedBoard, BoardReportRequest req) {
        BoardReportHistory boardReportHistory = new BoardReportHistory(reporter.getId(), reportedBoard.getId(),
                req.getContent());
        boardReportHistoryRepository.save(boardReportHistory);
        return boardReportHistory;
    }

    private void validateBoard(User reporter, Board reportedBoard, BoardReportRequest req) {
        if (reporter.isReportMySelf(reportedBoard.getUser().getId())) {
            throw new NotSelfReportException();
        }

        if (boardReportHistoryRepository.existsByReporterIdAndReportedBoardId(reporter.getId(),
                req.getReportedBoardId())) {
            throw new AlreadyReportException();
        }
    }
}

리팩토링 진행 후 코드입니다.

​

기존 코드보다는 깔끔해졌지만 이것도 100% 마음에 들지는 않습니다.

추후에 다시 리팩토링 진행해보도록 하겠습니다.

기존 코드에서 상수 추가, 도메인 메서드 분리, 서비스 메서드 분리 작업을 통해서 보다 더 객체지향적으로 설계했습니다.

​

테스트 코드까지 리팩토링 작업 완료했습니다.

​

더 자세한 코드는 아래 깃허브에서 보실 수 있고 피드백은 환영입니다!

 

https://github.com/sosow0212/community

GitHub - sosow0212/community: Community API Server (with JUnit5, Redis ...)