리팩토링
규칙
TDD와 클린코드 그리고 다른 서적들을 읽고, 블로그나 아티클을 찾아서 정리하고 팀원과 공유하여 리팩토링에서 나름의 규칙을 정해보았다.
- 절대 작동하는 것이 정상 작동하게 해야한다.
- 1번을 무조건 지킨다.
- 성능을 개선하기위해 노력한다.
- 클린코드를 지키기 위해 노력한다.
- 코드리뷰를 꼼꼼하게 본다.
2번은 단순한 강조가 아닌 모든 리팩토링 기능 구현이 마치면 전체 테스트를 실행하는 것이다. 이 방법으로 1번을 무조건 지킬 수 있도록 만든다.
그럼 규칙을 생각하며 리팩토링을 시작해보자
적용
이전 코드의 끝에서 일단 가장 크게 보이는게 categoryId가 1인 경우의 if문의 반복문 로직이 else 부분 한개의 포스팅에만 리뷰를 업데이트하는 부분과 중복이 일어나는 것이 눈에 보인다.
(파란 부분)
이것을 일단 묶어주어 중복을 해결해준다.
메소드 분리로 어느정도 깔끔해져 보인다.
하지만 메소드의 파라미터가 너무 많다고 생각되어 혼란을 야기할 수 있어보인다. 일단 다시 원래대로 돌리고, 간단하면서 작은 것부터 다시 시작해보자.
빨간 네모 칸의 카테고리가 공부 카테고리인지 명확히 만들어준다.
어떤 조건을 검증하는 것인지 명확하게 하기 위해 메소드 이름을 isStudyCategory로 정했고, 1 상수를 static final int STUDY_CATEGORY_ID로 만들어 공부 카테고리 아이디를 좀 더 명확히 했다.
다음으로 넘어가서 reviewCount를 유심히 봐보자
reviewCount는 reviewList가 add 될 때마다 1씩 더해지고있다. 언제부터 잘못된지 모를 레거시이고, 다른 일정이 있다는 이유로 리팩토링에 소홀하고 코드리뷰를 그냥 넘어가서 생긴 괴물이다.
당장 없애주도록 하자.
코드를 수정하고 정상 동작하는지 테스트를 전체 돌려보아 확인해본다.
문제가 없으니 넘어간다.
다음으로 눈에 띄는건 reviewList를 반복문에서 save가 반복되는 것
saveAll() 로 바꿔주면 성능면이나 시간적인 측면으로 모두 이로울 것으로 보여진다.
그래서 위와 같이 변경해준다.
전체 단위 테스트 실행시 모두 초록불을 확인하고 다음으로 넘어간다.
자세히 코드를 들여다 보면
Post의 reviewList에서 reviewBuilder를 추가했을때, reviewBuilder는 save 되기전이어서 id를 가지고 있지 않다.
이 부분을 고쳐보자.
(실무 코드에서는 이런 문제가 있지는 않지만, 임의로 코드를 만들다보니 생긴 일이니 양해 부탁드립니다.)
Post update 부분은 reviewRepository.saveAll() 이후에 나오는게 들어맞아보인다.
그래서 결과적으로 바꿔보면
이렇게 되는데 중간 과정이 좀 많이 바꼈다.
일단 mapCategoryAndPostRepository.getByCategory() 가 중복 되어 if문 밖으로 빼고, 공부 카테고리인 경우에는 댓글과 포스팅에 saveAll()을, 그 밖의 카테고리는 단일 save()로 변경했다. (ReviewBuilder에 post 내용을 빠뜨렸어서 추가했다.)
그리고 리팩토링을 했으니 전체 테스트를 실행시켜보면
???
침착하게 내용을 보면
Assert 부분에 값이 예상과 다른 문제다. 좀만 생각해보면
해당 부분의 Mock 처리를 하지 않아서 생긴일이다.
Mock을 만들어 주고, 실행하면
이 문제는 Post의 reviewList가 null인데 add를 해서 생기는 문제였다. 그래서
빈 reviewList를 만들어주니 해결되었다.
이제 중복을 제거하고 추상화 수준을 맞춰주자.
먼저 가장 밑에 쉽게 중복을 찾을 수 있는 post의 reviewList를 업데이트 하는 부분을를 메소드로 묶어주자
그리고 공부 카테고리에 postRepository.saveAll까지 한번에 묶어주면
post 저장 부분이 깔끔해졌다. 이제 review 저장 부분을 깔끔하게 만들어보면
좀 더 깔끔하게 만들어주고, 공부 카테고리에 reviewRepository.saveAll을 깔끔하게 만들어주면
이런 결과를 얻을 수 있다.
이제 추상화 레벨을 일치시키면
이런 결과를 얻을 수 있다.
그리고 마지막으로 순서대로 보기 편하도록 메소드 위치를 조정해준다.
TDD의 대한 내 생각
모든 가능성을 테스트 하는 것은 불가능하다. 하지만 TDD는 개발하는데에 있어 이로운 점이 더 많다.
내가 생각한 TDD의 강점은 4가지이다.
1. 계획적으로 일을 시작할 수 있다
2. 일에 대한 단위를 구분 지을 수 있고, 목적에 맞는 코드만 작성해 불필요한 코드를 줄인다
3. 리팩토링을 필수로 하기에 좀 더 코드가 유지보수가 용이해진다
4. 초록불을 봤을때 전투력이 올라간다는 점이다.
처음 켄트 백의 테스트 주도 개발에서 초록불이 자신감을 올려준다는 얘기를 가벼운 농담으로 인식했었는데, 이를 실제로 사용해보면서 개발해보니 느껴졌다. 뭔가 잘 되간다는 느낌까지도 든다. TDD 사용 이전에는 개발을 진행하면서 이전에 오류가 생길지도 모른다는 불안감이 알게모르게 있었는데 이 점은 TDD와 초록불을 확인을 통해 일정 부분 상쇄 되는것 같다.
하지만 TDD를 회의적으로 보는 많은 사람들의 생각은 흘려들으면 안된다고 생각하고, 나도 이해된다.
시간이 많이 소요되고, 모든 가능성을 테스트 코드로 만들 수 없는 노릇이다. 그래서 많은 노력과 수련을 통한 체득이 이뤄져야할 것이고, 단위 테스트를 통한 확인으로 적어도 해당 오류의 반복은 상당수 막을 수 있다는게 내 결론이다.
하지만 아직도 어느정도 단위로 테스트를 만들어야 효율적이고, 일정을 잘 지킬 수 있는 방법인지는 끊임없이 고민하고, 연습해 보아야겠다.
