쿠폰 로직 리팩토링 하기 !

글을 읽기 전 배경

• 모빌리티 스타트업에서 백엔드 엔지니어로 근무하고 있어요 !
• python, django를 주로 사용합니다 !

복잡한 결제와 쿠폰

결제는 언제나 복잡합니다. 주행 요금이 최초 산정되면 유저가 소유한 정기권, 쿠폰, B2B 제휴사, 거치대 할인, 각 파트너사 등 모든 경우를 적용하여 최종 요금이 부과됩니다. 그 중 쿠폰은 정기권과 함께 유저가 가장 많이 사용하는 할인 수단 중 하나입니다. 비즈니스 요구사항은 점차 다양해지면서 다양한 유형의 쿠폰이 생겨났고, 현재는 크게 4가지 타입의 쿠폰이 존재합니다.

N분 무료 쿠폰
잠금해제 무료 쿠폰
요율 할인 쿠폰
금액 할인 쿠폰

하나의 쿠폰이 사용되기 위해서는 아래와 같은 사항들이 고려되어야 합니다

(적용 시점) 현재 적용 가능한 쿠폰인가 ?
(적용 시점) 적용 시점에 만료가 되었다면 만료 처리를 진행 할 수 있는가 ?
(적용 시점) 각 쿠폰 타입에 따라 요금 할인이 올바르게 적용 되었는가 ?
(사용 시점) 결제 이후 쿠폰에 대한 사용 처리 했는가 ?

쿠폰의 적용 시점만 보더라도 “쿠폰의 타입의 구분”하고 “적용 가능 여부를 확인”하며, “쿠폰의 혜택에 맞는 요금 계산” 을 진행해야합니다. 만약 위의 과정을 하나의 플로우에서 처리한다면 어떨까요 ? 간단하게 생각하면 수많은 if / elif / else 체이닝을 통해 해결 할 수 있을 것 입니다.

하지만 이럴 경우, 테스트 코드 관점에서는 하나의 로직을 통으로 테스트 해야하기 때문에 단위 단위의 테스트 코드를 작성하기에 취약하고, 누군가 새로운 로직을 추가할 때 한 눈에 로직을 파악하기 어렵습니다. 사소한 변경에도 예상치 못한 사이드 이펙트가 발생할 수가 있고, 무엇보다 앞으로 새로운 유형의 쿠폰이 등장한다면 ? 유지보수의 난이도가 급격히 높아질 것입니다. 이시점에서라도 앞으로의 유지보수의 효율을 높이기 위해 쿠폰 적용 로직을 인터페이스 기반으로 리팩토링을 진행하기로 했습니다.

1차 리팩토링

우선 쿠폰의 대표적인 적용 과정을 추상화하여 이를 추상 클래스로 정의했습니다.

class CouponHandlerBase(ABC):
    """interface class"""
	
	@abstractmethod
    def can_consume(self) -> bool:
        """쿠폰 적용 가능 여부"""
        raise NotImplementedError
    
    @abstractmethod
    def consume(self, charge: int) -> int:
        """쿠폰 적용. 쿠폰 할인 금액 반환"""
        raise NotImplementedError

    @abstractmethod
    def finalize(self) -> None:
        """쿠폰 사용 후 처리"""
        raise NotImplementedError

이후 주행 쿠폰외에 다른 쿠폰(정기권을 할인 해주는 쿠폰이라던지..)이 생성될 것을 고려하여 공통으로 사용할 메서드들을 담는 CouponHandlerMixin 클래스를 정의했습니다. 그리고 이것들을 상속받는 주행 전용 쿠폰 클래스인 RidingCouponHandler를 정의했습니다.

class CouponHandlerMixin:
    """mixin class"""

    def _used(self) -> None:
        """쿠폰 사용 처리"""
        ...

    def _expired(self) -> None:
        """쿠폰 만료 처리"""
        ...

