레거시 코드에 테스트코드 추가와 리팩터링 하기
feat. 머리에 쥐남
우리는 레거시 코드를 늘 만나게되고 수정해야한다.
그 레거시 코드에는 테스트코드가 없을 수 있고 그러면 수정하거나 리팩터링할때 매우 불안하다..
그래서 이번에 레거시 코드에 테스트코드를 추가하면서 리팩터링하는 방법을 정리해보려 한다.
진행할 예제는 stingleton, static method call을 가지고 있어서 testing하기 어려운 코드이다.
그리 어렵지 않으니 한번 따라해보는것도 좋을듯 하다(원문).
(중간 중간 오타나 코드에 이상한 부분이 보여도 진행하는 방법에 중점을 두고 보시면 되겠습니다.)
레거시 코드를 다룰때 팁은 1. 리팩터링은 Deepest Branch에서 Shortest Branch로 나오면서 진행하는것이고 2. 테스트코드 추가는 Shortest Branch에서 Deepest Branch로 들어가면서 진행하는것이다.
리팩터링을 Deepest Branch에서 시작하는 이유는 어디에도 의존성을 갖지 않으므로 시작점이 되기에 충분하기 때문이다. 테스트코드를 Shortest Branch에서 진행하는 이유는 테스트하기에 가장 편한 상태를 발견하기 쉽기 때문이다.
1. 첫 번째 Shortest Branch 찾기
이번에 진행할 예제이다. Shortest Branch를 보니 바깥의 else가 보인다. 빠르게 테스트코드를 넣어 볼 수 있을것처럼 예상이된다.
2. 빠르게 테스트 추가
??? 테스트가 실패하여서 보았더니..'DependendClassCallDuringUnitTestException' 원치않는 의존성으로 테스트 수행에 실패하였다.
바로 이부분 때문이다. Singleton을 mocking 할 수 없다.
이럴때 seam을 생성한다. seam은 만나는 지점을 만들어주는것인데, 해당 코드에서는 TripService와 UserSession이 만나는 지점이다. 평소에도 의존관계때문에 테스트하기가 어렵거나 끊어내고 싶을때 이렇게 시작하는 습관을 가져보자.
3. UserSession.getInstance().getLoggedInuser()를 Protected접근제한자로 Extract Method해보자
protected로 한 이유는 무엇일까? 바로 테스트코드에서 TripService를 상속받아 테스트를 위한 TripService클래스를 만들고 해당 메소드를 override를 하기 위해서이다.
이렇게 우선 테스트가 돌아가도록만 설정해주었다. 일단 테스트가 문제없이 돌아가게 하는게 우선이다.
이렇게 하면 이제 테스트가 동작하는데 coverage를 조사하면 우리가 찾았던 Shortest Branch에 대해서 테스트가 cover함이 보인다.
그리고 테스트코드에서 logged in user와 명확성을 위한 리팩토링을 진행한다.
4. 두 번째 Shortest Branch를 찾아보자
code coverage를 보면 두 번째 Shortest Branch가 무엇인지 보이는데 예제에서는 logged in user이지만 친구가 없는경우이다. 빠르게 테스트 코드를 추가해보자.
테스트코드도 코드이기에 만들며 중복도 제거하고 코드를 가독성 좋게 유지하도록 하자
5. 세 번째 Shortest Branch를 찾아보자
coverage를 수행하여 다음 branch를 찾아보자. 생긴것을봐도 그렇고 서비스 코드를 이해해봐도 이부분이 제일 Deepest Branch이고 이제는 이것을 cover하면 된다. 테스트를 추가해보자
또 나왔다.. 원치 않는 의존성(static method call)으로 테스트가 실패했다.
가능하게하려면? 아까 위에서도 말했떤 seam을 만들어보자.
TripDAO.findTripsByUser를 extract method한다.
그리고 위에서 처럼 테스트코드를 통과시키기위한 작업을 진행한다.
테스트가 다 통과되었다.
seam을 생성하기 위해 extract method로 생성한 아래 2개의 메소드를 제외한 긍정적인 coverage를 얻었다.
이제 테스트가 있으니 잠시 쉬면서 자그마한 안도감과 함께 중복제거등 테스트코드를 정리해보자
User 만드는 부분이 중복이 많고 지저분하니 Builder를 이용해서 정리를해보자
함수의 인자 수는 다음과 같이 3가지가 허용되는데 0개, 1개 그리고 many(ex: withTrips(Trip... tirps))이다. 또한 메소드 이름과 파라미터를 combine하면 가독성이 높아 질 때도 있다(ex: addTripsTo(user)).
한결 깔끔해졌다. 이제 리팩터링으로 넘어가 보자.
5. 리팩터링을 위해 Deeptest Branch를 찾아보자.
Deepest Branch는 tripList = getTrips(user)이지만 위임이외는 별로 하는일이 없다. 그러면 두 번째 Deepest Branch인 if(friend.equals(loggedUser))가 포함된 loop를 살펴보자.
보통 메소드가 클 경우 하나 이상의 책임을 가지고 있을 수 있다.
그래서 무엇보다 우선해서 이 행위/책임이 이 메소드에 속하는지 살펴보자.
user 객체에서 friends를 뽑아와서 loggedUser와 친구인지를 조사하게되는데 이렇게 된 부분은 디미터 법칙을 위반했다고 볼 수 있다. 즉 User객체의 캡슐화가 깨진것 이다.
디미터 법칙은 협력하는 객체의 내부구조에 대한 결합으로 인해 발생하는 설계문제를 해결하기 위해 제안되었고 '낯선 자에게 말하지마라' 또는 '오직 인접한 이웃하고만 말하라'로 요약 할 수 있다. 디미터 법칙을 따르면 부끄럼 타는 코드를 작성할 수 있는데 이는 불필요한 어떤 것도 다른 객체에게 보여주지 않으며, 다른 객체의 구현에 의존하지 않는코드를 말한다. 여기에 더불어 '묻지 말고 시켜라'와 '의도를 드러내는 인터페이스' 기법을 함께 적용하면 좋은 코드를 만들 수 있다. 다시 한번 말하지만 디미터 법칙을 위반하는 설계는 '인터페이스와 구현의 분리 원칙'을 위반하고 그 내부 구현을 노출하는것을 의미하니 객체 사이에 불필요한 결합도를 높이지말자.
이 예시에서는 위반한 부분을 메소드 추출하고 테스트코드를 추가해보자.
6. 하나의 책임을 위반한 부분 리팩터링
테스트 코드를 먼저 추가해보자. 메소드명은 isFriendsWith(user)로 하였다.
이후 테스트를 돌아가게 하기위해 실제 코드를 추가 하였다.
리팩터링될 부분을 잠시 커멘트 처리하고 새로운 메소드를 테스트해보자.
테스트코드가 무사히 돌아가면 커멘트 처리한 부분은 삭제하고 코드를 정리한다.
7. 잠시 코드를 정리
guard clause를 메소드 처음부분으로 이동하여 가독성을 높혀보자.
이렇게 보니 이 메소드는 2개의 블록으로 구성된다. 첫 번째는 user가 valid한지 조사하고 두 번째는 user에 대한 trip을 구한다.
불필요한 변수를 없애본다. 이때 먼저 else를 추가하고, 테스트에 문제가 없을 때 변수를 없애고 바로 return 하도록 점진적으로 진행한다. Infinitest가 주요 역할을 한다.
ternary operator로 변경하였다.
loggedInUser를 inline하면서 변수를 없애보았다. getLoggedInUser를 2번 호출 하지만 깔끔해보인다.
new ArrayList<>()를 반환하던 부분은 noTrips로 extract method하여 가독성을 높혔다.
7. 테스트코드와 프러덕션 코드의 의미 비교
테스트 메소드를 읽어보면 프러덕션 코드의 의미와 완전히 동등하다. 영어로 테스트 메소드명을 작성한다면 프로덕션에서도 함께 사용가능해서 가독성을 더 높일수도 있을 듯 하다.
끄읏~~~!! NO! ???????
위 코드에는 설계상의 문제가 존재한다.
TripSerivce는 서버 사이드 클래스인데 UserSession(웹에 종속적인)에 의존성을 가진다.
이를 풀어내야한다.
이 문제의 해결법중 하나는 loggedInuser를 주입받으면 된다.
loggedInUser를 파라미터로 추가하고 테스트를 수정해서 동작하도록 하자.
그리고 getLoggedInUser메소드를 제거하자.
8. 종속성제거
9. 정적 메소드에 대한 의존성 제거
TripDao.findTripByUser로의 static call이다. 이것도 DI 해야한다.
해오던 것처럼 TripDAO에 테스트코드를 추가할 것인데 이를 TDD로 추가한다.
(참고: 현실과 다르지만 이상으로 static method는 좋지 않다고 생각한다. 이는 명령형스타일이고 OOP 스럽지않은데 그 이유는 다형성에 좋지않고 코드블록사이의 의존성을 끊기 힘들어진다. 또한 표현력에서도 떨어지며 응집도도 떨어지게 만든다. 객체와 static method를 혼합해서쓰면? 아마 그 코드는 명령형 스타일을 따르게 될것이다.)
원하는 바를 테스트에 기술하고 이것이 동작하도록 코드를 만든다.
코드를 주입받도록 프러덕션 코드도 수정하였다.
serice에서 static method가 아니라 insatnce method를 사용하도록 하고, spring autowire로 dependency가 inject되도록 변경하였다.
테스트코드도 점전적으로 TripService를 TestingTripService가아니라 리얼 TripService로 변경하면서 테스트를 수행하여 정합성을 유지하자.
이렇게 mock에 대해 expectation을 기술하며 테스트를 진행하면된다.
결국 TestingTripService는 이제 필요 없어졌다. 삭제하자.
10. 프러덕션 코드 마무리 정리
TestingTripService가 없어지면서 trips는 private로 바꿀수 있게 되었고 validate로직도 extract method하였다.
이렇게 레거시코드에 테스트코드를 추가하고 리팩터링하는 방법을 알아보았다.
테스트 코드와 리팩터링은 변경에 용이하며 유지 보수좋고 가독성 좋은 코드를 만드는데 있어서 필수 존재라고 생각한다. 필자도 잘하지 못하기에 연습을 꾸준히 하려하지만 어려운것은 어쩔수 없다.
특히 리팩터링이나 설계는 법칙이라고 불리는것들도 상황과 판단에 따라 다르기에 정답도 없다. 하지만 우리는 각자의 상황에 맞춰서 최선을 찾기위해 노력해야한다고 생각한다.
We will find a way. We always have!
