[SPRING BOOT] 즐겨찾기 추가/해제 코드 변천사

 

---

 

 

 

간략하게 상황을 적어보자면, 스프링 관련 프로젝트를 본격적으로 시작한 건 올해 2월 초였다.

그때 처음 api를 만드는 작업을 했었는데, 프로젝트가 급박하게 진행이 되다 보니 api가 돌아가게 하기 바빴다.

누군가에게 코드 리뷰를 받을 기회 같은 게 없었다.

그러니까 지금 내 상황은 요건을 받으면, 작동을 하게 만들 수는 있었지만 깔끔한 코드는 아닌..

클린코딩이 아닌... 뭐 그런 상황이었다.

 

아무튼 이번에 들어갈 다른 프로젝트를 앞두고, 미리 기능 같은 걸 만들어보게 되었다.

그 기능들을 만들면서, 회사에 있는 분께 피드백을 받게 되었다.

회사에서는 '사수'라는 개념이 없어서, 그동안은 코드 피드백을 받을 기회가 없었는데 이번 기회에 처음으로 코드 리뷰를 받게 된 셈이다.

 

'사수'가 꼭 있어야 하는 건 아니라고 생각하는 사람들도 있겠지만, 만약 존재한다면 확실히 도움이 될 거라는 걸 이번에 몸소 깨달았다.

내가 잘못 작성했다는 것을 스스로 깨닫기는 쉽지 않은데, 누군가 방향을 제시해 주면서 점점 그 목적지에 가까워지는 것 같다는 느낌을 받았기 때문이다.

 

처음에 짰던 코드가 조건문 사용이 너무 많아서 너무 부끄럽지만, 여러 번 피드백받으면서 코드 피드백이 된 것 같기도 하고?

앞으로도 코드를 짤 때 어떤 생각과 마음으로 작성을 해야 할지, 스스로 생각해 보게 되는 계기가 되어 블로그에 기록으로 남긴다.

 

물론 경력자들이 보시기에는 너무 간단한 코드이겠지만..

그 간단한 코드에서도 기능을 유지하며, 코드를 깔끔하고 가독성 있게 작성할 수 있는지 다시 생각을 해볼 수 있게 되었다.

 

무엇보다 나에게 피드백을 주신 분 덕분에 나의 수준에 대해 깨닫게 되고, 더 분발해야겠다고 느꼈다.

한편으로는 속상하기도 했다.

(물론) 나는 못하겠지만, 나름 열심히 공부한다고 생각.. 은 했었는데, 그 열심히가 과연 실제 퍼포먼스에 영향을 줄 정도인가.. 에 대한 생각말이다.

 

 

아무튼 아래는 서비스 기준이다.

사실 간단한 설명이 없어도, 코드를 본다면...

코드 변천사를 알아챌 수 있을 것 같다.

아래는 즐겨찾기를 추가/해제 하는 기능에 관한 것이다.

 

 

 

1.

    // 즐겨찾기 추가 및 삭제
    public void boardFavorite(Long boardId, Boolean enabled) {

        // board_id 및 member_id 정보 조회
        Board board = boardRepository.findById(boardId).orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Member member = memberRepository.findById(principal.getMemberId()).orElseThrow(() -> new IllegalArgumentException("Not Found Member"));
        Long getBoardId = board.getId();
        Long getMemberId = member.getId();

        BoardFavorite boardFavoriteBoardAndMember = boardFavoriteRepository.findByBoardAndMemberId(board, getMemberId);
        BoardFavorite boardFavoriteSort = boardFavoriteRepository.findFirstByMemberIdOrderBySortDesc(getMemberId); // sort 기준으로 정렬

        Long sortNum = 1L;
        if (!ObjectUtils.isEmpty(boardFavoriteSort)) {
            sortNum = boardFavoriteSort.getSort() + 1L;
        }

        if (Boolean.TRUE.equals(enabled)) {
            if (boardFavoriteBoardAndMember == null) {
                BoardFavorite boardFavorite = new BoardFavorite(board, getMemberId);
                boardFavorite.setSort(sortNum);
                boardFavoriteRepository.save(boardFavorite);
                log.info("즐겨찾기 추가 성공!!!!!");
            } else if (boardFavoriteBoardAndMember != null) {
                throw new IllegalArgumentException("이미 즐겨찾기가 추가되어 있습니다.");
            }
        }

        if (Boolean.FALSE.equals(enabled)) {
            if (boardFavoriteBoardAndMember != null) {
                boardFavoriteRepository.delete(boardFavoriteBoardAndMember);
                log.info("즐겨찾기 해제!!!!!");
            } else if (boardFavoriteBoardAndMember == null) {
                throw new IllegalArgumentException("이미 즐겨찾기가 해제되어 있습니다.");
            }
        }
    }

파라미터로 `boardId`랑 `enabled`를 받는다.

굳이 저렇게 Boolean.TRUE.equlas 라고 적을 필요가 없었는데, 나도 왜 저렇게 적었는지 모르겠다...

