CleanCode - 함수

함수

이번 챕터는 함수를 어떻게 하면 가독성 좋게 잘 만들 수 있는지 알려준다.
이론적으로 무엇이 좋더라 에서 멈추는 것이 아니라 무작정 따라해보면 좋은 지침들을 통해 어떻게 좋은 함수를 만들 수 있는지 알려준다.
단, 무작정 따라하는 것은 좋지만 맹신하고 맹목적으로 추종하진 말자. 분명 예외 케이스도 있을 것이다.
(https://www.youtube.com/watch?v=th7n1rmlO4I&ab_channel=%EC%BD%94%EB%94%A9%EC%95%A0%ED%94%8C)

작게 만들어라

책에서 첫 번째 규칙은 작게 만들라 한다. 구체적인 증거는 없지만 저자의 수십년간 경험을 통해 쌓인 빅데이터를 활용해 세운 규칙으로 보인다.
우테코 프리코스에서 주는 과제를 할 때는 함수의 길이가 15 줄을 넘지 않도록 하는 룰이 있었다.
무조건 맹신하여 n줄 이하로 만들어야해! 라는 것은 잘못되었다 생각하지만 내 경험으로도 짧은 함수가 읽기도, 고치기도 쉬운 경우가 더 많긴 하다.
단, 함수의 길이를 줄이기 위해 2~3줄 짜리 함수를 또 만드는 것은 개인적으로 잘못되었다 생각한다. 뭐든 적당히가 좋지 않을까?

들여쓰기를 줄여라

이것도 가독성을 위해, 리팩토링을 위해 지키면 좋은 지침으로 생각한다. 들여쓰기가 너무 많으면(inden가 깊으면) 가독성도 떨어지고 해당 함수의 길이 또한 길어질 뿐 아니라 더 나아가 생각해보면 너무 많은 역할을 하고 있을 확률이 크다.

개인적으로 들여쓰기 두번 정도는 나쁘지 않다 생각한다.

한 가지만 해라!

함수가 길다는 것은 그 함수는 여러가지 일을 하고 있다는 것이라 할 수 있다. 모든 긴 함수가 여러가지 일을 할까? 에 맞다고 확신할 수 없지만(지식의 공백으로 인해 자신감 또한 부족) 아주 몹시 매우 높은 확률로 맞다고 확신한다.

함수가 한 가지 기능만 하는지 판단하기 위해 책에서는 지정된 함수 이름 아래에서 추상화 수준이 하나인 단계만 수행한다면 그 함수는 한 가지 작업만 한다. 라고 한다.

여기서 내가 생각하는 추상화란 어떤 코드(동작)이 자세하게 기술되어 있는가(세부 동작), 자세하게 기술되어 있지 않은가(메서드)를 기준으로 해야 한다 생각한다.
즉, 세부 동작이 바로 보이게 된다면 해당 코드는 추상화 레벨이 낮은 것이고, 세부 동작이 함수 안에 기술되어 있어 특정 함수 A에서 함수 B를 호출하여 B가 보이지 않는다면 해당 함수 B는 함수 A 내부에서 추상화 레벨이 높다 생각한다.

함수 당 추상화 수준은 하나로

한 함수에 추상화 수준을 섞게 된다면 어떤 표현이 어떤 단계의 추상화인지 구분하기 어렵게 될 수 있다. 어렵기만하고 구분이 된다면 문제가 덜하겠지만, 여러 추상화 수준이 섞이게 되면 깨진 창문 이론처럼 해당 함수에 이것저것 세부사항을 점점 추가하여 더욱 알아보기 힘든 함수를 만들게 된다.

내려가기 규칙

코드는 위에서 아래로 이야기 읽듯 흘러가야 좋다 한다.
한 함수 다음에는 추상화 수준이 한 단계 낮은 함수가 오게 하도록 하는 것이 좋다

someMethodBigA -> someMethodSmallA -> someMethod 순서로 추상화 계층이 낮아진다.

public class SomeClass {
    void someMethodBigA(){
        ....
        someMethodSmallA();
        ....
    }

    void someMethodSmallA(){
        ....
        someMethod();
        ....
    }

    void someMethod(){
        ....
    }
}

이것저것 생각하며 추상화 수준이 하나인 함수를 구현하는 것이 여간 어려운 일이 아니다. 많은 프로그래머들(나 포함) 곤란을 많이 겪을텐데 머리는 알지만 손이 따라주지 않는다.
위에서 아래로 읽어 내려가듯 코드 구현하면 조금 더 쉽다고 한다.
나같은 경우는 일단 한 메서드에 구현해도 커밋 전 조금 길다 싶은 메서드나, 조건문 안에 있는 메서드, 조건절 정도를 메서드 추출하여 계층을 나누긴 한다. (얼마나 효과가 좋을지는 모르겠다)

조건문 (Switch, If 문)

public Money calculatePay(Employee e) throws InvalidEmployeeType {
    switch(e.type) {
        case COMMISSIONED:
            return calculateCommissionedPay(e); 
        case HOURLY:
            return calculateHourlyPay(e);
        case SALARIED:
            return calculateSalariedPay(e);
        default:
            throw new InvalidEmployeeType(e.type);
    }
}

위 코드는 문제점이 여럿 있다.

1. 함수가 길다(새로운 유형 추가에 따라 더욱 길어질 여지가 매우 높다)
2. 한 가지 작업만 하지 않는다 (여러 종류의 직원 유형에 대한 급여를 계산하고, 이러한 계산을 각각의 유형에 분기하여 처리)
3. 코드를 변경할 이유가 여럿이다. 즉, 한 번에 한 가지 이유만을 통해 코드가 변경되지 않을 것이다.
4. 새 직원을 추가할 때마다 코드를 변경해야 한다
5. 제일 최악은 위와 같은 구조가 사용하는 메서드마다 있을 수 있다.

예를 들어 isPayday(Employee e, Date date); 혹은 deliverPay(Employee e, Money pay); 같은 것이 있겠다.
특정 유형이 추가될 때 마다 isPayday와 deliverPay을 수정해야 한다. 얼마나 복잡한가?

조건문을 없애기 위한 디자인 패턴으로 추상 팩토리 라는 디자인 패턴을 책에서 제시한다.

서술적인 이름을 사용하라!

함수 네이밍에 대한 설명이다. 간략하게 책 내용을 바탕으로 내 의견을 말하자면 길어도 괜찮으니 함수가 어떤 동작을 하는지 동사로 설명 하라는 것으로 보인다.
예를 들어 이건 XX하는 OOO다 와 같이 서술하면 괜찮은 함수 이름이라 한다.
하나 더 말하면 한 가지 일만 하는 함수일수록 이름 만들기 더 쉽다 한다.

함수 인수

함수에서 가장 이상적인 인수는 0개다. 적으면 적을수록 좋다.
인수가 많다면 여러가지 문제가 있다.

1. 함수가 하는 일이 많아진다
2. 함수를 실행하기 위해 필요한 준비물(인수)들이 많아지고, 잘못 입력할 수 있다.
3. 테스트 코드 작성이 어렵다

개인적으로 3번 테스트 코드 작성에 대한 공감이 현재 거의 없는 상태이다. (약간은 공감하지만 테스트 코드를 작성한 경험이 많이 없다.)

단항

매우 흔한 경우로 여러 경우가 있겠지만 아래 경우만 소개한다.

1. 인수를 받아 어떤 변환을 거쳐 결과를 반환
2. 입력은 있지만, 출력이 없고 시스템 상태를 바꾸는 이벤트 함수
3. 입력 인수를 변환하여 아무 것도 반환하지 않는 함수
4. 입력 인수를 변환하여 변환한 값을 반환하는 함수

이 중 3번만 지양하라 한다. 변환 후 해당 인자를 통해 뭔가 하지 않는다면 상태 변화를 추적하기 어렵기 때문이 아닐까 생각한다.

플래그

함수 인자로 boolean을 넘기면 함수가 여러 일을 한다고 대놓고 광고하는 꼴이다. boolean 인자를 통해 무언가 판단을 하고 판단에 따라 동작이 나뉜다는 뜻이다.

인수 객체

글에서는 생략했지만 인자가 2-3개를 넘어가는 순간 일부를 독자적 클래스로 선언하는 것을 고려하라 한다.
눈속임이라 여길 수 있겠지만 결국 어떠한 개념 을 클래스로 묶기 때문에 관리 측면, 함수를 호출하는 측면에서도 더 좋을 것이다.

동사와 키워드

함수의 의도나 인수, 순서를 제대로 표현하기 위한 함수 이름을 생각해보자.
단항 함수 write(name)은 동사/명사 쌍이기 때문에 매우 직관적이다.(이름이 무엇이든 쓰겠다) 이보다 진화하면 writeField(name)이 된다. 이름이 field라는 사실도 알려줄 수 있다.
마지막은 함수에 키워드를 추가한다.
즉, 함수 이름에 인수 이름을 넣는다. 책에서는 assertEquals보다 assertExpectedEqualsActual(expected, actual)이 더 좋다 한다.
다른 예로는 SpringDataJpa에 findById와 같은 것이 있을 수 있다.

부수 효과를 일으키지 마라!

함수에서 예상하지 못 하는 부수 효과를 만들면 안 된다.
클래스 변수나, 전역 변수에 접근하여 어떤 상태를 변화시키면 추적이 매우 어렵고 위험한 동작이 될 수 있다.
책에서는 함수의 인자로 넘어온 값도 수정하지 말라 하는데, 개인적으로 이정도 유도리는 있어도 되는 것이 아닐까.. 생각한다.

부수 효과를 발생시키는 함수가 있는 경우 결합을 초래한다. 어떤 것과의 결합이냐면 해당 상태 변경을 해도 되는 상황 과의 결합이다.
잘못 호출하는 경우 의도치 않는 상태 변경이 발생하기 때문이다. 전역 변수 혹은 클래스 변수 상태 변화가 발생하는 함수면 함수 이름에 명시를 하는 것이 좋겠다.

출력 인수

이해하지 못 했다. 단지 내가 이해한 내용은 함수에서 상태 변경이 필요하면 함수가 속한 객체의 상태만 변경하라는 것으로 이해된다.

명령과 조회를 분리하라!

함수는 명령만 해서 상태를 변경하든지, 조회만 해서 값을 가져오든지 하나만 해야한다.

if (set("username", "unclebob")) 전혀 이해가지 않는다 username을 unclebob으로 변경? unclebob을 username으로 변경? 아니면 username이 unclebob인지 확인? 무엇인지 모른다.

if (attributeExists("username")) { 
    setAttribute("username", "unclebob");
}

이 코드가 훨씬 직관적이다. 조건문으로 조회 후 내부에서 변경한다.

오류 코드보다 예외를 사용해라!

조건문을 통해 오류 코드를 처리하면 오류 코드에 따른 조건 문기가 오류 코드 만큼 생길 것이고 복잡한 함수를 만들게 될 것이다.(이름 만들기는 쉬울 거 같다.)
오류 코드에 따라 조건 분기를 할 것이 아니라 try-catch를 통해 예외 처리를 하면 코드가 깔끔해진다.

try-catch 블록 뽑아내기

어설픈 try-catch는 오류 처리와 정상 동작을 뒤섞어 문제를 만들 수 있다.
try-catch도 하나의 동작이기 때문에 높은 수준의 추상화라고 생각할 수 있다.
try로 시작했다면 catch/finally로 끝나야하는 것이 마땅하다.

public void delete(Page page) {
    try {
        deletePageAndAllReferences(page);
    } catch (Exception e) {
        logError(e);
    }
}

private void deletePageAndAllReferences(Page page) throws Exception {
    deletePage(page);
    regstry.deleteReference(page.name);
    configKeys.deleteKey(page.name.makeKey());
}

private void logError(Exception e) {
    logger.log(e.getMessage());
}

의존성 자석

오류 코드와 조건문을 통해 처리하는 것은 어디선가 오류 코드 클래스를 정의했다는 것이다.
오류 코드 클래스를 정의해서 여러 클래스에서 사용할 때 변경이 발생하면 사용하는 클래스 어딘가에서 예상치 못한 부수효과가 발생할 수 있다.

반복하지 마라!

반복되는 코드가 등장하는 경우 정책 수정이 요구될 때 반복되는 코드 모든 것을 고쳐야 버그가 없이 고쳐지고, 코드 길이 또한 문제다.
이 얼마나 비효율적인가
단 유사한 것과 같은 것은 구분하자. 지나친 추상화는 가독성을 더 낮출 수 있으며 현재는 같아 보이지만 결국 다른 것일 때 (도메인이 다른 경우 자주 발생한다고 한다.) 추상화 구조를 깨트리기 위해 더 많은 수고가 투입된다.