약 1000줄에 가까운 코드 300줄로 줄인 후기
내가 지금 회사에 존재하기 약 1년전, 백엔드인 그는 프론트엔드 코드를 짜달라는 지시를 받게 되는데…
백엔드 개발자:
리팩토링의 필요성(그냥 다시 짜세요)
지금 회사에 레거시 코드가 종종 보이고, 어느 부분은 이러저런식으로 고쳐도 좋겠다 싶은 구간들이 조금씩 있다. 그런 부분들은 어느 회사에가도 있을법한 그런 소소한 부분들이고, 유지보수하는데 문제가 전혀 없는 정도이다.
그런데 결코 건드릴 수 없을정도로 어지러운(진짜임) 컴포넌트가 하나 있었다. 약 1000줄정도에 비슷한 변수명과 함수명만 3~4개 정도돼서 유지보수 하기도 너무 어려웠다. 그런것치고 기능도 잘 동작하고 크게 문제가 없는(없다고 믿고 싶은) 곳이라 따로 건드릴 생각이 없었는데.. 마침 이번에 고객사에서 해당 부분에 문제가 있는 것 같다고 연락이 왔다. 그래서 찬찬히 살펴보는데 생각보다 문제가 컸다. 다른 고객사가 말없이 사용해준것이 감사할만큼..
암튼 자의반 타의반(타의 90%)으로 무덤까지 가지고 가고 싶었던 아름다운 코드를 리팩토링을 하게 되었다.
리팩토링은 무조건 좋은 것이라고 생각했다.
개발자에게 리팩도링은 필수적인 요소가 맞긴 하다. 내가 어제 짠 코드를 조금 더 읽기 쉽게 리팩토링하는 것은 나에게도 좋지만 동료 개발자들에게도 좋은 일이다. 선임 개발자의 조언을 듣고 하는 리팩토링은 내가 발전할 수 있는 계기가 된다. 그런데 이번에는 생각이 조금 짧았던 것 같다. 이미 엉망이된 과거 코드를 고칠때는 리팩토링이 무조건 좋은 것은 아님을 리팩토링을 하겠다고 수락한 이후 3일 동안 쳐다만 보면서 깨달았다.(이런 상황에서는 리팩토링을 하는 것보다 새롭고 깔끔하게 다시 짜는것이 정신건강에 좋을 것 같다는 말을 과거의 나에게 해주고 싶다..)
뭔가 재밌을 것 같고, 뿌듯할 것 같고, 성장 할 것 같고…
사실 옆에서 해당 부분이 어떻게 개발되었는지 지켜본 선임개발자가 차라리 처음부터 다시 짜라고 했으나, 초큼의 귀찮음 + 스스로에게 도전과제 부여 같은 느낌으로 그냥 리팩토링을 했다. 이걸 계기로 어지러운 코드 리팩토링 경험도 가져가고 콘텐츠도 뽑아볼 수 있을 것 같다는 생각도 들었다.
그런데, 리팩토링 하기 전에 유지보수 할 때부터 컴포넌트 내부를 아무리 쳐다봐도 이해가 되지 않았어서 좀 애를 먹었다. 어느 함수가 무슨 역할을 하는지는 함수 내부를 뜯어봐도 헷갈렸다. 여기저기 흩어져있는 변수 선언과 state, ref도 헷갈리게 하는데 한 몫 했다.
리팩토링 기준
이렇게 저렇게 해보면서 리팩토링을 진행했는데 의식의 흐름대로 리팩토링을 할 수는 없는 코드라서 상당히 오래 고민하고 진행했다. 3일 정도 같은 코드를 쳐다보니까 뭐 때문에 잘 안 읽히는지, 어느 부분을 개선하면 좋을지 조금 보였다. 내가 생각했을때 과거 코드에서 부족하다고 생각한 점을 적어봤다.
- 응집도
코드를 살펴보면서 가장 어려웠던게 너무 분산되어있는 코드들이었다. 분명 한 컴포넌트 안에 있기는 한데 상단에 있는 함수를 중간에서 갖다 쓰고 어디선가 필요한 데이터를 중간 어딘가에 정의해두고.. 컴포넌트가 분명 세 덩이로 있는 것 같기는 한데 뭐가 필요하고 뭐가 필요하지 않은지를 파악하는 것도 쉽지 않았다. - 중복성
변수명이랑 함수명은 그렇다쳐도 비슷한 기능을 하는 함수만 세개정도 되는 것 같았다. 결국 아주 미묘하게 다른 무언가를 바꿔주기위해서 기존 함수와 다른 새로운 함수를 만든 것 같은데 이름도 비슷하고 내용물도 어디가 다른지 찾아내기 어려워서 이떻게하면 깔끔하게 만들 수 있을지 고민했던 것 같다. - 너무 많은 로직
복잡한 요구사항을 한 컴포넌트 안에서 다 처리하고 있었다.- 현재 시간기준 약 5분 전 까지의 데이터를 polling 방식으로 가져온다.
- 가져온 데이터는 기존 데이터와 비교하여 새로운 데이터가 있는지 확인한다.
- 새로운 데이터가 있으면 그 값을 기존 데이터에 추가한다.
- 새로 들어온 데이터 중에서 특정 값의 데이터는 따로 팝업을 하나 더 띄운다.
- 팝업을 확인하면 기존 데이터에서 checked 라는 값을 true로 변경하는 api를 호출한다.
- 데이터는 일정 시간이 지나기 쩐까지 화면에 계속 보여지도록 노출한다.
이걸 다 하나의 컴포넌트에서 해결하려고하니까(심지어 백엔드에서 해줘도 되는 일도 많았다. 다시 한 번 말하지만 지금 내가 리팩토링 한 이 코드는 백엔드 개발자가 짰다..) 코드가 어지러워 진 것 같다.
위에 세 가지만 조금 개선시켜주면 유지보수도 어렵지 않게 할 수 있을 것 같았다.
리팩토링 하기(나 어지러워..)
문제점을 알았으니 차근히 코드를 뜯어고쳐 보자.
-
상태 변수 모으기
일단 흩어져있는 변수와 state 값들부터 정리했다. 중간에 갑툭튀하는 변수들 때문에 컴포넌트 안에서 사용되는 데이터가 정확하게 어떤 것인지 파악하기 어려웠기 때문이다.
그런데 모아보니 생각보다 비슷한 네이밍 뿐만 아니라 알 수 없는 네이밍도 있었다.
고요속의 (주석) 외침 이랄까.. -
중복된 함수 정리하기
이미 고객사 에러 해결 문제로 중복된 함수 1개를 정리했는데, 똑같아보이는 함수가 2개는 더 있었다.(말도 안데…)
문제점은 함수가 똑같아 보이기만 하고 기능은 미묘하게 다르다는 것이다. 어떤 함수든 잘못 정리했다가는 잘 동작하던 기능이 망가질 수도 있었다.
그래서 이 시점에서 테스트 코드를 짰다. 엄청 잘 짠 것은 아니지만 기능이 잘 동작하는지만 알 수 있도록 해두고 직접 테스트도 여러번 진행했는데 이부분이 제일 신경이 많이 쓰였던 것 같다. -
UI 분리하기
변수를 모으고 함수 정리하고 났는데도 코드가 길고 로직이 많아서 보기가 힘든 느낌이었다. 한 컴포넌트 안에서 필터 기능, 조건에 따라 특정 컴포넌트를 띄우는 기능, 데이터를 나열하는 기능 등 필요한 것들을 다~~~~~~ 하고 있었기 때문에 코드가 길고 복작했다. 거기에 UI를 위한 CSS 코드까지 합쳐지니까 스크롤을 내려도 내려도 끝이없는 코드가 나와버린 것이다.
여기서 그나마 알아보기 쉬운 UI와 CSS 분리는 쉬울 것이라고 생각했다. 새로 컴포넌트를 만들고 해당 컴포넌트에 UI와 CSS만 옮기고 짜잔~ 하고 분리하려 했으나, 아주 끈끈하게 엮여 있는 컴포넌트들 때문에 에러 파티가 벌어졌다. -
로직 분리하기
UI를 분리하면서 나는 에러를 하나하나 해결해 가면서 로직을 옮겼다. 해당 과정을 통해서 필요한 데이터와 필요 없는 데이터를 구분 할 수 있었다. -
공통 함수 모으기
리팩토링을 다 하고 코드 줄 수도 약 300줄 정도로 줄었다. 휴…
그런데 각각의 컴포넌트에서 필요한 로직을 분리하고나니 중복으로 사용되는 함수가 생겨버렸다. 분명 위에서 중복 함수를 제거했는데 필요한 로직을 각 컴포넌트에 넣는답시고 옮기다가 다시 만들어 버린것이다…(난 바보)
사실 부모에 작성하고 부모에서 props로 넘겨주면 되긴 하는데 그러기엔 부모가 너무 비대해지고 이미 props로 넘겨주고 있는 데이터가 너무 많아서 자식으로 위치를 옮기다 보니까 그렇게 된 것이다. 이런 아이들은 부모로 다시 옮기기보다는 커스텀훅으로 빼주는게 좋다고 생각했다. 그리고 이왕 빼는김에 각 컴포넌트의 로직을 커스텀훅으로 옮겨서 UI와 로직도 분리해주는 주니 나름 깔끔한 컴포넌트로 재탄생 했다.
엄청 드라마틱하게 변한건 아니지만 기존 코드 잘 동작 하고, 있던 버그도 사라졌고, 유지보수도 가능해졌다.
그런데 해놓고 보니 막 깔끔하거나 읽기 쉬운 코드가 아닌 것 같아서 완벽한 리팩토링이란 존재하지 않는것은 아닌가 싶기도하고,, 난 아직 멀었나 싶기도 하다.
다음 기회에 더 잘해보는 걸로..