Skip to content

Latest commit

 

History

History
368 lines (253 loc) · 13.1 KB

ch04_주석.md

File metadata and controls

368 lines (253 loc) · 13.1 KB
Raw

나쁜 코드에 주석을 달지 마라. 새로 짜라 - 브라이언 @. 커니핸, P.J. 플라우저

언어자체가 표현력이 풍부하다면 주석은 필요하지 않다.

주석을 작성했다는 것은 실패를 만회하기 위함이다. 잔인하지만 자신에게 표현력이 없다는 사실을 생각하며 주석을 작성해야 한다.

생각해보자. 코드는 유지보수되고, 코드 그 자체는 “자신이 하는 일” 을 진실되게 얘기 할 수 있다 (의도적으로 함수 명을 반대로 짓지 않은 이상 그럴듯) .

하지만 주석은 코드 변화를 따라 그 버전을 유지 보수 하기란 현실적으로 힘들다.

따라서 애초에 코드의 표현력을 강화해, 주석이 필요 하지 않은 방향으로 작성하는게 베스트!! 🔥

주석은 나쁜 코드를 보완하지 못한다.

가끔 이런 경우가 있다.

“이 코드 너무 지저분한데? → 이런..주석이라도 달아야지”

그럴 시간에 “코드를 정리” 하라!

코드로 의도를 표현하라

이런 코드는 주석이 없다면, 의도 파악이 어렵다고 생각될 것이다.

...
// 복지 혜택을 받을 자격이 있는 직원인지 검사
if (employee.flags & HOURLY_FLAG) & (employee.age > 65) { .. }
...

사실 그렇지는 않음.

이 부분을 뭐… 객체 내부로 분리 하고 && 표현력을 가진 함수로 분리 하면 다음과 같아짐

if(employee.isEligibleForFullBenefits())

(주석 아니어도 코드로 표현하면 됨 )

좋은 주석

책에서 이 때는 주석을 사용해도 돼~ 라고 예시를 들어주고 있다.

하지만 “법적인 주석” 과 “코드 만으로는 설명할 수 없는 이유를 나타낸 주석” 외에는, 그냥 주석 대신 표현력이 좋은 코드를 작성하는게 더 좋아보인다.

법적인 주석

  • 클래스 파일 첫 머리에 “저작권, 소유권 정보”
  • 모든 법적인 정보를 명시할 필요는 없음. 외부 참조 할 수 있도록 참조를 걸어두자

정보 제공 주석

하지만 이 부분도, 주석 보다는 변수명이나 함수 “이름으로 정보를 담는” 것이 더 좋아보인다.

의도를 설명하는 주석

구현과, 의도를 설명하는 주석을 달고는 한다.

( 근데 이러는 경우도 거의 없지 않나..생각이 든다 )

// 스레드를 대량 생성하는 방법으로 어떻게든 race condition 을 만들기 위한 시도
for (int i = 0 ; i < 25000, i++) {
	...
	Thread thread = new Thread(widgetBuilderThread);
	thread.start();
}

의미를 명료하게 밝히는 주석

  • 모호한 “인수, 반환값” && “인수,반환값”이 표준라이브러리나 변경하지 못하는 코드에 속한 경우
