아래 리스트는 마틴 마울러의 Refactoring 책에서 소개하는 코드 냄새와 저자의 코드를 짜면서 사용하는 기교와 휴리스틱(heuristics)을 포함한다.
휴리스틱 (hueristics): 불충분한 시간이나 정보로 인하여 합리적인 판단을 할 수 없거나, 체계적이면서 합리적인 판단이 굳이 필요하지 않은 상황에서 사람들이 빠르게 사용할 수 있게 보다 용이하게 구성된 간편추론의 방법
다른 시스템(예: 소스 코드 관리 시스템, 버그 추적 시스템, 이슈 추적 시스템, 기타 기록 관리 시스템)에 저장할 정보는 주석으로 적절하지 못하다.
예를 들어, 변경 이력은 장황한 날짜와 따분한 내용으로 소스 코드만 번잡하게 만든다. 일반적으로 `최종 수정일, SPR(Software Problem Report)번호 등과 같은 메타 정보만 주석으로 넣는다.
주석은 코드와 설계에 기술적인 설명을 부연하는 수단이다.
오래된 주석, 엉뚱한 주석, 잘못된 주석은 더 이상 쓸모가 없다.
주석은 빨리 낡는다. 쓸모 없어질 주석은 아예 달지 않는게 낫다. 쓸모 없어진 주석은 재빨리 삭제하는 편이 갖아 낫다. 쓸모 없는 주석은 일단 들어나고 나면 코드에서 쉽게 멀어진다. 코드와 무관하게 혼자 따로 놀며 코드를 그릇된 방향으로 이끈다.
코드만으로 충분한데 구구절절 설명하는 주석은 중복된 주석이다.
주석으로 코드와 같은 설명을 하는 것이나 Javadoc에 함수 서명(signature)만 달랑 기술하는 경우가 이 예이다. 주석은 코드만으로 다하지 못하는 설명을 부언한다.
주석을 달 참이라면 시간을 들여 최대한 멋지게 작성하자
단어는 신중하게 선택한다. 문법과 구두점을 올바로 사용한다. 주절대지 않는다. 당연한 소리를 반복하지 않는다. 간결하고 명료하게 작성한다.
주석으로 처리된 코드를 발견하면 즉각 지워버려라! 걱정할 필요 없다. 소스코드 관리 시스템이 기억한다.
주석으로 처리된 코드는 신경이 아주 거슬린다. 코드가 매일매일 낡아가며 더 이상 존재하지 않는 함수와 변수를 사용하며 모듈을 오염시키며 읽는 사람을 헷갈리게 만든다. 그러나 아무도 삭제하지 않는다. 누군가에게 필요하거나 다른 사람이 사용할 코드라고 생각하기 때문이다.
그러나 주석으로 처리된 코드를 지워도 소스코드 관리 시스템이 기억해서 누군가 정말 필요하다면 이전 버전을 가져올 것이다! 주석으로 처리된 코드를 내버려 두지 마라.
한 명령으로 전체를 체크아웃해서 한 명령으로 빌드할 수 있어야 한다.
빌드는 간단히 한 단계로 끝나야 한다. 소스 코드 관리 시스템에서 이것저것 따로 체크할 필요 없고 스크립트를 잇달아 실행해 각 요소를 따로 빌드할 필요가 없어야 한다.
모든 단위 테스트는 한 명령으로 돌려야 한다.
IDE에서버튼 하나로 모든 테스트를 돌리면 가장 이상적이다. 아무리 열악해도 셸에서 명령 하나로 가능해야 한다. 모든 테스트를 한번에 실행하는 건 방법이 빠르고, 쉽고, 명확해야 한다.
함수에서 인수 개수는 작을 수록 좋다.
함수에서 인수는 아얘 없으면 가장 좋고, 다음으로 하나, 둘, 셋이 차례로 좋다. 넷 이상은 그 가치가 매우 의심스러우니 최대한 피한다.
출력 인수 (함수의 인자를 출력 값으로 사용하는 것)은 직관적이지 않다.
함수를 사용할 때 사용자는 인수를 출력이 아니라 입력으로 간주하기 때문에, 함수에서 뭔가의 상태를 변경해야 하면 출력 인수를 쓰지 말고 함수가 속한 객체의 상태를 변경한다.
플래그 인수는 혼란을 초래하므로 피해야 마땅하다.
boolean 인수는 함수가 여러 기능을 수행한다는 증거다. (함수는 한가지 일만 수행해야 한다.)
아무도 호출하지 않는 함수는 삭제한다.
죽은 코드는 낭비다. 과감히 삭제하자. 소스 코드 관리 시스템이 모두 기억하므로 걱정할 필요가 없다.
소스 파일 하나에 언어 하나만 사용하자.
오늘날 프로그래밍 환경은 소스 파일 하나에 xml, html, yml, javadoc, js 등을 지원한다. 혹은 하나의 jsp파일에 html, java, tag 등의 구문이 포함될 수 있다. 매우 혼란스럽고 조잡하기 때문에 소스 파일에는 최대한 언어 수와 범위를 최대한 줄이도록 노력해야 한다.
함수나 클래스는 다른 프로그래머가 당연하게 여길 만한 동작과 기능을 제공해야 한다.
당연한 동작을 구현하지 않으면 코드를 읽거나 사용하는 사람이 더 이상 함수 이름만으로 함수 기능을 직관적으로 예상하기 어렵다. 저자를 신뢰하지 못하므로 코드를 일일이 살펴야 한다.
코드는 올바로 동작해야 한다. 모든 경계 조건을 찾아내고, 모든 경계 조건을 테스트하는 테스트 케이스를 작성하라.
모든 경계 조건, 모든 구석진 곳, 모든 예외는 우아하고 직관적인 알고리즘을 좌초시킬 암초다. 스스로의 직관에 의존하지 말고 모든 경계조건을 찾아내고, 모든 경계 조건을 테스트하자.
안전 절차를 무시하면 위험하다.
컴파일러 경고 일부를 꺼버리면 빌드가 쉬워질지 모르지만 자칫하면 끝없는 디버깅에 시달린다. 실패하는 테스트 케이스를 제껴두고 나중으로 미루는 태도는 위험하다.
코드의 중복을 제거하라
코드에서 중복을 발견할 때마다 추상화할 기회로 간주하라. 중복된 코드를 하위 루틴이나 다른 클래스로 분리하라. 이렇듯 추상화로 중복을 정리하면 설계 언어의 어휘가 늘어난다. 다른 프로그래머들이 그만큼 어휘를 사용하기 쉬워진다. 추상화 수준을 높였으므로 구현이 빨라지고 오류가 적어진다.
간단한 함수로 교체한다.
다형성으로 대체한다.
중복은 중복이므로 Template method 패턴이나 Strantegy 패턴으로 중복을 제거한다.
추상화로 개념을 분리할 때는 철저해야 한다.
예를 들어, 세부 구현과 관련된 상수, 변수, 유틸리티 함수는 기초 클래스에 넣으면 안 된다. 기초 클래스는 구현 정보에 위치해야 마땅하다. 저차원 개념은 파생 클래스에 넣고 모든 고차원 개념은 추상 클래스에 넣는다.
우수한 소프트웨어 설계자는 개념을 다양한 차원으로 분리해 다른 컨테이너에 넣는다. 때로는 기초 클래스와 파생 클래스를 분리하고, 때로는 소스 파일과 모듈과 컴포넌트로 분리한다. 어떤 경우든 철저하게 분리해야 한다. 잘못된 추상화는 임시변통으로 고치기는 불가능하다.
기초 클래스가 파생클래스를 아얘 몰라야 마땅하다.
개념을 기초 클래스와 파생 클래스로 나누는 가장 흔한 이유는 고차원 기초 클래스 개념을 저차원 파생 클래스 개념으로부터 분리해 독립성을 보장하기 위해서다. 그러므로 기초 클래스가 파생 클래스를 사용한다면 뭔가 문제가 있다는 말이다.
잘 정의된 모듈은 인터페이스가 아주 작다.
부실하게 정의된 모듈은 인터페이스가 구질구질하다. 그래서 간단한 동작 하나에도 온갖 인터페이스가 필요하다. 잘 정의된 인터페이스는 많은 함수를 제공하지 않는다. 그래서 결합도가 낮다. 부실하게 정의된 인터페이스는 반드시 호출해야 하는 온갖 함수를 제공하여 결합도가 높다.
클래스가 제공하는 메서드 수는 작을 수록 좋다. 함수가 아는 변수 수도 작을 수록 좋다. 클래스의 인스턴스 변수도 작을 수록 좋다.
자료를 숨기고, 유틸리티 함수를 숨기고, 상수와 임시 변수도 숨기자. 메서드나 인스턴스 변수가 넘쳐나는 클래스를 피하자.
하위 클래스에서 필요하다는 이유로 protected 변수나 함수를 마구 생성하지 말자.
인터페이스를 매우 작게 그리고 매우 깐깐하게 만들자. 정보를 제한해 결합도를 낮추자.
죽은 코드는 시스템에서 제거하라
죽은 코드란 실행되지 않는 코드다. 불가능한 조건을 확인하는 if문이나 throw 문이 없는 try 문에서 catch 블록, 아무도 호출되지 않는 유틸리티 함수나 switch/case 문에서 불가능한 case 조건 등이 좋은 예다.
죽은 코드는 시간이 지나면 악취를 풍긴다. 발견하면 적절한 장례식을 치뤄주자. 시스템에서 제거해라.
사용되는 가까운 위치에 정의하라
변수와 함수는 사용되는 위치에 가깝게 정의한다. 지역변수는 처음으로 사용하기 직전에 선언하고 수직으로 가까운 곳에 위치해야 한다. 선언한 위치로부터 몇백 줄 아래에서 사용하면 안 된다.
private 함수는 처음으로 호출한 직후에 정의한다.
어떤 개념을 특정 방식으로 구현했다면 유사한 개념도 같은 방식으로 구현한다.
일관성 있는 변수 이름, 함수 이름을 사용하자. 이런 간단한 일관성만으로도 코드를 읽고 수정하기 쉬워진다.
필요 없는 잡동사니 코드는 깔끔하게 정리하자
비어 있는 기본 생성자가 왜 필요한가? 아무도 사용하지 않을 변수, 아무도 호출하지 않을 함수, 정보를 제공하지 못하는 주석 등 필요 없는 잡동사니 코드는 모두 제거하자.
서로 무관한 개념을 인위적으로 결합하지 않는다.
일반적인 enum은 특정 클래스에 속할 이유가 없다. enum이 클래스에 속한다면 enum을 사용하는 코드가 특정 클래스를 알아야만 한다. 범용 static 함수도 마찬가지로 특정 클래스에 속할 이유가 없다.
인위적인 결합은 직접적인 상호작용이 없는 두 모듈 사이에서 일어난다. 뚜렷한 목적 없이 변수, 상수, 함수를 당장 편한 위치 (잘못된 위치)에 넣어버린 결과다.
함수, 상수, 변수를 선언할 때는 시간을 들여 올바른 위치를 고민하자.
클래스 메서드는 자기 클래스의 변수와 함수에 관심을 가져야지 다른 클래스의 변수와 함수에 관심을 가져서는 안된다.
메서드가 다른 객체의 참조자(accessor)와 변경자(mutator)를 사용해 그 객체 내용을 조작한다면 메서드가 그 객체 클래스의 범위를 욕심내는 탓이다. 자신이 그 클래스에 속해 그 클래스 변수를 직접 조작하고 싶다는 뜻이다.
boolean 선택자 인수를 인자값으로 넘기지 말자
선택자 인수가 여러 함수를 하나로 조합한다. 선택자 인수는 큰 함수를 작은 함수 여럿으로 쪼개지 않으려는 게으름의 소산이다. 일반적으로 인수를 넘겨 동작을 선택하게 하는 대신 새로운 함수를 만드는 것이 좋다.
코드를 짤 때는 의도를 최대한 분명히 밝힌다.
행을 바꾸지 않고 표현한 수식, 헝가리식 표기법, 매직 번호 등은 모두 저자의 의도를 흐린다.
코드는 독자가 자연스럽게 기대할 위치에 배치한다.
때로는 개발자가 영리하게 기능을 배치한다. 독자에게 직관적인 위치가 아니라 개발자에게 편한 함수에 배치한다. 코드는 독자가 자연스럽게 기대할 위치에 배치하자.
반드시 static 함수로 정의해야겟다면 재정의할 가능성은 없는지 꼼꼼히 따져본다.
Math.max()와 같은 static 함수는 재정의할 가능성이 없는 좋은 static 메서드다. 하지만 간혹 우리는 static으로 정의하면 안되는 함수를 static으로 만든다.
HourlyPayCalculator.calculatePay(employee, overtimeRate);
위 함수는 static 함수로 적절할까? 특정 객체와 관련이 없으면서 모든 정보를 인수에서 가져오니까 적절해 보이기도 하지만, 수당을 계산하는 알고리즘이 여러 개 일지도 모른다. 예를 들어 OvertimeHourlyPayCalculator와 StraightTimeHourlyPayCalculator를 분리하고 싶을지도 모른다. 그러므로 위 함수는 Employee 클래스에 속하는 인스턴스 함수인게 맞다.
일반적으로 static 함수보다 인스턴스 함수가 더 좋다. 조금이라도 의심스럽다면 인스턴스로 정의하자. static 함수로 정의해야겠다면 재정의할 가능성은 없는지 꼼꼼히 살펴본다.
서술적인 변수 이름은 많을 수록 좋다.
프로그램 가독성을 높이는 가장 효과적인 방법 중 하나가 계산을 여러 단계로 나누고 중간 값으로 서술적인 변수 이름을 사용하는 방법이다.
Matcher match = headerPattern.matcher(line);
if(match.find())
{
String key = match.group(1);
String value = match.group(2);
headers.put(key.toLowerCase(), value);
}서술적인 변수 이름은 많이 써도 괜찮다. 일반적으로는 많을수록 더 좋다. 계산을 몇 단계로 나누고 중간 값에 좋은 변수 이름만 붙여도 해독하기 어렵던 모듈이 순식간에 읽기 쉬운 모듈로 탈바꿈한다.
함수는 이름과 기능이 일치해야 한다.
이름만으로 분명하지 않기에 구현을 살피거나 문서를 뒤적여야 한다면 더 좋은 이름으로 바꾸거나 더 좋은 이름을 붙이기 쉽도록 기능을 정의해야 한다
알고리즘이 올바르다는 사실을 확인하고 이해하려면 기능이 빤히 보일 정도로 함수를 깔끔하고 명확하게 재구성하는 방법이 최고다.
구현이 끝났다고 선언하기 전에 함수가 돌아가는 방식을 확실히 이해하는지 확인하라. 테스트 케이스를 모두 통과한다는 사실만으로 부족하다. 작성자가 알고리즘이 올바르다는 사실을 알아야 한다.
의존하는 모든 정보를 명시적으로 요청하는 편이 좋다.
한 모듈이 다른 모듈에 의존한다면 물리적인 의존성도 있어야 한다. 논리적인 의존성만으로는 부족하다. 의존하는 모듈이 상대 모듈에 대해 뭔가를 가정하면(논리적으로 의존하면) 안 된다. 의존하는 모든 정보를 명시적으로 요청하는 편이 좋다.
선택 유형 하나에는 switch 문을 한 번만 사용한다. 같은 선택을 수행하는 다른 코드에서는 다형성 객체를 생성해 switch문을 대체한다.
새 유형을 추가할 확률보다 새 함수를 추가할 확률이 높은 코드에서는 switch문이 더 적합하다. 단, 유형보다 함수가 더 쉽게 변하는 경우는 극히 드물기 때문에 모든 switch문을 의심해야 한다. switch문이 더 쉬운 선택이라 많은 개발자가 자주 선택하지만, 다형성을 먼저 고려하자.
팀은 업계 표준이 기반한 구현 표준을 따르고, 팀이 정한 표준은 팀원들 모두가 따라야 한다.
구현 표준은 인스턴스 변수 이름을 선언하는 위치, 클래스/메서드/변수 이름을 정하는 방법, 괄호를 넣는 위치 등을 명시해야 한다. 표준을 설명하는 문서는 코드 자체로 충분해야 하며 별도 문서를 만들 필요는 없어야 한다.
팀의 표준은 실제 괄호를 넣는 위치는 중요하지 않다. 모두가 동의한 위치에 넣는다는 사실이 중요하다. 이 사실을 이해할 정도로 팀원들이 성숙해야 한다.
일반적으로 코드에서 숫자를 사용하지 말고 상수 뒤로 숨겨라
코드 내애서 사용되는 숫자를 상수로 바꾸어 사용하자.
매직 숫자라는 용어는 단지 숫자만 의미하지 않는다. 의미가 분명하지 않은 토큰을 모두 가리킨다. 예를 들어 테스트 코드에서 시급 직원을 의미하는 "Jone Doe"이나 그 직원번호 7777도 매직 넘버이다.
assertEquals(7777, Employee.find("Jone Doe").employeeNumber());따라서 아래와 바꾸는게 옳다.
assertEquals(
HOURLY_EMPLOYEE_ID,
Employee.find(HOURLY_EMPLOYEE_NAME).employeeNumber());
)코드에서 뭔가를 결정할 때는 정확히 결정한다.
검색 결과 중 첫 번째 결과만 유일한 결과로 간주하는 행동, 부동소수점을 통화로 표현하는 행동, 갱신할 가능성이 희박하다고 lock과 트랜잭션 관리를 건너뛰는 행동은 아무리 잘봐줘도 게으름이다.
결정을 내리는 이유와 예외를 처리할 방법을 분명히 알아야 한다. 호출하는 함수가 null을 반환할지도 모른다면 null을 반드시 점검한다. 조회 결과가 하나뿐이라고 짐작한다면 하나인지 확실히 확인한다. 통화를 다뤄야 한다면 정수를 사용하고 반올림을 올바로 처리한다. concurrent 특성으로 동시 갱신 가능성이 있다면 lock을 사용한다.
설계 결정을 강제할 때는 규칙보다 관례를 사용한다. 명명 관례도 좋지만 구조자체로 강제하면 더 좋다.
예를 들어, enum 변수가 멋진 switch/case문보다 추상 메서드가 있는 기초 클래스가 더 좋다. switch/case 문을 매번 똑같이 구현하게 강제하기는 어렵지만, 파생 클래스는 추상 클래스를 모두 구현하지 않으면 안되기 때문이다.
조건의 의도를 분명히 밝히는 함수료 표현하라
예를 들어
if (shouldBeDeleted(timer))
라는 코드는 다음 코드 보다 좋다.
if (timer.hasExpired() && !timer.isRecurrent())
부정조건은 긍정조건보다 이해하기 어렵다. 가능하면 긍정 조건으로 표현한다.
예를 들면,
if (buffer.shouldCompact())
라는 코드가 다음 코드 보다 좋다.
if (!buffer.shouldNotCompact())
한 함수 안에서 여러 작업을 수행하는 경우 한 가지만 수행하는 함수 여럿으로 쪼개자
예를 들어,
public void pay() {
for(Employee e : employees) {
if (e.isPayday()) {
Money pay = e.calculatePay();
e.deliverPay(pay);
}
}
}위 코드는 직원 목록을 루프로 돌며 직원의 월급일을 확인하고, 해당 직원에게 월급을 지급한다. 위 함수는 다음 함수 셋으로 나누는 편이 좋다.
public void pay() {
for (Employee e : employees) {
payIfNecessary(e);
}
}
private void payIfNecessary(Employee e) {
if (e.isPayday()){
calculateAndDeliverPay(e);
}
}
private void calculateAndDeliverPay(Emplouyee e) {
Money pay = e.calculatePay();
e.deliverPay(pay);
}시간적인 결합을 숨겨서는 안된다. 함수를 짤 때는 함수 인수를 적절히 배치해 함수가 호출되는 순서를 명백히 드러낸다.
일종의 연결 소자를 생성해 시간적인 결합을 노출한다. 각 함수가 내놓는 결과는 다음 함수에 필요하도록 시각적인 결합을 명백히 드러낸다.
코드 구조에 일관성을 유지하라
코드 구조를 잡을 때는 이유를 고민하라. 구조에 일관성이 없어 보인다면 남들이 맘대로 바꿔도 괜찮다고 생각한다. 시스템 전반에 걸쳐 구조가 일관성 있다면 남들도 일관성을 따르고 보존한다.
경계 조건을 한 곳에서 별도로 처리하자
경계 조건은 빼먹거나 놓치기 십상이다. 경계 조건은 한곳에서 별도로 처리한다. 코드 여기저기에서 처리하지 않는다. 예를 들어 코드 여기저기에 +1이나 -1을 흩어놓지 않는다. 경계 조건을 변수로 캡슐화하자.
함수 내 모든 문장은 추상화 수준이 동일해야 한다. 그리고 그 추상화 수준은 함수 이름이 의미하는 작업보다 한 단계만 낮아야 한다.
추상화 최상위 단계에 둬야 할 기본 값 상수나 설정 관련 상수를 저차원 함수에 숨겨서는 안 된다. 대신 고차원 함수에서 저차원 함수를 호출할 때 인수로 넘긴다.
함수 내 모든 문장은 추상화 수준이 동일해야 한다. 그리고 그 추상화 수준은 함수 이름이 의미하는 작업보다 한 단계만 낮아야 한다. 필요한 추상화 수준을 분리한 후 테스트 케이스를 통과한다는 목표를 세워라.
import 문은 패키지를 단순히 검색 경로에 추가하므로 진정한 의존성이 생기지 않는다. 와일드 카드를 사용하면 모듈 간에 결합성이 낮아진다.
상수를 인터페이스에 넣고 그 인터페이스를 상속해 해당 상수를 사용하는 것은 언어의 범위 규칙을 속이는 행위다.
java의 enum을 맘껏 사용하라! 더이상 public static final int라는 옛날 기교를 사용할 필요가 없다.
서술적인 이름은 신중하게 고른다. 신중하게 선택한 이름은 추가 설명을 포함한 코드보다 강력하다. 신중하게 선택한 이름을 보고 독자는 모듈 내 다른 함수가 하는 일을 짐작한다.
구현을 드러내는 이름은 피하라. 작업 대상 클래스나 함수가 위치하는 추상화 수준을 반영하는 이름을 선택하라.
기존 명명법을 사용하는 이름은 이해하기 쉽다.
함수나 변수의 목적을 명확히 밝히는 이름을 선택한다.
이름 길이는 범위 길이에 비례해야 한다. 범위가 작으면 아주 짧은 이름을 사용해도 괜찮다. 하지만 범위가 길어지면 긴 이름을 사용한다.
이름에 유형 정보나 범위 정보를 넣어서는 안 된다. m_dlsk f와 같은 접두어는 불필요하다.
함수, 변수, 클래스가 하는 일을 모두 기술하는 이름을 사용한다. 이름에 부수 효과를 숨기지 않는다.
테스트 케이스는 잠재적으로 깨질 만한 부분은 모두 테스트해야 한다. 테스트 케이스가 확인하지 않은 조건이나 검증하지 않은 계산이 있다면 그 테스트는 불완전하다.
커버리지 도구는 테스트가 빠뜨리는 공백을 알려준다. 커버리지 도구를 사용하면 테스트가 불충분한 모듈, 클래스, 함수를 찾기가 쉬워진다.
사소한 테스트는 짜기 쉽다. 사소한 테스트가 제공하는 문서적 가치는 구현에 드는 비용을 넘어선다.
불분명한 요구사항은 테스트 케이스를 주석으로 처리하거나 테스트 케이스에 @Ignore를 붙여 표현한다.
경계 조건은 각별히 신경 써서 테스트한다.
버그는 서로 모이는 경향이 있다. 한 함수에서 버그를 발견했따면 그 함수를 철저히 테스트하는 편이 좋다.
테스트 케이스가 실패하는 패턴으로 문제를 진달할 수 있다. 테스트 케이스를 최대한 꼼꼼히 짜라는 이유도 여기에 있다. 합리적인 순서로 정렬된 꼼ㄲ모한 케이스는 실패 패턴을 드러낸다.
통과하는 테스트가 실행하거나 실행하지 않은 코드를 살펴보면 실패하는 테스트 케이스의 실패 원인이 드러난다.
느린 테스트는 실행하지 않게 된다. 일정이 촉박하면 느린 테스트 케이스를 제일 먼저 건너뛴다. 그러므로 테스트 케이스가 빨리 돌아가게 최대한 노력한다.
이 장에서 소개한 휴리스틱과 냄새 목록이 완전하다 말하기 어렵다. 완전한 목록이 목적이 아니라 여기서 소개한 목록은 가치 체계를 피력할 뿐이다.
규칙만 따른다고 깨끗한 코드가 얻어지지 않는다. 휴리스틱 목록을 익힌다고 소프트웨어 장인이 되지는 못한다. 전문가 정신과 장인 정신은 가치에서 나온다. 그 가치에 기반한 규율과 절제가 필요하다.
클린 코드