저 때 당시에는 postman으로 테스트했을 때, boolean 값이 인식이 잘 되질 않아서, 저렇게 했었던 것 같다.

거기다가 중복 if문을 사용해서, 그 안에서 테이블에 null 값이 있는지 여부를 또 체크했다.

그리고 또 다른 if문을 사용해서 false를 체크했다.

else로 쓰면 될 것을 굳이 굳이 또 다른 if문으로 만들 필요가 있었나?라고 지금 와서 다시 생각을 해본다.

 

 

 

2.

    // 즐겨찾기 추가 및 삭제
    public void boardFavorite(Long boardId, Boolean enabled) {

        // board_id 및 member_id 정보 조회
        Board board = boardRepository.findById(boardId)
                .orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Member member = memberRepository.findById(principal.getMemberId())
                .orElseThrow(() -> new IllegalArgumentException("Not Found Member"));
        Long getBoardId = board.getId();
        Long getMemberId = member.getId();

        BoardFavorite boardFavoriteBoardAndMember = boardFavoriteRepository.findByBoardAndMemberId(board, getMemberId);
        BoardFavorite boardFavoriteSort = boardFavoriteRepository.findFirstByMemberIdOrderBySortDesc(getMemberId); // sort 기준으로 정렬
        Long sortNum = !ObjectUtils.isEmpty(boardFavoriteSort) ? boardFavoriteSort.getSort() + 1L : 1L;

        if (enabled) {
            if (boardFavoriteBoardAndMember == null) {
                BoardFavorite boardFavorite = new BoardFavorite(board, getMemberId);
                boardFavorite.setSort(sortNum);
                boardFavoriteRepository.save(boardFavorite);
            } else  {
                throw new IllegalArgumentException("FavoriteBoard already exists");
            }
        } else {
            if (boardFavoriteBoardAndMember != null) {
                boardFavoriteRepository.delete(boardFavoriteBoardAndMember);
            } else  {
                throw new IllegalArgumentException("FavoriteBoard already deleted");
            }
        }
    }

위 코드랑 뭐가 달라진 것이냐 하면?

 

        BoardFavorite boardFavoriteSort = boardFavoriteRepository.findFirstByMemberIdOrderBySortDesc(getMemberId); // sort 기준으로 정렬

        Long sortNum = 1L;
        if (!ObjectUtils.isEmpty(boardFavoriteSort)) {
            sortNum = boardFavoriteSort.getSort() + 1L;
        }

정렬에 대한 조건문 부분이 달라졌다.

 

        BoardFavorite boardFavoriteSort = boardFavoriteRepository.findFirstByMemberIdOrderBySortDesc(getMemberId); // sort 기준으로 정렬
        Long sortNum = !ObjectUtils.isEmpty(boardFavoriteSort) ? boardFavoriteSort.getSort() + 1L : 1L;

if문으로 했던 것을 삼항연산자를 사용해서, 한 줄로 줄였다.

 

if문을 너무 많이 사용하고 있다는 피드백을 받았다.

삼항연산자를 이용하거나, 다른 방법으로 코드를 줄여서 가독성 있게 만들 수 있는 것에 대해 고민을 항상 해야겠다는 생각이 들게 된 계기였다.

 

그리고 1.에서 if문을 2개를 사용했던 것을 if-else 로 변경했다.

그리고 참인 값에 대해서는 Boolean.TRUE.equals 이 형태를 제거하고, 단순하게 Boolean 값만을 넣었다.

 

 

 

3.

    // 즐겨찾기 추가 및 삭제
    public void boardFavorite(Long boardId, Boolean enabled) {

        // board_id 및 member_id 정보 조회
        Board board = boardRepository.findById(boardId).orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Long memberId = principal.getMemberId();

        BoardFavorite boardFavoriteBoardAndMember = boardFavoriteRepository.findByBoardAndMemberId(board, memberId);
        BoardFavorite boardFavoriteSort = boardFavoriteRepository.findFirstByMemberIdOrderBySortDesc(memberId); // sort 기준으로 정렬
        Long sortNum = !ObjectUtils.isEmpty(boardFavoriteSort) ? boardFavoriteSort.getSort() + 1L : 1L;

        if (enabled) {
            if (boardFavoriteBoardAndMember == null) {
                BoardFavorite boardFavorite = new BoardFavorite(board, memberId);
                boardFavorite.setSort(sortNum);
                boardFavoriteRepository.save(boardFavorite);
            } else  {
                throw new IllegalArgumentException("FavoriteBoard already exists");
            }
        } else {
            if (boardFavoriteBoardAndMember != null) {
                boardFavoriteRepository.delete(boardFavoriteBoardAndMember);
            } else  {
                throw new IllegalArgumentException("FavoriteBoard already deleted");
            }
        }
    }

2번째 피드백을 받은 후에 또 수정을 진행했는데!

사실 서비스 쪽보다도 다른 부분을 많이 수정했었다.

근데 이 게시물에는 service에 대한 것만 올리고 있기 때문에!

