[클린코드] 4장. 주석

스터디 메인 페이지

목차

- ☆ 표시가 붙은 부분은 스터디 중 나온 얘기 혹은 제 개인적인 생각이나 제가 이해한 방식을 적어놓은 것으로, 책에서 말하고자 하는 바와 다를 수 있습니다.

- 모든 이미지의 출처는 클린 코드(로버트 C. 마틴 저) 책 입니다.

 

---

 

4장 주석

⚈ "나쁜 코드에 주석을 달지 마라. 새로 짜라."

 

⚈ 주석은 언제나 실패를 의미한다. 주석을 달 때마다 자신에게 표현력이 없다는 사실을 푸념해야 마땅하다.

• 주석은 거짓말을 한다. 주석은 오래될수록 코드에서 멀어진다. 프로그래머들이 주석을 유지하고 보수하기란 현실적으로 불가능하다.

 

---

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

⚈ 코드에 주석을 추가하는 일반적인 이유는 코드 품질이 나쁘기 때문이다. 자신이 저지른 난장판을 주석으로 설명하려 애쓰는 대신에 그 난장판을 깨끗이 치우는 데 시간을 보내라!

 

---

코드로 의도를 표현하라!

⚈ 많은 경우 주석으로 달려는 설명을 함수로 만들어 표현해도 충분하다.

// 직원에게 복지 혜택을 받을 자격이 있는지 검사한다.
if ((employee.flags & HOURLY_FLAG) && (employee.age > 65))

VS

if (employee.isEligibleForFullBenefits())

 

---

좋은 주석

⚈ 어떤 주석은 필요하거나 유익하다.

• 법적인 주석
• 정보를 제공하는 주석 (e.g. 정규표현식이 시각과 날짜를 뜻한다고 설명하는 주석. -> ☆ 주석 없이 상수로 빼는게 더 좋을 것 같긴하다.)
• 의도를 설명하는 주석 (☆ 예를들어 알고리즘 기법이 사용됬다면 코드를 아무리 잘 표현해도 배경지식이 없다면 코드 해석이 불가하다. 이런건 작성한 알고리즘적 의도를 작성해주는게 맞다.)
• 의미를 명료하게 밝히는 주석 : 인수나 반환값이 표준 라이브러리나 변경하지 못하는 코드에 속한다면 의미를 명료하게 밝히는 주석이 유용하다.)
• 결과를 경고하는 주석 (e.g. "// SimpleDateFormat은 스레드에 안전하지 못한다. 따라서 각 인스턴스를 독립적으로 생성해야 한다.")
• TODO 주석 : 앞으로 할 일을 남겨두는 것. 단, 나쁜 코드를 남겨 놓는 핑계가 되어서는 안 된다.
• 중요성을 강조하는 주석 (e.g. "// 여기서 trim은 정말 중요하다. 문자열에 시작 공백이 있다면 다른 문자열로 인식된다.")

 

⚈ ☆ TODO 주석의 경우 IntellJ에서 목록을 볼 수 있다.

 

---

나쁜 주석

⚈ 일반적으로 대다수 주석은 허술한 코드를 지탱하거나, 엉성한 코드를 변명하거나, 미숙한 결정을 합리화하는 등 프로그래머가 주절거리는 독백에서 크게 벗어나지 못한다.

 

⚈ 이해가 안 되어 다른 모듈까지 뒤져야 하는 주석은 독자와 제대로 소통하지 못하는 주석이다. 그런 주석은 바이트만 낭비할 뿐이다.

 

⚈ 같은 이야기를 중복하는 주석을 피하자

• 주석이 코드 내용을 그대로 중복하는 경우를 피해라.
• 주석이 코드보다 더 많은 정보를 제공하지 못한다. 코드보다 읽기가 쉽지도 않다. 코드보다 부정확하다.

 

⚈ 오해할 여지가 있는 주석을 피하자

 

⚈ 의무적으로 다는 주석을 피하자

• 모든 함수에 Javadocs를 달거나 모든 변수에 주석을 달아야 한다는 규칙은 어리석기 그지없다.
• 아무런 가치도 없다.

 

⚈ ☆ Javadocs란?

대학생때 작성했던 Javadocs 이다. 연구 프로젝트라 사실 의무적으로 달아야했다 ㅋㅋㅋ

 

  위와 같이 양식에 맞춰 적어두면 아래와 같이 문서로 뽑을 수 있게 된다.

 

  또한 우리가 IDE에서 함수 설명 보는 것도 위와 같이 적어둔 Javadocs 주석이다.

⚈ 이력을 기록하는 주석을 피하자

• 현재는 VCS(Version Control System)가 있으므로 의미가 없다.

 

⚈ 있으나 마나 한 주석을 피하자