class RidingCouponHandler(CouponHandlerMixin, CouponHandlerBase):
    def __init__(self):
        ...

    def consume(self, charge: int) -> int:
        # 세부 구현체에서 overriding
        pass
		
    def can_consume(self) -> bool:
        # 쿠폰 적용 가능 여부 로직의 경우 공통된 부분이기 때문에 상위 구현체에서 구현
        pass

    def finalize(self) -> None:
        # 쿠폰 사용 후 처리 로직의 경우 공통된 부분이기 때문에 상위 구현체에서 구현
        pass

그리고 RidingCouponHandler를 상속 받는 최종적으로 세부 구현체를 구현했습니다.

class UnlockFreeCoupon(RidingCouponHandler):
    """잠금 해제 무료 쿠폰"""

    def consume(self, charge: int) -> int:
		# 쿠폰 적용 로직 구현
        ...

이 모든 것을 다이어그램으로 그리면 아래와 같습니다.

어떤가요 ? 하나의 로직에서 쿠폰의 유형을 구분하고, 적용 가능한지 확인 한 후, 실제 적용을 했을 때보다 정리가 되었고, 각 쿠폰 구현체 별 로직을 좀 더 파악 할 수 있지 않나요 ? 하지만 1차 리팩토링 이후 몇 가지 문제점을 발견했습니다.

1차 리팩토링의 문제점 발견

1. 추상 메서드와 일반 메서드가 함께 존재하는 안티패턴
2. 불필요한 상속으로 인한 복잡성 증가 및 유연성 감소

문제점 1) 추상 메서드와 일반 메서드가 함께 존재하는 안티패턴

• RidingCouponHandler를 보면 추상 메서드와 일반 메서드가 공존합니다. 하나의 클래스 내에서 추상 메서드와 일반 메서드가 함께 존재한다면, 이 클래스를 상속받는 하위 클래스에서는 반드시 추상 메서드를 구현해야 하지만 일반 메서드는 선택적으로 오버라이드할 수 있습니다. 이는 상속 관계에서 일관성이 떨어질 수 있고, 예상치 못한 동작을 유발할 수 있기 때문에 안티 패턴입니다.
• 따라서 추상 클래스에서는 추상 메서드만을 정의하고, 구현된 메서드는 일반 클래스에서 처리하는 것이 코드의 일관성을 유지하며, 각 클래스 계층 구조가 명확해지게 됩니다. 여기서 공통된 로직이 필요한 경우, 이를 다루기 위한 별도의 클래스나 인터페이스를 고려하는 것이 좋습니다.

문제점 2) 불필요한 상속으로 인한 복잡성 증가 및 유연성 감소

• 실제 세부 구현체인 UnlockFreeCoupon을 이해하기 위해서는 기본 인터페이스부터, 공통 로직이 포함된 Mixin 클래스, 상위 구현체인 RidingCouponHandler 클래스를 이해해야합니다. 상속 단계에 대한 대한 명확한 룰은 없지만 상속 계층이 간단할 수록 이해하기 쉽고, 유지보수에 용이합니다.
• 많은 상속 계층이 있다면, 자칫 상위 클래스의 변경이 하위 클래스에 예상치 못한 영향을 끼칠 수 있습니다. 또한 많은 상속은 결국 클래스간 의존성을 높임과 동시에 다이아몬드 문제를 늘 염두에 두고 개발을 진행해야합니다.

그렇게 1차 리팩토링 후 개섬점을 발견했고, 액션 아이템을 도출했습니다.

추상 클래스와 구현체의 명확한 구분
불필요한 상속을 줄여 유지보수에 용이한 단순한 구조 만들기

2차 리팩토링

우선 추상 매서드와 일반 메서드와의 불편한 공존을 제거하고 공통된 로직은 Mixin 클래스에 구현하면서 RidingCouponHandler의 불필요한 상속을 정리했습니다.

class CouponHandlerBase(ABC):
    """interface class"""
		
    @abstractmethod
    def consume(self, charge: int) -> int:
        """쿠폰 적용. 쿠폰 할인 금액 반환"""
        raise NotImplementedError

class CouponHandlerMixin:
    """mixin class"""

    def _used(self) -> None:
        """쿠폰 사용 처리"""
				...

    def _expired(self) -> None:
        """쿠폰 만료 처리"""
				...

    def can_consume(self) -> bool:
				"""쿠폰 적용 여부"""
        ...

    def finalize(self) -> None:
				"""쿠폰 사용 후 처리"""
        ...