그냥 이 시점에 수정한 것에 대해서만 적는다.

 

        Board board = boardRepository.findById(boardId)
                .orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Member member = memberRepository.findById(principal.getMemberId())
                .orElseThrow(() -> new IllegalArgumentException("Not Found Member"));
        Long getBoardId = board.getId();
        Long getMemberId = member.getId();

이때는 `memberId` 조회하던 것을 제거했다.

해당 repository에서 조회를 할 필요가 없어 보였다.

 

        Board board = boardRepository.findById(boardId).orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Long memberId = principal.getMemberId();

사용자가 로그인은 하지만, 각자 자신의 페이지만 보는 상황이다.

그래서 굳이 db에서 해당 사용자를 조회를 할 필요가 없다.

그래서 로그인 사용자에 대한 정보만 가져오면 된다고 생각되어, 이 부분을 수정했다.

 

 

 

---

구분선을 넣은 이유는 위까지는 즐겨찾기 추가/해제 기능을 하나의 메소드에서 구현을 했다면!

아래부터는 즐겨찾기 추가/해제 메소드를 분리를 했다.

그래서 구분선으로 구분을 했다.

 

4.

   // 즐겨찾기 추가 및 삭제
    public void boardFavorite(Long boardId, Boolean enabled) {

        // board_id 및 member_id 정보 조회
        Board board = boardRepository.findById(boardId).orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Long memberId = principal.getMemberId();

        BoardFavorite boardFavoriteBoardAndMember = boardFavoriteRepository.findByBoardAndMemberId(board, memberId);
        BoardFavorite boardFavoriteSort = boardFavoriteRepository.findFirstByMemberIdOrderBySortDesc(memberId); // sort 기준으로 정렬
        Long sortNum = !ObjectUtils.isEmpty(boardFavoriteSort) ? boardFavoriteSort.getSort() + 1L : 1L;

        if (enabled) {
            if (boardFavoriteBoardAndMember == null) {
                BoardFavorite boardFavorite = new BoardFavorite(board, memberId);
                boardFavorite.setSort(sortNum);
                boardFavoriteRepository.save(boardFavorite);
            } else {
                throw new IllegalArgumentException("FavoriteBoard already exists");
            }
        } else {
            throw new IllegalArgumentException("FavoriteBoard already exists or deleted");
        }
    }

    public void deleteBoardFavorite(Long boardId) {

        Board board = boardRepository.findById(boardId).orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Long memberId = principal.getMemberId();

        BoardFavorite boardFavoriteBoardAndMember = boardFavoriteRepository.findByBoardAndMemberId(board, memberId);

        if (boardFavoriteBoardAndMember != null) {
            boardFavoriteRepository.delete(boardFavoriteBoardAndMember);
        } else {
            throw new IllegalArgumentException("FavoriteBoard already deleted");
        }
    }

즐겨찾기 추가와 해제에 대한 메소드를 위처럼 분리했다.

`boardFavorite`이 즐겨찾기 추가이고, `deleteBoardFavorite`이 즐겨찾기 해제에 대한 메소드이다.

우선 여기까지는 기존 메소드에 있는 내용을 각각 분리만 한 수준이다.

여전히 if-else의 굴레에서 벗어나질 않는...

 

 

 

5.

    public void boardFavorite(Long boardId) {

        Board board = boardRepository.findById(boardId).orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Long memberId = principal.getMemberId();

        BoardFavorite existingFavorite = boardFavoriteRepository.findByBoardAndMemberId(board, memberId);

        if (existingFavorite != null) {
            throw new IllegalArgumentException("FavoriteBoard already exists");
        }
        boardFavoriteRepository.save(new BoardFavorite(board, memberId));
    }

    public void deleteBoardFavorite(Long boardId) {

        Board board = boardRepository.findById(boardId).orElseThrow(() -> new IllegalArgumentException("Not Found Board"));
        Long memberId = principal.getMemberId();

        BoardFavorite existingFavorite = boardFavoriteRepository.findByBoardAndMemberId(board, memberId);

        if (existingFavorite == null) {
            throw new IllegalArgumentException("FavoriteBoard already deleted");
        }
        boardFavoriteRepository.delete(existingFavorite);
    }

여기까지가 최종!

기존에 if-else문으로 되어있던 것을, 조건을 반대로 해서!

if문을 만들어, exception 처리를 했다.

이렇게 하니, if-else 대신에 단순히 if문만 사용하면 되니, 가독성 측면에서도 4번에 작성한 것보다 훨씬 나아 보인다.

 

1번이랑 5번이랑 단순히 코드의 줄 수만 비교해도 10줄 이상이 줄었다.

이번에 피드백을 받고 수정을 하면서, 코드를 작성할 때는 프로그램이 돌아가게만 할 게 아니라!

나와 추후에 다른 사람이 유지보수할 것을 생각해, 가독성 있으면서도 이해하기 쉬운 코드를 작성해야겠다는 생각을 했다.

이건 당장에 될 것 같지는 않고, 항상 의식하면서 노력을 해야겠다.