클린 코드 - 4장 주석
03 Apr 2021Intro
초서-독서 한 내용을 그대로 적는 곳이기 때문에 책을 읽지 않은 분들이 보기에 맥락이 애매할 수 있습니다.
초서 : 책을 읽는데 그치는 것이 아닌 손을 이용해 책의 중요한 내용을 옮겨 적음으로써 능동적으로 책의 내용을 수용하고 판단하여 새로운 지식을 재창조하는 과정. 메타인지 학습법
Index
- 들어가면서
- 1장 깨끗한 코드
- 2장 의미 있는 이름
- 3장 함수
- 4장 주석 «
- 5장 형식 맞추기
- 6장 객체와 자료 구조
- 7장 오류 처리
- 8장 경계
- 9장 단위 테스트
- 10장 클래스
- 11장 시스템
- 12장 창발성(創發性)
- 13장 동시성
- 14장 점진적인 개선
- 15장 JUnit 들여다보기
- 16장 SerialDate 리팩터링
- 17장 냄새와 휴리스틱
4장 주석
" 우리에게 프로그래밍 언어를 치밀하게 사용해 의도를 표현할 능력이 있다면, 주석은 거의 필요하지 않으리라. 아니, 전혀 필요하지 않으리라. "
" 우리는 코드로 의도를 표현하지 못해, 그러니까 실패를 만회하기 위해 주석을 사용한다. "
" 주석을 달 때마다 자신에게 표현력이 없다는 사실을 푸념해야 마땅하다. "
" 주석이 언제나 코드를 따라가지는 않는다. 아니, 따라가지 못한다. "
" 진실은 한 곳에만 존재한다. 바로 코드다. 코드만이 자기가 하는 일을 진실되게 말한다. "
주석은 나쁜 코드를 보완하지 못한다
코드에 주석을 추가하는 일반적인 이류는 코드 품질이 나쁘기 때문 이다.
표현력이 풍부하고 깔끔하며 주석이 거의 없는 코드가,
복잡하고 어수선하며 주석이 많이 달린 코드보다 훨씬 좋다.
” 자신이 저지른 난장판을 주석으로 설명하려 애쓰는 대신에 그 난장판을 깨끗이 치우는 데 시간을 보내라! “
코드로 의도를 표현하라!
두 예제가 있다. 어느 쪽이 나은지는 한 눈에 알아볼 수 있다.
// 직원에게 복지 혜택을 받을 자격이 있는지 검사한다.
if ((employee.flags & HOURLY_FLAG) &&
(employee.age > 65))
vs
if (employee.isEligibleForFullBenefits())
좋은 주석
글자 값을 한다고 생각하는 주석:
- 법적인 주석
- 저작권 정보와 소유권 정보 등을 소스 파일 첫머리에 주석으로 표시
- 정보를 제공하는 주석
- 기본적인 정보
// 테스트 중인 Responder 인스턴스를 반환한다.
protected abstract Responder responderInstance();
또는
// kk:mm:ss EEE, MMM dd, yyyy 형식이다.
Pattern timeMatcher = Pattern.compile(
"\\d*:\\d*:\\d* \\w*,\\w* \\d*,\\d*");
위 주석은 코드에서 사용한 정규표현식이 시각과 날짜를 뜻한다고 설명한다.
- 의도를 설명하는 주석
- (구현을 이해하게 도와주는 선을 넘어) 결정에 깔린 의도를 설명하는 주석이다.
다음 예제는 두 객체를 비교할 때 다른 어떤 객체보다 자기 객체에 높은 순위를 주기로 결정한다.
- (구현을 이해하게 도와주는 선을 넘어) 결정에 깔린 의도를 설명하는 주석이다.
public int compareTo(Object o) {
if (o instanceof WikiPagePath) {
WikiPagePath p = (WikiPagePath) o;
String compressedName = StringUtil.join(names, "");
String compressedArgumentName = StringUtil.join(p.names, "");
return compressedName.compareTo(compressedArgumentName);
}
return 1; // 오른쪽 유형이므로 정렬 순위가 더 높다.
}
- 의미를 명료하게 밝히는 주석
- 모호한 인수나 반환값은 그 의미를 읽기 좋게 표현하면 이해하기 쉬워진다.
- 일반적으로는 인수나 반환값 자체를 명확하게 만들면 더 좋겠지만, 인수나 반환값이
표준 라이브러리나 변경하지 못하는 코드에 속한다면 주석이 유용하다.
public void testCompareTo() throw Exception {
WikiPagePath a = PathParser.parse("PageA");
WikiPagePath ab = PathParser.parse("PageA.PageB");
WikiPagePath b = PageParser.parse("PageB");
...
assertTrue(a.compareTo(a) == 0); // a == a
assertTrue(a.compareTo(b) != 0); // a != b
assertTrue(ab.compareTo(ab) == 0); // ab == ab
...
}
그릇된 주석을 달아놓을 위험이 상당히 높다.
위 주석이 올바른지 검증하기 쉽지 않다. (주석은 코드를 따라가지 않는다!)
- 결과를 경고하는 주석
- 때로는 다른 프로그래머에게 결과를 경고할 목적으로 주석을 사용한다.
// 여유 시간이 충분하지 않다면 실행하지 마십시오.
public void _testWithReallyBigFile() {
whiteLinesToFile(10000000);
response.setBody(testFile);
response.setReadyToSend(this);
String responseString = output.toString();
assertSubString("Content-Length: 1000000000", responseString);
assertTrue(bytesSent > 1000000000);
}
위 예제는 @Ignore 속성을 이용하여 테스트를 꺼버릴 수 있다..
@Ignore("실행이 너무 오래 걸림")
- TODO 추석
- ‘앞으로 할 일’을
// TODO
주석으로 남겨두면 편하다. - 더 이상 필요 없는 기능을 삭제하라는 알림
- 누군가에게 문제를 봐달라는 요청
- 더 좋은 이름을 떠올려 달라는 부탁
- 앞으로 발생할 이벤트에 맞춰 코드를 고치라는 주의 등
- ‘앞으로 할 일’을
// TODO-MdM 현재 필요하지 않다.
// 체크아웃 모델을 도입하면 함수가 필요 없다.
protected VersionInfo makeVersion() throws Exception {
return null;
}
그래도 TODO 로 떡칠한 코드는 바람직하지 않다.
그러므로 주기적으로 TODO 주석을 점검해 없애도 괜찮은 주석은 없애라.
- 중요성을 강조하는 주석
...
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 블록에 있는 주석은 무슨 뜻일까? (의미가 전해지지 않는다)
IOException
이 발생하면 속성 파일이 없다는 뜻이란다.
그러면 누가 모든 기본값을 읽어 들이는가?
loadedProperties.load()
를 호출하기 전에 읽나?
아니면 loadedProperties.load()
가 예외를 잡아 기본값을 읽어 들인 후 예외를 던져주는가?
저자가 catch
블록을 비워놓기 뭐해서 몇 마디 덧 붙였을 뿐인가?
답을 알아내려면 다른 코드를 뒤져보는 수밖에 없다.
즉 독자와 제대로 소통하지 못하는 주석이다.
- 같은 이야기를 중복하는 주석
// this.closed 가 true 일 때 반환되는 유틸리티 메서드다.
// 타임아웃에 도달하면 예외를 던진다.
public synchronized void waitForClose(final long timeoutMillis) throws Exception {
if (!closed) {
wait(timeoutMillis);
if (!closed)
throw new Exception("MockResponseSender could not be closed");
}
}
헤더에 달린 주석이 같은 코드 내용을 그대로 중복 한다.
이와 같은 주석은
- 자칫하면 코드보다 주석을 읽는 시간이 더 오래 걸린다.
- 주석이 코드보다 더 많은 정보를 제공하지 못한다.
- 실제로 코드보다 부정확해 함수를 대충 이해하고 넘어가게 만든다.
엔진 후드를 열어볼 필요가 없다며 고객에게 아양 떠는 중고차 판매원과 비슷하다.
아래는 쓸모없고 중복된 Javadocs 가 매우 많은 코드이다.
이러한 같은 이야기를 중복하는 주석은 코드만 지저분하고 정신 없게 만든다.
public abstract class ContainerBase
implements Container, Lifecycle, Pipeline,
MBeanRegistration, Serializable {
/**
* 이 컴포넌트의 프로세서 지연값
*/
protected int backgroundProcessorDelay = -1;
/**
* 이 컴포넌트를 지원하기 위한 생명주기 이벤트
*/
protected LifecycleSupport lifecycle = new LifecycleSupport(this);
/**
* 이 컴포넌트를 위한 컨테이너 이벤트 Listener
*/
protected ArrayList listeners = new ArrayList();
...
}
- 오해할 여지가 있는 주석
// this.closed 가 true 일 때 반환되는 유틸리티 메서드다.
// 타임아웃에 도달하면 예외를 던진다.
public synchronized void waitForClose(final long timeoutMillis) throws Exception {
if (!closed) {
wait(timeoutMillis);
if (!closed)
throw new Exception("MockResponseSender could not be closed");
}
}
위에서 본 주석은 중복이 상당히 많으면서도 오해의 여지 또한 존재한다.
closed
가true
로 변하는 순간 에 메서드는 (바로)반환되지 않는다.closed
가true
여야 메서드는 반환된다. (즉시 반환이 아니라는 뜻)
누군가 주석만 읽고 closed
가 true
로 변하는 순간 (즉시)함수가 반환되리라는 생각으로 경솔하게 함수를 호출할지도 모른다.
- 의무적으로 다는 주석
- 모든 함수에 Javadocs 를 달거나 모든 변수에 주석을 달아야 한다는 규칙은 어리석다.
/**
* @param title CD 제목
* @param author CD 저자
* @param track CD 트랙 숫자
* @param durationInMinutes CD 길이(단위: 분)
*/
public void addCD(String title, String author, int tracks, int durationInMinutes) {
...
}
- 이력을 기록하는 주석
- 예전에는 모듈 첫머리에 변경 이력을 기록하고 관리하는 관례가 바람직했다.
당시에는 소스 코드 관리 시스템이 없었으니까.
하지만 이제는 혼란만 가중할 뿐이다. 완전히 제거하는 편이 좋다.
- 예전에는 모듈 첫머리에 변경 이력을 기록하고 관리하는 관례가 바람직했다.
- 있으나 마나 한 주석
- 너무 당연한 사실을 언급하며 새로운 정보를 제공하지 못하는 주석
/**
* 기본 생성자
*/
protected AnnualDateRule() {}
/** 월 중 일자 */
private int dayOfMonth;
/**
* 월 중 일자를 반환한다.
*
* @return 월 중 일자
*/
public int getDayOfMonth() {
return dayOfMonth;
}
위와 같은 주석은 지나친 참견이다.
결국은 코드가 바뀌면서 주석은 거짓말로 변한다.
아래 예시를 보자
private void startSending() {
try {
doSending();
}
catch (SocketException e) {
// 정상. 누군가 요청을 멈췄다.
}
catch (Exception e) {
try {
response.add(ErrorResponder.makeExceptionString(e));
response.closeAll();
}
catch (Exception e1) {
// 이게 뭐야!
}
}
}
첫 번째 주석은 적절해 보인다.
catch
블록을 무시해도 괜찮은 이유를 설명하는 주석이다.
하지만 두 번째 주석은 전혀 쓸모가 없다.
다음과 같이 코드 구조를 개선할 수 있다.
private void startSending() {
try {
doSending();
}
catch (SocketException e) {
// 정상. 누군가 요청을 멈췄다.
}
catch (Exception e) {
addExceptionAndCloseResponse(e);
}
}
private void addExceptionAndCloseResponse(Exception e) {
try {
response.add(ErrorResponder.makeExceptionString(e));
response.closeAll();
}
catch (Exception e) { }
}
마지막 try/catch
블록을 독자적인 함수로 만드는 데 노력을 쏟았어야 했다.
- 무서운 잡음
- 때로는 Javadocs 도 잡음이다.
아래 나오는 (오픈 소스 라이브러리 코드)Javadocs 는 어떤 목적을 수행할까? 닶: 없다.
단지 문서를 제공해야 한다는 잘못된 욕심으로 탄생한 잡음일 뿐이다.
- 때로는 Javadocs 도 잡음이다.
/** The name. */
private String name;
/** The version. */
private String version;
/** The licenceName. */
private String licenceName;
/** The version. */
private String info;
아래 version 은 복붙 하다가 생긴 오류인가 보다.
결국 주석 작성에 주의를 기울이지 않는다면, 읽는 사람은 아무 이익을 얻을 수 없다.
- 함수나 변수로 표현할 수 있다면 주석을 달지 마라
// 전역 목록 <smodule> 에 속하는 모듈이 우리가 속한 하위 시스템에 의존하는가?
if (smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))
위 코드에서 주석을 없애고 다시 표현하면 다음과 같다.
ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees.contains(ourSubSystem))
위와 같이 주석이 필요하지 않도록 코드를 개선하는 편이 더 좋다.
- 위치를 표시하는 주석
// Actions ////////////////////////////////////
일반적으로 위와 같은 주석은 가독성만 낮춘다.
극히 드물게 유용한 경우도 있지만 자주 사용하면 독자가 흔한 잡음으로 여겨 무시한다.
- 닫는 괄호에 다는 주석
- 중첩이 심하고 장황한 함수라면 의미가 있을지도 모르지만,
작고 캡슐화된 함수에는 잡음일 뿐이다.
닫는 괄호에 주석을 달아야겠다고 생각이 든다면 대신에 함수를 줄이려 시도하자.
- 중첩이 심하고 장황한 함수라면 의미가 있을지도 모르지만,
- 공로를 돌리거나 저자를 표시하는 주석
/* 길동이 추가함 */
소스 코드 관리 시스템을 이용하도록 하자.
현실적으로 이런 주석은 그냥 오랫동안 코드에 방치되어 점차 부정확하고 쓸모없는 정보로 변하기 쉽다.
- 주석으로 처리한 코드
- 주석으로 처리된 코드는 다른 사람들이 지우기를 주저한다.
이유가 있어 남겨놓았으리라 생각해서 (중요한 코드일까봐)지우면 안 된다고 생각한다.
이 역시 코드 관리 시스템이 우리를 대신해 코드를 기억해준다.
그냥 코드를 삭제해라. 잃어버릴 염려 없다.
- 주석으로 처리된 코드는 다른 사람들이 지우기를 주저한다.
- HTML 주석
- 소스 코드에서 HTML 주석은 혐오 그 자체다.
주석에 HTML 태그를 삽입해야 하는 책임은 프로그래머가 아니라 도구가 져야 한다.
- 소스 코드에서 HTML 주석은 혐오 그 자체다.
- 전역 정보
- 근처에 있는 코드만 기술하라.
코드 일부에 주석을 달면서 시스템의 전반적인 정보를 기술하지 마라.
- 근처에 있는 코드만 기술하라.
/**
* 적합성 테스트가 동작하는 포트: 기본값은 <b>8082</b>
*
* @param fitnessePort
*/
public void setFitnessePort(int fitnessePort) {
this.fitnessePort = fitnessePort;
}
위 주석은 심하게 중복되었다는 사실 외에도 기본 포트 정보(함수에서 알 필요 없는)를 기술한다.
하지만 함수 자체는 포트 기본값을 전혀 통제하지 못한다.
즉 시스템 어딘가에 있는 다른 함수를 설명하고 있다.
(어딘가에 있는)포트 기본값 설정하는 코드가 변해도 아래 주석이 변하리라는 보장은 전혀 없다.
- 너무 많은 정보
- 주석에다 흥미로운 역사나 관련 없는 정보를 장황하게 늘어놓지 마라.
독자에게 불필요하며 불가사의한 정보일 뿐이다.
- 주석에다 흥미로운 역사나 관련 없는 정보를 장황하게 늘어놓지 마라.
- 모호한 관계
- 주석과 주석이 설명하는 코드는 둘 사이 관계가 명백 해야 한다.
/*
* 모든 픽셀을 담을 만큼 풍부한 배열로 시작한다(여기에 필터 바이트를 더한다).
* 그리고 헤더 정보를 위해 200바이트를 더한다.
*/
this.pngBytes = new byte[((this.width + 1) * this.height * 3) + 200];
여기서 필터 바이트란 무엇일까?
주석을 다는 목적은 코드만으로 설명이 부족해서 다.
그런데 위에서는 주석 자체가 다시 설명을 요구한다.
- 함수 헤더
- 짧고 한가지만 수행하며 이름을 잘 붙인 함수가
주석으로 헤더를 추가한 함수보다 훨씬 좋다.
- 짧고 한가지만 수행하며 이름을 잘 붙인 함수가
- 비공개 코드에서 Javadocs
- (공개 API 가 아닌)시스템 내부에 속한 클래스에 Javadocs 를 생성할 필요 없다.
예제
다음은 에라스토테네스의 체 알고리즘을 구현한 코드를 리팩터링하는 과정을 보여준다.
변경 전
/**
* 이 클래스는 사용자가 지정한 최대 값까지 소수를 생성한다. 사용된 알고리즘은
* 에라스토테네스의 체다.
* <p>
* 에라스토테네스: 기원전 276년에 리비아 키레네에서 출생, 기원전 194년에 사망
* 지구 둘레를 최초로 계싼한 사람이자 달력에 윤년을 도입한 사람.
* 알렉산드리아 도서관장을 역임.
* </p>
* 알고리즘은 상당히 단순하다. 2에서 시작하는 정수 배열을 대상으로
* 2의 배수를 모두 제거한다. 다음으로 남은 정수를 찾아 이 정수의 배수를 모두 지운다.
* 최대 값의 제곱근이 될 때까지 이를 반복한다.
*
* @author 홍길동
* @version 2021.04.03
*/
import java.util.*;
public class GeneratePrimes {
/**
* @param maxValue 는 소수를 찾아낼 최대 값
*/
public static int[] generatePrimes(int maxValue) {
if (maxValue >= 2) { // 유일하게 유효한 경우
// 선언
int s = maxValue + 1; // 배열 크기
boolean[] f = new boolean[s];
int i;
// 배열을 참으로 초기화
for (i = 0; i < s; i++)
f[i] = true;
// 소수가 아닌 알려진 숫자를 제거
f[0] = f[1] = false;
// 체
for (i = 2; i < Math.sqrt(s) + 1; i++) {
if (f[i]) { // i 가 남아 있는 숫자라면 이 숫자의 배수를 구한다.
for (j = 2 * i; j < s; j += i)
f[j] = false; // 배수는 소수가 아니다.
}
}
// 소수 개수는?
int count = 0;
for (i = 0; i < s; i++) {
if (f[i])
count++; // 카운트 증가
}
int[] primes = new int[count];
// 소수를 결과 배열로 이동한다.
for (i = 0, j = 0; i < s; i++) {
if (f[i]) // 소수일 경우에
primes[j++] = i;
}
return primes;
}
else // maxVlue < 2
return new int[0]; // 입력이 잘못되면 비어 있는 배열을 반환한다.
}
}
개선될 점을 대략 파악하면 다음과 같다.
- 너무 많은 정보 (헤더에 역사설명 부분)
에라스토테네소: 기원전 276년에 리비아 키레네에서 출생
등
- 의무적으로 다는 Javadocs 주석
@author 홍길동
등
- 모호한 class 명
GeneratePrimes
보다는Generator
가 좀 더 명시적
- 코드가 바뀌면 덩달아 바뀌어야 하는 주석들
@param maxValue 는 소수를 찾아낼 최대 값
- 모호한 변수 네이밍
int s = ...
boolean[] f = ...
int count = ...
등
- 명시적인 함수로 대체
if (maxValue >= 2) // 유일하게 유효한 경우
등generatePrimes()
메서드 내 대부분은 함수로 쪼개야 한다. (함수가 여러가지 일을 함)
- 증복된 내용
count++; // 카운트 증가
등
위와 같은 문제점들을 리팩터링 해보자.
리팩터링 결과
/**
* 이 클래스는 사용자가 지정한 최대 값까지 소수를 구한다.
* 알고르짐은 에라스토테네스의 체다.
* 2에서 시작하는 정수 배열을 대상으로 작업한다.
* 처음으로 남아 있는 정수를 찾아 배수를 모두 제거한다.
* 배열에 더 이상 배수가 없을 때까지 반복한다.
*/
public class PrimeGenerator {
private static boolean[] crossedOut;
private static int[] result;
public static int[] generatePrimes(int maxValue) {
if (maxValue < 2)
return new int[0];
else {
uncrossIntegersUpTo(maxValue);
crossOutMultiples();
putUncrossedIntegersIntoResult();
return result;
}
}
private static void uncrossIntegersUpTo(int maxValue) {
crossedOut = new boolean[maxValue + 1];
for (int i = 2; i < crossedOut.length; i++)
crossedOut[i] = false;
}
private static void crossOutMultiples() {
int limit = determineIterationLimit();
for (int i = 2; i <= limit; i++) {
if (notCrossed(i))
crossOutMultiplesOf(i);
}
}
private static int determineIterationLimit() {
// 배열에 있는 모든 배수는 배열 크기의 제곱근보다 작은 소수의 인수다.
// 따라서 이 제곱근보다 더 큰 숫자의 배수는 제거할 필요가 없다.
double iterationLimit = Math.sqrt(crossedOut.length);
return (int) iterationLimit;
}
private static boolean notCrossed(int i) {
return crossedOut[i] == false;
}
private static void putUncrossedIntegersIntoResult() {
result = new int[numberOfUncrossedIntegers()];
for (int j = 0, i = 2; i < crossedOut.length; i++)
if (notCrossed(i))
result[j++] = i;
}
private static int numberOfUncrossedIntegers() {
int count = 0;
for (int i = 2; i < crossedOut.length; i++)
if (notCrossed(i))
count++;
return count;
}
}
- 들어가면서
- 1장 깨끗한 코드
- *2장 의미 있는 이름
- 3장 함수
- 4장 주석 «
- 5장 형식 맞추기
- 6장 객체와 자료 구조
- 7장 오류 처리
- 8장 경계
- 9장 단위 테스트
- 10장 클래스
- 11장 시스템
- 12장 창발성(創發性)
- 13장 동시성
- 14장 점진적인 개선
- 15장 JUnit 들여다보기
- 16장 SerialDate 리팩터링
- 17장 냄새와 휴리스틱