[클린코드] 2장 함수

클린 코드

 

함수 🙌

---

간결한 함수로 작성하자

---

해당 함수는 다음과 같은 기능을 가지고 있다.

페이지가 테스트 페이지인지 확인한 후 테스트 페이지라면 설정 페이지와 해제 페이지를 넣는다.
테스트 페이지든 아니든 페이지를 HTML로 렌더링하여 리턴한다.

 

하지만 함수가 길다. 😵

 

const renderPageWithSetupAndTeardowns = ({ pageData, isSuite }) => {
    const isTestPage = pageData.hasAttribute('Test');
    if (isTestPage) {
        const testPage = pageData.getWikiPage();
        const newPageContent = [];
        includeSetupPages(testPage, newPageContent, isSuite);
        newPageContent.push(pageData.getContent);
        includeTeardownPages(testPage, newPageContent, isSuite);
        pageData.setContent(newPageContent);
    }
    return pageData.getHtml();
}

 

다음과 같이 간결하게 작성할 수 있겠다. 😄

 

const renderPageWithSetupAndTeardowns = ({ pageData, isSuite }) => {
    if (isTestPage(pageData){
        includeSetupAndTeardownPages(PageData, isSuite);
    }
    return pageData.getHtml();
}

 

함수가 간결해진 것뿐만 아니라 한 가지 일만 하도록 되었다.

 

물론 함수가 다음과 같이 한 가지가 아닌 세 가지를 한다고 볼 수 있다.

 

✅  페이지가 테스트 페이지인지 판단한다.
✅  그렇다면 설정 페이지와 해제 페이지를 넣는다.
✅  페이지를 HTML로 렌더링한다.

 

그렇지만, 함수의 기능은 지정된 함수 이름 아래에서

추상화 수준이 하나인 단계만 수행한다면 그 함수는 한 가지 작업만 한다고 한다.

 

함수는 한 가지를 해야 한다. 그 한 가지를 잘해야 한다. 그 한 가지 만을 해야 한다.
함수 내에서 추상화 수준이 하나인 단계만 수행한다면 한 가지 작업만 한다.

 

 

다음 임대/매매 중계 수수료를 구하는 함수이다.

정보는 부동산 중개수수료를 참고하였다.

 

public Long calculate(Long price, String Type) {
        BrokerageRule rule;
        switch (type) {
         case purchase:
          if (price < 50_000_000) {
              rule = BrokerageRule.builder().brokeragePercent(0.6).limitAmount(250_000L).build();
          } else if(price < 200_000_000) {
              rule = BrokerageRule.builder().brokeragePercent(0.5).limitAmount(800_000L).build();
          } else if(price < 600_000_000) {
              rule = BrokerageRule.builder().brokeragePercent(0.4).limitAmount(null).build();
          } else if(price < 900_000_000) {
              rule = BrokerageRule.builder().brokeragePercent(0.5).limitAmount(null).build();
          } else {
              rule = BrokerageRule.builder().brokeragePercent(0.9).limitAmount(null).build();
          }
          return rule.calcMaxBrokerage(price);
        case rent:
            if (price < 50_000_000) {
                rule = BrokerageRule.builder().brokeragePercent(0.5).limitAmount(200_000L).build();
            } else if (price < 100_000_000) {
                   rule = BrokerageRule.builder().brokeragePercent(0.4).limitAmount(300_000L).build();
            } else if (price < 300_000_000) {
                rule = BrokerageRule.builder().brokeragePercent(0.3).limitAmount(null).build();
            } else if (price < 600_000_000) {
                rule = BrokerageRule.builder().brokeragePercent(0.4).limitAmount(null).build();
            } else {
                rule = BrokerageRule.builder().brokeragePercent(0.8).limitAmount(null).build();
            }
           return rule.calcMaxBrokerage(price);
        default:
            throw new IllegalArgumentException();

    }

위 함수는 몇 가지 문제가 있다. 😇

 

일단, 함수가 길다.
새 유형을 추가하면 더 길어진다.
생성(BrokerageRule)과 계산(calcMaxBrokerage)을 같이 하기에 한 가지 작업만 수행하지 않는다.
SRP(Single Responsibillity Principle)를 위반한다.
OCP(Open Closed Principle)를 위반한다.

 

어떻게 해결해야 할까? 🧐

 

타입을 분리한다.
🟢  매매 중계 수수료 정책(purchaseBrokeragePolicy)
🟢  임대 중계 수수료 정책(rentBrokeragePolicy)

 

매매/임대 중계 수수료 정책에서 생성과 계산을 분리한다.

🟢  임대 중계 수수료 정책(RentBrokeragePolicy)

 

 

RentBrokeragePolicy

public class RentBrokeragePolicy {
    public Long calculate(Long price) {
        /**
         * 생성과 계산을 분리합니다.
         */
        BrokerageRule brokerageRule = createBrokerageRule(price);
        return brokerageRule.calcMaxBrokerage(price);
    }

    private BrokerageRule createBrokerageRule(Long price) {
        BrokerageRule rule;
        if (price < 50_000_000) {
            rule = BrokerageRule.builder().brokeragePercent(0.5).limitAmount(200_000L).build();
        } else if(price < 100_000_000) {
            rule = BrokerageRule.builder().brokeragePercent(0.4).limitAmount(300_000L).build();
        } else if(price < 300_000_000) {
            rule = BrokerageRule.builder().brokeragePercent(0.3).limitAmount(null).build();
        } else if(price < 600_000_000) {
            rule = BrokerageRule.builder().brokeragePercent(0.4).limitAmount(null).build();
        } else {
            rule = BrokerageRule.builder().brokeragePercent(0.8).limitAmount(null).build();
        }
        return rule;
    }
}

 

매매/임대 모두 계산하는 방식은 동일하다. 👀

 

🟢  따라서 OCP원칙에 따라 확장에는 열려 있고 변경에는 닫혀 있게 변경한다.

 

 

BrokeragePolicy

public interface BrokeragePolicy {
    BrokerageRule createBrokerageRule(Long price);

     default Long calculate(Long price) {
        BrokerageRule brokerageRule = createBrokerageRule(price);
        return brokerageRule.calcMaxBrokerage(price);
    };
}


public class RentBrokeragePolicy implements BrokeragePolicy {
    @Override
    public BrokerageRule createBrokerageRule(Long price) {
        BrokerageRule rule;
        if (price < 50_000_000) {
            rule = BrokerageRule.builder().brokeragePercent(0.5).limitAmount(200_000L).build();
        } else if(price < 100_000_000) {
            rule = BrokerageRule.builder().brokeragePercent(0.4).limitAmount(300_000L).build();
        } else if(price < 300_000_000) {
            rule = BrokerageRule.builder().brokeragePercent(0.3).limitAmount(null).build();
        } else if(price < 600_000_000) {
            rule = BrokerageRule.builder().brokeragePercent(0.4).limitAmount(null).build();
        } else {
            rule = BrokerageRule.builder().brokeragePercent(0.8).limitAmount(null).build();
        }
        return rule;
    }
}

 

기존에 작성된 controller는 다음과 같다. 😉

 

BrokerageController

@RestController
public class BrokerageController {


    @GetMapping("/api/brokerage")
    public Long calcBrokerage(@RequestParam BrokerageType type, @RequestParam Long price) {
        switch (type) {
            case PURCHASE:
                PurchaseBrokeragePolicy purchaseBrokeragePolicy = new PurchaseBrokeragePolicy();
                return purchaseBrokeragePolicy.calculate(price);
            case RENT:
                RentBrokeragePolicy rentBrokeragePolicy = new RentBrokeragePolicy();
                return rentBrokeragePolicy.calculate(price);
            default:
                throw new IllegalArgumentException();
        }
    }
}

 

새로운 타입이 추가된다면

controller의 switch/case에 계속 추가해야 하며,

기존 코드의 동작 방식을 파악해야 하고
불필요하게 중복 코드가 생성된다.

 

새로운 타입이 추가되어 외부 변경에 유연하게 대응할 수 있도록 Factory를 추가한다. 👍

 

@RestController
public class BrokerageController {


    @GetMapping("/api/brokerage")
    public Long calcBrokerage(@RequestParam BrokerageType type, @RequestParam Long price) {
        BrokeragePolicy policy = BrokeragePolicyFactory.getBrokeragePolicyBy(type);
        return policy.calculate(price);
    }
}

public class BrokeragePolicyFactory {

    public static BrokeragePolicy getBrokeragePolicyBy(BrokerageType type) {
        switch (type) {
            case PURCHASE:
                return new PurchaseBrokeragePolicy();
            case RENT:
                return new RentBrokeragePolicy();
            default:
                throw new IllegalArgumentException();
        }
    }
}

 

새로운 타입이 추가되면 controller는 수정할 필요 없이 기존 함수(calculate)를 그대로 사용하면 된다.

 

 

함수 인수

---

클린코드에서는 함수 인수의 개수는 0개 이상 2개 이하가 적당하다고 한다.
( 🙅‍♂️ ) 3개인 경우는 가능하면 피하면 좋고

사용해야 한다면 특별한 경우에만 사용하라고 권고한다.

 

 

Side Effect를 일으키지 마라

---

 

아래의 함수는 입력한 패스워드가 맞는지 검증하는 함수이다.

 

checkPassword

const checkPassword = async ({ userName, password }) => {
  const user = await userService.findUserByName(userName);
  if (user) {
    const codedPhrase = await userService.getPhraseEncodedByPassword(user);
    const phrase = cryptographer.decrypt(codedPhrase, password);
    if ('Vaild Password' === phrase) {
      Session.initialize();
      return true;
    }
  }
  return false;
};

 

이름만 봐서는 세션을 초기화한다는 사실이 드러나지 않는다. 🧐
함수 이름만 보고 호출하는 사용자는

세션 초기화 기능을 수행하여 부수효과(side effect)를 발생하게 되는 위험에 처해진다.

만약 위와 같은 기능이 꼭 필요하다면

함수 이름을 checkPasswordAndInitializeSession처럼 기능에 맞게 변경해야 한다.

 

 

 

명령과 조회를 분리하라!

---

함수는 뭔가를 수행(객체 상태를 변경)하거나 뭔가에 답(객체 정보를 반환)하거나 둘 중 하나만 해야 한다.
둘 다 하면 혼란을 초래한다. 😵

 

if (approve('proposal', enrollment)) ...

 

이 함수는 요청(proposal)한 수강신청정보(enrollment)가 존재하면 승인을 하여 true를 반환하고 없으면 false를 반환한다.

코드를 보면 올바르게 동작하는 것 같지만,

if문에 넣고 보면 수강신청을 요청 상태로 승인된다면.. 으로 보인다.

요청한 수강신청 정보를 승인하여 성공한다면..으로 보이지 않는다.

또한, 명령(수강신청을 승인)과 조회(요청한 수강신청인지)가 합쳐서져 혼돈을 가져오고 있다.

 

해결책은 명령과 조회를 분리해 혼란을 애초에 뿌리 뽑는 방법이다. 👍

if(enrollmentExists('proposal')) {
    approve(enrollment);
}

 

 

 

반복하지 마라!
DRY 원칙
Don't Repeat Yourself

말을 다듬고 문장을 고치고 문단을 정리한다.
빠짐없이 테스트하는 코드를 짠다.
코드를 다듬고, 함수를 만들고, 이름을 바꾸고, 중복을 제거하고 전체 클래스를 쪼갠다.

 

참고

제로베이스 - 한달한권(클린코드)