public void testCompareTo() throws Exception {
	WikiPagePath a = PathParser.parse("PageA");
	WikiPagePath ab = PathParser.parse("PageA.PageB");
	WikiPagePath b = PathParser.parse("PageB");
	WikiPagePath aa = PathParser.parse("PageA.PageA");
	WikiPagePath bb = PathParser.parse("PageB.PageB");

	assertTrue(a.compareTo(a) == 0 ); // a == a
	assertTrue(a.compareTo(b) != 0); // a != b
	assertTrue(aa.compareTo(ab) == -1); // aa < ab
  • 하지만.. 주석이 올바른지 검증하는 것은 항상..쉽지 않다. 주석이 코드의 의미에 맞는지 까지 검증해야 하니까.
    • 의미를 명료하게 밝히는 주석이 필요하면서도, 주석이 위험한 이유다.

결과를 경고하는 주석

//오래걸리는 테스트입니다

Junit4 부터는 이렇게 할 수도 있긴함

@Ignore("실행이 너무 오래 걸리는 테스트")

스레드 세이프 하지 않은 객체를 리턴 하는 정적함수의 경우에는 특히 주석을 달아주면 좋을 것이다

public static SimpleDateFormat makeStandardHttpDateFormat(){
	// SimpleDateFormat 은 안전하지 않기에, 각 인스턴스를 독립적으로 생성해야 한다 
	..
	return df;
}

TODO 주석

  • 프로그래머가 필요하다고 여기지만, 당장 구현 하기는 어려운 업무를 기술하면 좋다 다음과 같은 용도로 사용하면 좋다.
    • 더 이상 필요 없는 기능 삭제를 알리기
    • 누군가에게 이 부분을 봐달라고 요청하기
    • 부탁하기
    • 앞으로 발생할 이벤트에 맞춰 코드를 고치라는 주의
  • 하지만 TODO 주석을 남기는것이 “ 나쁜 코드를 남기는 것에 대한 핑계가 되어선 안됨!” (찔린다)
//TODO 현재 필요하지 않다
// 체크아웃 모델을 도입하면 함수가 필요 없다 ( 앞으로 발생할 이벤트에 맞춰 코드를 고치라는 주의 에 해당)

IDE 에서 TODO 목록들 확인 가능하니, TODO 도 정기적으로 관리하자

중요성을 강조

다른 사람들이 대수롭게 여기지 않을 수도 있는 부분을 강조하기 위해

// trim 으로 문자열의 시작 공백을 제거해야 한다. 문자열 시작 공백이 있으면 다른 문자열로 인식되기 때문
String listItemContent = match.group(3).trim();

공개 API 의 javadocs

공개 API 작성시에는 좋은 주석을 생각하며 훌륭한 Javadocs 를 작성해야 한다.

나쁜 주석

주절거리는 주석

코드의 어느 부분에 대한 주석인지도 안 적혀 있고, 상황에 대한 설명도 거의 없이 주절거리는 주석일 경우

→ 이 주석의 의미가 뭔지를 파악하기 위해, 여러 모듈들을 뒤져봐야 함.

public void loadProperties() {
    try {
        String propertiesPath = propertiesLocation + "/" + PROPERTIES_FILE;
        FileInputStream propertiesStream = new FileInputStream(propertiesPath);
        loadedProperties.load(propertiesStream);
    } catch (IOException e) {
        // 속성 파일이 없다면 기본값을 모두 메모리로 읽어 들였다는 의미다. 
    }
}

코드만 봐도 충분할 같은 이야기를 중복하는 주석

코드 내용을 그대로 중복 - 추가적인 정보를 제공하는 것도 아닌데, 읽기도 쉽지 않은 경우

→ 코드 읽는 시간보다 주석 읽는 시간이 더 오래걸리게 됨.

// 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");
        }
    }
}

톰캣에 이런 코드가 있음

public abstract class ContainerBase extends LifecycleMBeanBase implements Container {
	/**
     * The cluster with which this Container is associated.
     */
    protected Cluster cluster = null;

ContainerBase 에 있는 Cluster 이니 당연히 컨테이너랑 관련된 클러스터임을 유추 가능한데, 굳이 주석으로 중복해서 말하고 있음.

오해할 여지가 있는 주석

아래코드는 주석을 보고, 코드를 읽으면 매우 혼란스러워집니다… 차라리 코드만 읽었다면 “double checking 을 하는 동기화 코드군” 이라고 알 수 있을텐데 말이죠

// 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 를 달거나, 주석을 달아야 한다는 규칙은 매우 안 좋음.
    • 문서를 제공해야한다는 욕심…
    • 주석을 쓰려고 말을 막 쓰거나, 복붙을 하다가 잘못된 주석을 퍼트리게 될 가능성 농후

이력 기록용 주석

  • 예전에는 모듈을 편집할 때 마다, 지금까지 모듈에 가한 변경을 모두 기록하는 주석을 달았음.
    • 이 때는 “소스 코드 관리 시스템이 없었으니까” → 변경이력을 주석으로 관리했던 거.
    • 이젠 필요없음. 완전히 삭제하자

있으나 마나 한 주석

