[프로그래밍] Bad Smell을 가진 나쁜 코드에 대한 고찰

경력이 그렇게 오래된 것은 아니지만 코드 리뷰, 프로젝트 유지 보수를 하면서 느낀 경험에 따라 코드에 대해 좋고 나쁨을 판단하는 기준이 생겼다. 나만의 나쁜 코드에 대한 기준을 정리하고자 이 글을 쓴다. 피드백을 주시면 더욱 좋을 것 같다.

(주로 쓰는 언어가 자바이고 안드로이드만 한 상태라 자바, 안드로이드 관련해서 정리함)

1. 너무 긴 코드

- 한 메소드 혹은 클래스 안에 너무 많은 내용이 들어가 있는 상태인 경우가 대부분임

- 메소드라면 해당 안의 기능을 따로 메소드로 빼내야할 필요성이 있음

- 클래스라면 혹시 너무 많은 책임을 담당하고 있지 않은지 검토할 필요성이 있으며 필요하다면 분리할 책임들을 담당할 클래스를 만들어야 함

2. if ~ else if ~ else if~ ..... else

- 이런 코드들은 볼 때마다 가끔씩 내가 어느 경우를 보고 있는 건지 헷갈릴 때가 있음(보통은 저 안에 또 여러 줄의 코드가 있어 가독성이 떨어짐)

- 쓸모없는 조건문이 있는지 확인해서 있다면 제거하는 것이 좋음

- 만약 switch case문으로 바꿀 수 있다면 바꾸는 것이 좋을 것 같음

- 또한 그 안에서 처리하는 부분은 따로 함수로 빼내면 가독성 향상에 도움이 될 수 있음

3. 중복되는 코드

- 같이 일하는 사람들을 엿 먹이기 딱 좋은 코드들 중 하나라고 생각함

- 중복되는 코드는 나중에 수정하는 거 자체가 배보다 배꼽이 더 큰 일이므로 무조건 제거하는 것이 좋음

- 제거하는 방법은 메소드로 추출하거나 클래스로 추출 혹은 상속 등으로 해결하면 됨

4. {}이 중첩되는 코드

- 이런 코드를 짰다고 해서 틀린 것은 아니지만 그렇다고 가독성이 좋다고는 볼 수 없는 것 같음({} 안을 보다가 의식이 혼미해짐을 느낄 수 있고 막상 짠 사람한테 물어봐도 의식이 같이 혼미해지는 경우도 종종 보임)

- 최대한 중첩이 되지 않도록 짜는 것이 좋고 가독성을 위한다면 많아야 1~2번 정도가 적당하다고 생각함

- if문의 경우 미리 걸러내야하는 경우에서 return, continue, break 등으로 걸러내고 정상적인 프로세스를 돌리는 형태로 {} 중첩을 벗어날 수 있음

- for문이나 while문과 같은 반복문의 경우 함수로 따로 추출해서 적당히 명명하는 것이 좋아보임

5. 변수, 메소드명이 적절치 않은 코드

- 메소드명이 명사이거나 변수명이 동사이거나 이런 경우가 있음(직접 번역하면 뭔 말인지 모르는 경우가 많고 이걸로 놀림당할 수도 있음)

- 아예 의미가 없는 경우도 있음(솔직히 이런 코드 만나면 유지보수고 뭐고 다 때려치고 싶음)

- 메소드명에 적절한 동작을 하지 않는 경우도 태반임(예를 들면 get~~~인데 안에서 값을 바꾸고 있다던가 setA인데 정작 A이외의 다른 값들도 바꾸고 있다던가....)

- 각 언어별로 명명법들을 기본적으로 한 번은 숙지하는 것이 좋고 그 이후에 그 명명법에 맞춰 정하는 습관을 들이는 것이 필요함(처음 보는 사람들이 이런 코드들을 보면 정말 기가 차요.... ㅜㅜ)

- 메소드는 해당 메소드명에 있는 동작만 하고 끝내는 것이 바람직함(이게 안 되면 해당 메소드 내용이 과연 연관된 일인 것인가에 대한 생각이 필요함)

- 만약 같이 일하는 사람들과 평생 일할 것이고 이런 식으로 명명하고 있다면 상관없음 <- (아마 전혀 없을 일임.....)

6. try {} catch(Exception e) {} 구문이 많은 코드

- 보통은 이런 코드들이 프로젝트 유지보수 시에 많이 만날 수 있는데 Fatal Error(애플리케이션이 죽는 에러)를 막기 위해 임시조치한 코드들임

- 하지만 이런 코드들 때문에 다른 버그들이 생겨날 수 있는데 그 이유는 보통 Exception 처리할 때 모든 Exception에 대해 e.printStackTrace(); 이러고 끝내는 경우가 많기 때문. 정상적인 에러에 대해서는 해당 플로우를 처리해줘야 하는 것이 맞는데 그런 경우들을 다 무시하는 격이니 나중에 저런 버그들이 생기면 굉장히 원인을 찾기 어렵게 됨(보통은 버그리포트가 현상 혹은 특정 상황에서의 크래시 리포트로만 오기 때문)

- 시간이 없다면 일단 그대로 냅두고 TODO로 남겨놨다가 beta 혹은 dev 때는 풀어서 그 에러가 무엇인지 정확히 파악하는 것이 중요함

7. 주석처리된 코드(엄밀히 말하면 코드는 아니지만....)

- 이런 건 지워줘야 다른 사람들이 볼 때 좋음(코드를 쭉 따라가다가 저런 코드들 만나면 김이 팍 샌다 ㅜㅜ)

- 코드를 되살리기 위해 하는 거라면 차라리 커밋을 올리고 태그를 다는 것이 더 좋을 것 같음

끝