나쁜 코드에 주석을 달지 마라. 새로 짜라.
잘 달린 주석은 어떤 정보보다 유용하지만, 경솔한 주서거은 코드에 거짓과 잘못된 정보를 퍼뜨려 부작용을 초래한다. 코드 자체를 치밀하게 작성해 의도를 표현할 수 있다면, 주석은 필요하지 않다.
주석은 오래될수록 코드에서 멀어진다. 주석을 엄격하게 관리하고 복구성과 관련성, 정확성을 높이는 것이 방법일 수도 있지만, 코드를 깔끔하게 정리하고 표현력을 강화하는 쪽이 더 효과적이다.
부정확한 주석은 없는 것보다 훨씬 못하다. 부정확한 주석은 읽는 사람으로 하여금 현혹하고 오도한다. 더 이상 지킬 필요가 없는 규칙이나 지켜서는 안되는 규칙을 명시한다. 따라서 최대한 주석을 줄이도록 노력하자.
이번 섹션에서는 주석을 줄여야하는 이유와 그럼에도 사용할 수 있는 좋은 주석, 나쁜 주석의 예시를 다룬다.
코드에 주석을 추가하는 이유는 코드만으로 의도를 제대로 표현하지 못하기 때문이다. (== 짜고 보니 짜임새도 엉망이고 알아먹기 힘들다..)
그렇다고 해서 주석을 추가해 설명하려하면 안된다. 주석으로 설명하기보단 코드를 깔끔하게 다듬는 데 시간을 써야한다. 표현력이 풍부하고 깔끔하며 주석이 거의 없는 코드가, 복잡하고 어수선한 주석이 달린 코드보다 훨씬 좋다.
코드만으로 의도를 설명하기 어려운 경우가 존재하긴 한다. 그렇다고 해도, 코드로 최대한 표현하려고 노력해야 한다.
Bad:
//직원에게 복지 해택을 받을 자격이 있는지 검사한다.
if((employee.flags & HOULRY_FLAG) && (employee.age > 65))
...
Good:
if(employee.isEligibleForFullBenefits())
두 코드를 비교하면 명확하게 아래처럼 주석 없이도 표현할 수 있음을 알 수 있다. 대부분은 주석의 설명을 함수로 축약해 만들어 표현하면 충분히 의도가 드러난다.
그럼에도 불구하고 특정 주석들은 유익하다. 몇 가지 예시를 소개한다. 하지만 가장 좋은 것은 늘 주석을 달지 않는 방법을 생각하는 것이다.
저작권, 소유권 등 법적인 이유로 회사가 정립한 구현 표준에 따라야 하는 경우. 모든 계약 조건을 명시하는 것이 아니라 표준 라이센스나 외부 문서에 대한 참조만 추가하는 것으로 충분하다.
Example:
//Copyright (C) 2003,2004,2005 by Object Mentor, Inc. All rights reserved.
//GNU General Public License 버전 2 이상을 따르는 조건으로 배포한다.
다음과 같은 법적 정보를 코드 첫머리에 추가하는 형식이 일반적이며, 요즘의 IDE들은 이런 헤더를 자동으로 축소해 코드만 깔끔하게 표시해주기도 한다.
기본적인 정보를 주석으로 제공하기도 한다. 그러나 이는 정말 꼭 필요한지, 코드로 표현할 수는 없는지 확인해야 한다.
Bad:
//테스트 중인 Responder 인스턴스를 반환한다.
protected abstract Responder responderInstance();
위 코드의 주석이 유용하다할지라도, 함수 이름을 아래처럼 바꾼다면 주석이 필요 없어진다.
Good:
protected abstract Responder responderBeingTested();
특히 날짜와 같은 형식은 주석으로 나타내도 무방하다. 하지만, 시각과 날짜를 변환해주는 클래스를 제작해서 변환해준다면, 더 깔끔하고 주석도 필요 없어진다.
때로 주석은 구현의 이해를 돕는 수준에서 벗어나 제작 과정 아래 깔린 의도까지 설명해준다.
Bad:
public int compareTo(Object o) {
if(o instanceof WikiPagePah) {
WikiPagePath p = (WikiPagePath) o;
String compressedName = StringUtil.join(names, "");
String compressedArgumentName = StringUtil.join(p.names, "");
return compressedName.compareTo(compressedArgumentName);
}
return 1; // 옳은 유형이르모 정렬 순위가 더 높다.
}
위 코드는 두 객체를 비교할 때 어떤 객체보다 자기 객체에 높은 우선 순위를 주기로 했음을 주석으로 표현했다. 위 코드는 아래와 같이 수정하면, 주석 없이 의도를 표현할 수 있다.
public void testConcurrentAddWidgets() throws Exception {
WidgetBuilder widgetBuilder = new WidgetBuilder(new Class[]{BoldWidget.class});
String text = "'''bold text'''";
ParentWidget parent = new BoldWidget(new MockWidgetRoot(), "'''bold text'''");
AtomicBoolean failFlag = new AtomicBoolean();
failFlag.set(false);
//스레드를 대량 생성하는 방법으로 어떻게든 경쟁 조건을 만들려 시도한다.
for (int i = 0; i < 25000; i++) {
WidgetBuilderThread widgetBuilderThread = new WidgetBuilderThread(widgetBuilder, text, parent, failFlag);
Thread thread = new Thread(widgetBuilderThread);
thread.start();
}
assertEquals(false, failFlag.get());
}
반면 위 코드는 저자가 문제를 해결한 방식과 달라지긴 했지만, 주석을 사용하지 않고도 저자의 의도는 분명히 드러냈다. (중간에 있는 주석은 독자들에게 설명하기 위한 주석, 저런 식으로 구현하면 된다고 말하고 싶은 듯)
모호한 파라미터나 반환값의 의미를 표현하기 위해서 사용하는 주석은 괜찮다. 일반적으로 파라미터나 반환값 자체를 명확하게 만들면 더 좋지만, 표준 라이브러리나 변경하지 못하는 코드에 속하는 경우 의미를 명료하게 밝히는 주석은 효과적이다.
assertTure(a.compareTO(a) == 0); // a == a
위 주석은 표준 라이브러리의 함수 compareTo의 의미를 명료하게 밝히는 용도로 사용한다. 반면 이런 주석이 올바른지 검증하는 것은 쉽지 않기 때문에, 의미를 명료히 밝히는 주석은 더 위험할 수도 있다.
다른 프로그래머들에게 결과를 경고하는 목적으로 사용하는 주석도 존재한다.
public static SimpleDateFormat makeStandardHttpDateFormat(){
// SimpleDateFormat은 스레드에 안전하지 못하다.
// 따라서 각 인스턴스를 독립적으로 생성해야 한다.
SimpleDataFormat df = new SimpleDataFormat("EEE, dd MMM yyyy HH:mm:ss z");
df.setTimeZone(TimeZone.getTimeZone("GMT"));
return df;
}
테스트 코드의 경우 오래 걸리거나 결과가 위험할 수 있는 테스트 케이스에 대해서 @Ignore annotation을 추가하고, @Ignore("실행이 너무 오래 걸림") 등으로 설명을 추가하는 방식으로 미리 경고와 함께 테스트케이스를 끌 수 있다.
앞으로 할 일을 '//TODO:' 의 형태로 남겨두면 나중에 찾기도 편하다. 현대적인 IDE들은 TODO 주석들을 Highlight 할 수 있는 플러그인을 제공하기도 한다.
TODO 주석은 필요하다고 생각되지만, 당장 구현하기는 어려운 임무를 기술한다. 혹은 필요 없는 기능을 검토 후 삭제해달라는 기록, 코드 리뷰 요청, 더 좋은 이름을 요구하는 부탁, 앞으로의 변화에 맞게 코드를 수정하라는 주의 등의 용도로 사용하면 효과적이다.
반면 시스템에 나쁜 코드를 남겨두는 용도로 사용되거나, 너무 많은 TODO 를 남발하는 것은 좋지 않다. 주기적으로 TODO를 점검해 제거해주도록 하자.
//TODO: Admin module 구현 완료시 생성한 사용자 기록하기
자칫 대수롭지 않다고 여겨질 뭔가의 중요성을 강조하기 위해서 주석을 사용하자. 반면 해당 주석도 너무 많이 남발되는 경우 프로그래머로 하여금 해당 주의를 무시해버리도록 만들 수 있으니 꼭 필요한 경우에만 사용하자.
String listItemContent = match.group(3).trim();
// 여기서 trim은 정말 중요하다. trim 함수는 문자열에서 시작 공백을 제거한다.
// 문자열에 시작 공백이 있으면 다른 문자열로 인식된다.
new ListItemWidget(this, listItemContent, this.level + 1);
return buildList(text.substring(match.end()));
설명이 잘된 API는 사용자에게 유용하다. 공개 API를 구현한다면 반드시 잘 고려한 Javadocs를 추가해야 한다. 반면, 해당 주석을 추가할 때도 본 장에서 언급한 내용들은 명심하면서 추가하자. Javadocs 또한 독자를 오도하거나, 잘못 위치하거나, 그릇된 정보를 전달해 현혹시킬 가능성이 있다.
대부분의 주석이 여기에 속한다. 일반적으로 주석은 허술한 코드를 지탱하거나 변명하고, 합리화하는 독백에서 크게 벗어나질 못한다.
주석을 달기로 결정했다면, 충분히 시간을 들여 최고의 주석을 달자. 예시는 아래와 같다.
public void loadProperties() {
try{
String propertiesPath = propertiesLocation + "/" + PROPERTIES_FILE;
FileInputStream propertiesStream = new FileInputStream(propertiesPath);
loadedProperties.load(propertiesStream);
}
catch (IOException e){
// 속성 파일이 없다면 기본 값을 모두 메모리로 읽어 들였다는 의미다.
}
}
catch 블록 내부 주석은 이해하기 힘들다. 이해하기 위해서는 다른 코드까지 전부 살펴봐야 한다. 즉, 그것은 다른 모듈까지 뒤지도록 하는 주석은 독자에게 제대로 전달되지 않는 주절거리는 주석에 불과하다.
주석이 같은 코드 내용을 그대로 중복하는 경우에 해당된다. 아래 코드를 보자.
// this.closed가 true일 때 반환되는 util
// 타임아웃에 도달하면 예외를 던진다
public synchronized void waitForClose(final long timeoutMillis) throws Exception {
if(!closed) {
wait(timeoutMillis);
if(!closed) throw new Exception("MockResponseSender could not be closed");
}
}
위 코드의 주석은 코드보다 더 많은 정보를 제공하지 못한다. 이 경우 주석을 제거하는 편이 낫다. 혹은 가끔은 너무 당연한 사실을 언급하고, 새로운 정보를 제공하지 못하는 주석도 존재한다.
/**
* 월 중 일자
*/
private int dayOfMonth;
이런 주석들은 개발자로 하여금 다른 주석까지도 무시하고 대충 넘어가도록 만든다. 또한, 그렇게 방치된 주석은 결국 거짓 주석으로 변한다. 이런 주석을 추가하기보단 코드를 정리해라.
의도는 좋았지만, 독자로 하여금 오해할 여지를 만드는 주석은 좋지 않다.
바로 위 예제 코드에서 코드는 this.cloed가 true로 변하는 순간에 메서드가 반환되는 것은 아니다. this.closed가 true이거나, 타임아웃을 기다린 후 그때도 this.closed가 true가 아니면 예외를 던진다.
이런 사소한 차이가 프로그래머로 하여금 어떤 이유로 자신의 코드가 느려지는지 찾지 못하게 만든다.
모든 함수에 의무적으로 누가 작성했고, 어떤 변경 내역을 갖는지 작성하지 말자. 규칙이 있다면, 꼭 필요한 부분에만 추가하자.
대부분의 기업이나 프로젝트는 Git과 같은 소스코드 관리 시스템을 사용하고, 그것이 저자와 공로, 수정 변경 내역을 알아서 관리해준다.
이 또한 같은 말인데, 주석을 추가하기보단, 함수나 변수로 표현함으로써 최대한 코드를 개선하자.
Bad:
// 전역 모듈 <smodule>에 속하는 모듈이 우리가 속한 하위 시스템에 의존하는가?
if(smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))
위 주석을 없애고 코드로 표현한 결과는 아래와 같다.
Good:
ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moudleDependees.contains(ourSubSystem))
가끔 소스파일에서 특정 위치를 표시하기 위해 주석을 사용한다.
// Actions //////////////////////////////
위와 같은 배너 아래 특정 기능을 모으면 유용한 경우도 있긴 하다. 하지만, 일반적으로는 가독성만 낮추기 때문에 제거하는 편이 낫다.
혹은 반드시 사용해야 한다면 꼭 필요한 경우에만 사용함으로써 주의를 환기하는 용도로는 사용할 수 있다. 남용하면 이 또한 독자로 하여금 잡음으로 여기게 된다.
닫는 괄호에 어디서 시작한 괄호인지 추가하기보단 함수를 줄이려고 노력하자.
주석으로 처리해두면, 다른 사람 또한 해당 코드를 지우기를 주저한다. (이유가 있을 것이라 생각해서)
대부분의 코드는 소스코드 관리 시스템을 통해 저장되고, 이는 반드시 되찾을 수 있으니 주석으로 처리하지 않아도 잃어버리지 않을 수 있다.
주석을 뽑아 웹페이지에 올리기 위해서 추가되는 HTML 태그들 또한 제거하자. 주석에 해당 태그를 삽입하는 것은 다른 추가적인 도구로 처리되어야 한다.
주변 코드가 아닌 시스템의 전반적인 정보를 기술해서는 안된다. 바로 아래 함수가 아니라 시스템 어딘가에 있는 다른 함수를 설명하려 하지 말자.
코드와 크게 관련 없는 정보나 역사를 장황하게 늘어두지 말자.
주석은 그것이 설명하는 코드와 관계가 명확해야 한다. 독자가 주석을 읽고 어떤 소리인지 알 수 있는 주석을 추가하자.
// 모든 픽셀을 담을 만큼 충분한 배열로 시작한다(여기에 필터 바이트를 더한다.)
// 헤더 정보를 위해 200 바이트를 추가한다.
this.pngBytes = new byte[((this.width + 1) * this.height * 3) + 200];
위 주석을 읽고 필터 바이트가 +1을 의미하는지, *3을 의미하는지 알 수 없다. 또한 200이 갑자기 왜 추가되는지도 판단하기 어렵다.
주석 자체가 새로운 설명을 요구하도록 추가해선 안된다.
짧고 한 가지만 수행하며, 이름을 잘 붙인 함수는 긴 설명이 포함된 헤더가 필요가 없다. (그렇게 만들라는 뜻)
외부에 공개할 생각이 아니라면 공개 API처럼 Javadocs를 추가하진 말자. 코드가 보기 싫고 산만해지기만 한다.