  • 너무 당연한 사실을 언급한 것.
  • 새로운 정보를 제공하지 못하는 것. → 앞에서 얘기한 “중복된 정보를 제공하는 주석” 과 비슷함.
/** 월 중 일자 **/
private int datOfMonth;
  • 심지어는 코드에 분풀이하는 주석을 남기기도.. → 그런 주석 달 시간에 리팩토링이나 하자
  • 개발자는 이런 주석 읽다보면 “ 아 여기 주석들 읽을 필요 없을 듯..” 하고 “주석을 무시하는 습관” 에 빠져 버림.

함수나 변수로 표현할 수 있다면 주석을 달지 마라 ( 가장 중요 )

// 전역 목록 <smodule>에 속하는 모듈이 우리가 속한 하위 시스템에 의존하는가?
if (module.getDependSubsystems().contains(subSysMod.getSubSystem()))

“.”도 많음.

ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees**************************************************.contains(ourSubSystem))**************************************************

위치를 표시하는 주석

// Actions //////////////////////

보통 이런 “//////” 주석들은 “배너” 를 위해 존재함.

배너는 눈에 띠어야 하는데 “///” 를 남용하면, 무시하고 싶은 주석이 되고 배너는 눈에 안 띠게 됨.

닫는 괄호에 다는 주석

  • 닫는 괄호에 주석을 달아야 겠다는 생각이 들었다 ? → 함수를 줄이려는 시도를 해야 한다
    • ex) 중첩이 심한 코드에서 “ ~~까지는 ~~블록이다” 이런거 표시하려고 닫는 괄호에 주석 달아놓는 경우 있음. → 중첩 코드를 분리해버려
public class wc {
	public static void main(String[] args) {
		BufferedReader in = new BufferedReader(new InputStreamReader(System.in));
		String line;
		int lineCnt = 0; int charCnt = 0; int wordCnt = 0;
		try {
			while((line = in.readLine()) != null ) {
				lineCnt++;
				charCnt += line.length();
				String words[] = line.split("\\W");
				wordCnt += words.length;
			} // while
			System.out.println("wordCnt =" + wordCnt);
			System.out.println("charCnt =" + charCnt);
		} // try 
	} catch (Exception e) {
	} // catch 
}

공로를 돌리거나 저자를 표시하는 주석

  • 우리한테는 “소스 코드 관리 시스템(git같은)” 이 있음. 이거로 충분함!
    • 오히려 주석은 버전관리가 더 어렵죠

주석으로 처리한 코드

// InputStream resultStream = formatter.getResultStream();

그냥 지우자!!

이거 보면, 아 이유가 있어서 남겨놨나봐… 생각하는데 그냥 지워라!

  • 소스코드관리 시스템이 우리 대신해서 코드를 기억해 줌. 남겨놔야 할 코드는 없음.

HTML 주석

혐오 그 자체.

  • 주석에 HTML 태그를 삽입해야 할 책임은 프로그래머가 아닌, 도구가! 져야 한다.
/**
* 적합성 테스트를 수행하기 위한 과업
* <p/>
**/

전역 정보 ( 근처에 있는 코드에 주석을 달라 )

  • 주석 근처에 있는 코드를 설명하는게 아니라면 → 주석이 코드 변화를 따라가기는 매우매우 어려울 것

너무 많은 정보

장황한 역사나, 관련없는 정보들을 늘어놓지 말자

모호한 관계

  • 주석과 주석이 설명하는 코드 사이 관계가 명백해야 함
// 모든 피셀을 담을 만큼 충분한 배열로 시작한다 ( 이 과정에서 필터 바이트를 더한다 )
// 그리고 헤더 정보를 위해 200 바이트를 더한다 
this.pngBytes = new byte[((this.width + 1 ) * this.height * 3 ) + 200 ];

주석을 보니 “필터 바이트” 를 더하는 과정이 있다고 함 → 근데 어디가 필터 바이트를 더하는 부분이지..?

적어도 이런식으로라도 해 줬다면 주석도 없고, 애매한 주석도 없었을텐데.. ( 아래 코드가 필터 바이트 부분인지 나는 모름. 그냥 예시 )

int filterByte = this.width + 1;

함수 헤더

  • 짧고 한 가지만 수행하며, 이름을 잘 붙인 함수가 주석을 추가한 함수보다 훨 좋다

비공개 코드에 javadocs 는 쓸모 없다

최종 예제

책에 나온 코드를 적으신 분이 계시네욥

  • 리팩토링 전 코드

generate-primes-cleancode-benchmark/GeneratePrimes.java at master · siom79/generate-primes-cleancode-benchmark

  • 리팩토링 후 코드

generate-primes-cleancode-benchmark/GeneratePrimesCleanCode.java at master · siom79/generate-primes-cleancode-benchmark