[리팩토링 2판 파이썬 코드로 변경해보기] 3탄 함수 쪼개기, 변수명 변경

리팩토링 2판의 제일 처음 나오는 예제를 파이썬 코드로 변경 하면서, 책의 내용에 따라 리팩토링 해보고 저자의 의견과 내 의견을 정리

import json
import math


def statement(invoice, plays):
    total_amount = 0
    volume_credits = 0
    result = f"청구 내역 (고객명: {invoice['customer']})\n"
    dollar_format = '${:,.2f}'

    for perf in invoice["performances"]:
        play = plays[perf["playID"]]
        this_amount = 0

        if play["type"] == "tragedy":
            this_amount = 40000
            if perf["audience"] > 30:
                this_amount += 1000 * (perf["audience"] - 30)
        elif play["type"] == "comedy":
            this_amount = 30000
            if perf["audience"] > 20:
                this_amount += 10000 + 500 * (perf["audience"] - 20)
            this_amount += 300 * perf["audience"]
        else:
            raise Exception("알 수 없는 장르")

        volume_credits += max(perf["audience"] - 30, 0)

        if play["type"] == "comedy":
            volume_credits += math.floor(perf["audience"] / 5)

        result += f' {play["name"]}: {dollar_format.format(this_amount / 100)} ({perf["audience"]}석)\n'
        total_amount += this_amount

    result += f"총액: {dollar_format.format(total_amount / 100)}\n"
    result += f"적립 포인트': {volume_credits}점\n"
    return result


if __name__ == '__main__':
    with open('invoices.json') as json_file:
        invoice = json.load(json_file)[0]

    with open('plays.json') as json_file:
        plays = json.load(json_file)

    print(statement(invoice, plays))

 

if문

위 코드를 처음 봤을 때, 가장 눈에 띄고 처리해야할 것 처럼 보였던 것은 if 문이였다. 책을 기준으로 볼 때, switch 문은 아래와 같이 분리할 수 있다.

import json
import math


def statement(invoice, plays):

    # if 문은 amount_for로 분리 되었다
    def amount_for(perf, play):
        if play["type"] == "tragedy":
            this_amount = 40000
            if perf["audience"] > 30:
                this_amount += 1000 * (perf["audience"] - 30)
        elif play["type"] == "comedy":
            this_amount = 30000
            if perf["audience"] > 20:
                this_amount += 10000 + 500 * (perf["audience"] - 20)
            this_amount += 300 * perf["audience"]
        else:
            raise Exception("알 수 없는 장르")
        return this_amount

    total_amount = 0
    volume_credits = 0
    result = f"청구 내역 (고객명: {invoice['customer']})\n"
    dollar_format = '${:,.2f}'

    for perf in invoice["performances"]:
        play = plays[perf["playID"]]
        this_amount = amount_for(perf, play) # if문의 결과 값이 할당된다
        volume_credits += max(perf["audience"] - 30, 0)

        if play["type"] == "comedy":
            volume_credits += math.floor(perf["audience"] / 5)

        result += f' {play["name"]}: {dollar_format.format(this_amount / 100)} ({perf["audience"]}석)\n'
        total_amount += this_amount

    result += f"총액: {dollar_format.format(total_amount / 100)}\n"
    result += f"적립 포인트': {volume_credits}점\n"
    return result


if __name__ == '__main__':
    with open('invoices.json') as json_file:
        invoice = json.load(json_file)[0]

    with open('plays.json') as json_file:
        plays = json.load(json_file)

    print(statement(invoice, plays))

 

함수를 내부 함수로 추출하였다. 이유는 추출한 함수에 매개변수로 전달할 필요가 없어서 편하기 때문이라고 한다. 물론 그 장점도 있지만 내 생각에는 flat 하게 유지하기 위해서 함수 밖으로 빼는게 더 낫지 않을까? 라는 생각이 들었다. 함수 추출로 인한 가독성 향상의 측면이 내부함수를 쓸 때 보다 좀 더 좋아 질 수 있기 때문이다.

단순히 위 코드만 보았을 때는 그렇다고 생각한다. 하지만, 저자 생각에 위 코드에서 좀 더 확장성 있게 설계를 가져가기 위한 장치로 보고, 일단 넘어가기로 했다.

여기 까지 수정 후 테스트를 돌리면, 테스트가 통과하는 것을 볼 수 있다.

Ran 1 test in 0.002s OK

 

변수명 변경 

책에서는 내부함수인 amount_for에서 perf와 this_amount를 좀 더 명확한 의미로 바꾼다. 바꾼 코드는 아래와 같다.

def statement(invoice, plays):

    def amount_for(a_performance, play):
        result = 0
        if play["type"] == "tragedy":
            result = 40000
            if a_performance["audience"] > 30:
                result += 1000 * (a_performance["audience"] - 30)
        elif play["type"] == "comedy":
            result = 30000
            if a_performance["audience"] > 20:
                result += 10000 + 500 * (a_performance["audience"] - 20)
            result += 300 * a_performance["audience"]
        else:
            raise Exception("알 수 없는 장르")
        return result
    
    ...이하 생략...

저자는 변수명을 명확하게 하기 위해서 위와 같이 정했다고 말한다. 그러나 내 생각에는 그다지 변수명이 명확하다고 느끼지 못하였다.

첫째로, result 라는 변수명은 너무 제너럴 하다. 여기저기에 다 사용될 수 있기 때문에 result 보다는 좀 더 구체적인 이름이 더 좋을 것 같다. 둘째로, a_performance는ㄹ 자바스크립트의 변수명 특징으로 보인다. 파이썬에서는 위와 같이 쓰는 사례를 본적이 없다. 저자는 동적 타입언어로 개발할 때, 변수에서 의미를 추정할 수 있도록 짓기 위해서 위와 같이 정했다고 한다 하지만, 파이썬에서는 타입힌팅을 제공하기 때문에, 타입힌팅을 추가해주는게 더 적절해 보인다. (일단은.. 저자의 마음대로)

여기 까지 수정 후 테스트를 돌리면, 테스트가 통과하는 것을 볼 수 있다.

Ran 1 test in 0.002s OK 

현재 까지 수정된 코드의 완성본은 아래와 같다.

import json
import math


def statement(invoice, plays):

    def amount_for(a_performance, play):
        result = 0
        if play["type"] == "tragedy":
            result = 40000
            if a_performance["audience"] > 30:
                result += 1000 * (a_performance["audience"] - 30)
        elif play["type"] == "comedy":
            result = 30000
            if a_performance["audience"] > 20:
                result += 10000 + 500 * (a_performance["audience"] - 20)
            result += 300 * a_performance["audience"]
        else:
            raise Exception("알 수 없는 장르")
        return result

    total_amount = 0
    volume_credits = 0
    result = f"청구 내역 (고객명: {invoice['customer']})\n"
    dollar_format = '${:,.2f}'

    for perf in invoice["performances"]:
        play = plays[perf["playID"]]
        this_amount = amount_for(perf, play)
        volume_credits += max(perf["audience"] - 30, 0)

        if play["type"] == "comedy":
            volume_credits += math.floor(perf["audience"] / 5)

        result += f' {play["name"]}: {dollar_format.format(this_amount / 100)} ({perf["audience"]}석)\n'
        total_amount += this_amount

    result += f"총액: {dollar_format.format(total_amount / 100)}\n"
    result += f"적립 포인트': {volume_credits}점\n"
    return result


if __name__ == '__main__':
    with open('invoices.json') as json_file:
        invoice = json.load(json_file)[0]

    with open('plays.json') as json_file:
        plays = json.load(json_file)

    print(statement(invoice, plays))