코드를 읽다가 같은 일을 두 방식으로 하고 있는 것을 발견했다. find() 뒤에 직접 null을 확인해 예외를 던지는 쪽과 자체 헬퍼 getOrFail()을 쓰는 쪽이 수십 대 수십으로 섞여 있었다.
Table of contents
Open Table of contents
원인 — 시기가 만든 두 갈래
이력을 보니 시기가 달랐다. getOrFail()이 없던 시절 코드가 앞의 방식이고 생긴 뒤 코드가 뒤의 방식이었다.
새 방식이 생겼는데 옛 코드를 안 바꾼 것이고 틀린 것이 아니라 시기가 다른 것이었다. 어느 쪽이 잘못됐는지를 묻기 전에 왜 둘인지를 먼저 봐야 판단이 선다.
조치 — 정하고 점진적으로 옮기기
getOrFail() 쪽으로 통일하기로 하고 그 이유를 적었다. 짧고 예외 처리가 한 자리에 있으며 실수로 확인을 빠뜨릴 수 없다는 것이다.
두 가지 방법이 있었는데 옛 것을 한 번에 다 바꾸는 것과 그 파일을 고칠 때 같이 바꾸는 것이다. 뒤쪽을 골랐다. 이유는 한 번에 바꾸면 안 건드려도 될 파일까지 배포 대상이 돼서다. 대신 두 방식이 반년 넘게 공존하는 것을 감수했다. 새 코드는 새 방식으로 쓰고 고치는 코드는 옮기며 안 고치는 코드는 그대로 뒀더니 반년 뒤 수십이 몇으로 줄었다.
판단 기준 — 통일할 값이 있는 것
다른 갈래도 찾으니 Carbon 날짜 형식과 foreach·array_map 순회와 implode 문자열 결합과 오류 처리와 Log:: 다섯이 나왔다. 전부 통일하지 않고 통일할 값이 있는 것만 했다.
섞이면 동작이 달라지는 것과 부르는 쪽이 헷갈리는 것과 하나가 실수를 유발하는 것을 통일 대상으로 삼았다. Log::는 한쪽이 수집이 안 되고 오류 처리는 섞이면 호출부가 헷갈리므로 통일했고, foreach와 implode는 취향 차이라 안 건드렸다. 취향 차이를 통일하려면 시간이 많이 들고 얻는 것이 적다.
설정 — 자동으로 잡는 것과 사람이 보는 것
관례를 문서에 적고 금지한 것은 phpcs 규칙으로 CI 빌드에서 걸리게 했다. 권장하는 것은 자동으로 못 잡으므로 검토에서 봤다.
옛 방식을 새로 쓰는 것도 find() 뒤 null 확인 패턴으로 막았다. 막지 않으면 고치는 만큼 새로 생겨서 숫자가 줄지 않는다. 남은 옛 방식 개수를 grep -c로 월 1회 세어 기록했더니 줄어드는 것이 보였고 석 달 동안 안 줄어든 파일은 아무도 안 고치는 파일이라 따로 시간을 내서 바꿨다. 숫자를 안 세면 언젠가 다 바뀌겠거니 하고 끝난다.
교훈 — 혼자 정하지 않기
처음에는 혼자 정하고 문서에 적었는데 다른 사람이 모르고 옛 방식을 계속 썼다. 문서에 적는 것만으로는 안 되고 같이 정하고 알려야 했다.
의견을 물으니 한 명이 여러 건을 처리하다 하나가 없을 때는 예외를 안 던지고 건너뛰는 편이 낫다는 경우를 알려 줬다. 그 예외를 관례에 넣어 정리했고 혼자 정했으면 그 경우를 몰랐다. 정하는 자리에 있었던 사람은 그 뒤로 그 방식을 썼다.
정리
- 같은 일을 두 방식으로 하고 있으면 시기가 다른 것일 수 있다
- 하나로 정하고 이유를 적는다
- 한 번에 다 바꾸지 말고 고칠 때 같이 바꾼다
- 통일할 값이 있는 것만 한다
- 섞이면 동작이 다르거나 실수를 유발하는 것을 통일한다
- 취향 차이는 안 건드린다
- 금지는
phpcs로 자동으로 잡고 권장은 사람이 본다 - 남은 개수를 세지 않으면 언제 끝날지 알 수 없다