• 생성자 위에 "// 기본 생성자"를 적어두는 것 처럼 의미 없는 주석을 피하자.

 

⚈ 함수나 변수로 표현할 수 있다면 주석을 달지 마라

 

⚈ 닫는 괄호에 다는 주석

• 애초에 이 책에 나온대로 작고 캡슐화된 함수에는 필요가 없다.
• 닫는 괄호에 주석을 달아야겠다는 생각이 든다면 대신에 함수를 줄이려 시도하자.

 

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

• VCS가 있으므로 이것도 의미 없다.

 

⚈ 주석으로 처리한 코드

• 주석으로 처리된 코드는 다른 사람들이 지우기를 주저한다. 이유가 있어 남겨놓았으리라고, 중요하니까 지우면 안 된다고 생각한다.
• 질 나쁜 와인병 바닥에 앙금이 쌓이듯 쓸모 없는 코드가 점차 쌓여가게 된다.

 

⚈ HTML 주석을 피하자

• Javadocs와 같은 도구로 주석을 뽑아 웹 페이지에 올릴 작정이라면 주석에 HTML 태그를 삽입해야 하는 책임은 프로그래머가 아니라 도구가 져야한다.
• ☆ 도구가 져야한다고만 써있는데, 어떻게 하는건지 잘 모르겠다. 코드에 Javadocs까지 작성하고 싶은 경우 어떻게 하란건지?! 주석에 필요한 정보가 들어가야만 하고, 그 중에 코드가 존재할 경우 물론 별도 문서로 만들수도 있지만 주석으로 처리하게 될 수도 있다. 그 경우 <pre>를 안붙일수가 없다. 또 강조가 필요할 경우에도 HTML 주석을 쓰지 않고 어떻게 해야할지 모르겠다. 주석은 아니지만 REST API 작성 시 스웨거 문서 생성시에도 HTML을 작성하는 경우가 있다. 최대한 지양하되, 꼭 필요하다면 해야하지 않나 싶다.

대학생때 작성했던 Javadocs인데 강조를 위해 HTML 주석을 썼었다.

 

⚈ 전역 정보

• 주석을 달아야 한다면 근처에 있는 코드만 기술하라. 코드 일부에 주석을 달면서 시스템의 전반적인 정보를 기술하지 마라.

 

⚈ 너무 많은 정보

• 주석에다 흥미로운 역사나 관련 없는 정보를 장황하게 늘어놓지 마라.

 

⚈ 모호한 관계

• 이왕 공들여 주석을 달았다면 적어도 독자가 주석과 코드를 읽어보고 무슨 소린지 알아야 한다.

 

---

☆ 실습 해봤다.

⚈ 내 경우에 원래도 주석을 좀 싫어해서 잘 안다는 편이었다. 그래서 주로 주석을 달게되면 '의도를 설명하는 주석' 형태로 많이 달게된 것 같다. 이하 코드는 JWT 토큰을 기반으로 유저 정보를 획득해둔 userInfo에 varargs인 requiredRoles에 해당하는 역할이 포함되어 있는지 확인하는 코드이다. 익명 부분을 설명하기 위해 주석을 달았었다.

public class RoleChecker {
    public boolean chkRole(UserInfo userInfo, String... requiredRoles) {
        if (userInfo == null || requiredRoles == null || requiredRoles.length == 0)  // 필요한게 없음 -> 익명도 가능
            return true;

        for (String role : requiredRoles) {
            if (role.equals(ROLES.NONE))    // 권한이 필요없을 경우 -> 익명도 가능
                return true;
            if (userInfo.hasRole(role))
                return true;
        }
        return false;
    }
}

 

⚈ 클린코드를 읽으며 다시 코드를 봐보니 함수만으로 충분히 설명할 수 있었다. 그래서 리팩토링 해봤다.

• isAnonymousPermissionPossible로 우선 권한 없이 익명으로도 가능한지 확인한다. 이 경우 해당 함수에서도 모든 requiredRoles 목록을 확인해야 하므로 시간복잡도는 O(N+N)으로 늘어나게 되겠지만, requiredRoles에 포함될 역할의 수가 애초에 적으므로 문제가 되지 않는다.
• 이후 익명 이외의 역할들에 대해 체크한다.
• 다시 보니 함수 자체가 boolean을 리턴하는게 좀 명확하지 않은 것 같다. 이하 코드에 적용은 하지 않았지만, "권한이 유효한가" 정도의 의미로 바꿔야 boolean으로 리턴하는게 더 이해하기 쉬울 것 같다.

public class RoleChecker {
    public boolean userInfoHas(UserInfo userInfo, String... requiredRoles) {
    	if (isAnonymousPermissionPossible(requiredRoles))
        	return true;

        for (String role : requiredRoles) {
            if (userInfo.hasRole(role))
                return true;
        }
        
        return false;
    }
}