그리고 세부 구현체에서 Mixin 클래스와 추상 클래스와 상속받으면서, 중복을 제거하는 동시에 추상 클래스의 행동을 강제할 수 있었습니다.

class UnlockFreeCoupon(CouponHandlerMixin, CouponHandlerBase):
    """잠금 해제 무료 쿠폰"""

    def __init__(self, coupon: Coupon, riding: Riding):
        ...

    def consume(self, charge: int):
	    # 쿠폰 적용 로직 구현
        ...

최종 다이어그램은 아래와 같습니다. 어떤가요 ? 1차 리팩토링 이후와 비교해보았을 때 확실히 단순해졌고, 이해하기 쉬워졌습니다.

테스트코드

이전에는 쿠폰 적용 로직을 테스트 하기 위해서는 모든 적용 로직을 한 번에 테스트를 했어야 하나, 이제는 각 단계별로 구분하여 단위 별 테스트를 할 수 있게 되었습니다.

@pytest.fixture
def coupon(user) -> Coupon:
    expire_at = (timezone.now() + relativedelta(days=10)).strftime('%Y-%m-%d')
    return baker.make(Coupon, user=user, is_active=True, expire_at=expire_at)


@pytest.fixture
def time_free_coupon(coupon) -> Coupon:
    coupon.type = Coupon.TypeChoices.TIME_ONCE
    coupon.value = 10
    coupon.save()
    return coupon

@pytest.fixture
def expired_coupon(coupon) -> Coupon:
    coupon.expire_at = (timezone.now() - relativedelta(days=1)).strftime('%Y-%m-%d')
    coupon.save()
    return coupon
​
@pytest.mark.django_db
class TestCoupon:
    def test_10분_무료_쿠폰을_적용한다(self, time_free_coupon, endpoint, auth_client):
        response: Response = auth_client.post(endpoint, data={'coupon_id': time_free_coupon.id})

        assert response.status_code == status.HTTP_200_OK
        assert 600 == response.json()['amount']


    def test_적용_시점에_만료된_쿠폰은_사용할_수_없다(self, expired_coupon, endpoint, auth_client):
        response: Response = auth_client.post(endpoint, data={'coupon_id': expired_coupon.id})

        assert response.status_code == status.HTTP_412_PRECONDITION_FAILED

마무리

절차적으로 적용되던 쿠폰 로직를 인터페이스를 적용한 로직으로 리팩토링하면서 얻을 수 있는 효과는 아래와 같습니다.

이해하기 쉽습니다.

• 쿠폰 적용이 추상 클래스에 정의되어(InterfaceClass), 기능 클래스(MixinClass)와 실제 구현체(class)가 구분되어 각 클래스간 구조와 역할을 한 눈에 파악할 수 있습니다.

더 이상 if 분기를 따라가며 로직을 파악하지 않아도 됩니다.

• 변화에 빠르게 대응 할 수 있습니다
• 새로운 쿠폰 유형과 새로운 적용 로직이 등장하더라도, 더 이상 어디에 if / elif / else 체이닝을 추가할지 고려하지 않고 쉽게 추가 할 수 있습니다.

안전합니다.

각 구현체의 크기가 작고, 명확하게 구분되어 있어 테스트 코드를 작성 할 때도, 특정 구현체의 로직을 변경하더라도 안전하게 테스트하고 적용 할 수 있습니다.

물론 이번 리팩토링이 완벽하지는 않습니다. 리팩토링을 하면서도 이게 맞나 싶은 고민점들이 많았고, 공통의 기능을 Mixin으로 빼는 것이, 최종 구현체를 이렇게 구현하는 것이 맞는지 계속 의문이 들고 있습니다. 하지만 언제나 개선을 안한 것 보다는 한 것이 낫고, 저의 고민이 들어간 코드가 반영된 것만으로도 행복합니다. 무엇보다 이번 경험을 통해 이전에는 해보지 못한 고민을 해본 것이 가장 큰 의미라고 생각합니다.