택배 송장 [STEP 2] Wan

[hsw1920]

@zdodev
소대 안녕하세요! 완입니다 :)
Step2 PR 올립니다.
이전 Step1 PR 리뷰를 받기 전 Step2를 진행중에 있어서 Step1의 코멘트를 반영하여 Step2에 반영하였습니다. Step1이 아직 머지되지않았는데 함께 확인 부탁드립니다!
감사합니다 :)

미션 요구사항

1. SOLID OCP / 객체 미용 체조 2원칙 적용
   • ParcelInformation 타입은 discountedCost 내부에서 switch 분기처리를 하고 있음.
   • 할인 정책 추가 시 ParcelInformation 타입 내부 코드는 수정이 일어남 -> OCP 위배
   • DiscountStrategy 프로토콜 및 이를 채택하는 전략패턴을 통해 이를 해결
   • 요구 메서드는 func applyDiscount(deliveryCost: Int) -> Int
   • NoDiscount, VIPDiscount, CouponDiscount 타입 추가

내용

기존 discountedCost 값은 ParcelInformation 내부의 연산 프로퍼티로 작성되어있었습니다.
이를 Step1에서 CostInformation으로 분리하여 getDiscountedCost() 메서드를 통해 가져오도록 리팩터링이 이뤄졌었습니다.
그리고 Step1의 리팩터링 이후에도 여전히 getDiscountedCost() 메서드 내부에서는 policy가 추가됨에따라 코드의 변경이 이뤄지게됐습니다. -> OCP 위배.

가이드에 따라 이를 DiscountStrategy 프로토콜을 활용한 전략패턴을 통해 해결을 시도하였습니다.

// 열거형 Discount 유지하고 strategy 프로퍼티 추가

enum Discount: Int {
    case none = 0, vip, coupon
    private var strategy: DiscountStrategy {
        switch self {
        case .none:
            return NoDiscountStrategy()
        case .vip:
            return VIPDiscountStrategy()
        case .coupon:
            return CouponDiscountStrategy()
        }
    }
    
    func calculateDiscountedCost(deliveryCost: Double) -> Int {
        return strategy.applyDiscount(deliveryCost: deliveryCost)
    }
}

// DiscountStrategy protocol 타입을 만들고 이를 채택한 각 Strategy를 생성하여 추가할 수  있음.
protocol DiscountStrategy {
    func applyDiscount(deliveryCost: Double) -> Int
}

struct NoDiscountStrategy: DiscountStrategy {
    func applyDiscount(deliveryCost: Double) -> Int {
        return deliveryCost.convertToInt()
    }
}

struct VIPDiscountStrategy: DiscountStrategy {
    func applyDiscount(deliveryCost: Double) -> Int {
        let discountedCost = deliveryCost * DiscountRatio.vip
        return discountedCost.convertToInt()
    }
}

struct CouponDiscountStrategy: DiscountStrategy {
    func applyDiscount(deliveryCost: Double) -> Int {
        let discountedCost = deliveryCost * DiscountRatio.coupon
        return discountedCost.convertToInt()
    }
}

이를 통해 기존 Step1 까지와 달라진 점은 discountedCost를 연산하는 연산프로퍼티나 Step1에서의 getDiscounted() 메서드나 모두 내부의 하드 코딩을 통해 deliveryCost를 리턴하는 방식에서,
DiscountStrategy 프로토콜을 채택하는 새로운 인스턴스를 구현함으로써 코드의 변경이 아닌 새로운 인스턴스의 추가를 통해서 확장에 열려있고 수정에 닫혀있는 할인된 가격을 구하는 방법을 구현하였단 점입니다.

고민이 되었던 부분

전략패턴을 통해서 DiscountStrategy를 채택한 인스턴스를 추가하는것 역시 연산 프로퍼티인 strategy 내부의 switch문에서 수정이 일어나고있습니다.
이를 해결하려면 결국 strategy가 주입되어야 할 것 같은데 그렇게 되면 discount를 결정하는 ParcelOrderView에서 각 strategy를 주입해야할 것이고, 이는 비즈니스 로직의 추가가 view의 변경을 불러일으키는 것 같다 생각하여 지금 상태로 두었는데 적절한 생각인지(?) 궁금합니다. 따로 ViewModel같은 역할이 없어서 혼란스러운데 MVC 패턴이 뭔가 책임을 나누기가 애매하게 느껴지고 오히려 익숙하지 않은것 같아 걱정입니다.

Step1 에서의 코멘트 반영

• self 키워드, 중괄호 개행 통일
• get 접두사 관련된 연산 프로퍼티와 메서드 수정

조언을 얻고 싶은 부분

struct ParcelInformation {
    private let receiverInformation: ReceiverInformation
    private let costInformation: CostInformation
    
    init(receiverInformation: ReceiverInformation,
         costInformation: CostInformation) {
        self.receiverInformation = receiverInformation
        self.costInformation = costInformation
    }
    
    var receiverName: String {
        return receiverInformation.name
    }
    
    var receiverAddress: String {
        return receiverInformation.address
    }
    
    var receiverMobile: String {
        return receiverInformation.mobileNumber
    }
    
    func calculateTotalCost() -> Int {
        return costInformation.calculateTotalCost()
    }
}

struct ReceiverInformation {
    private let _address: String
    private let _name: Name
    private let _mobileNumber: MobileNumber
    
    var address: String {
        return _address
    }
    
    var name: String {
        return _name.value
    }
    
    var mobileNumber: String {
        return _mobileNumber.value
    }
    
    init(address: String,
         name: Name,
         mobileNumber: MobileNumber) {
        self._address = address
        self._name = name
        self._mobileNumber = mobileNumber
    }
}

get 접두사 관련하여 연산 프로퍼티를 고민하였고, 네이밍과 관련한 고민을 하던와중 아티클을 발견하였습니다.
https://soojin.ro/blog/english-for-developers-swift - 노수진님 블로그
스위프트의 getter에서는 get없이 바로 타입 이름으로 시작하도록 권장하셔서 메서드는 아니지만 연산 프로퍼티에 이를 적용해보았습니다.
그리고 은닉화된 프로퍼티에는 접두사로 _를 붙이고(ex. _name) 보여질 값은 그대로 작성하였습니다.(ex. name)

1. '_'를 접두어로 종종 찾아볼 수 있었는데 이게 권장될만한 선언방식인지 궁금합니다. (찾아봐도 잘 안나오더라구요..)
2. 연산프로퍼티와 메서드의 구분에 대한 부분.
   기준을 세운점은 해당 인스턴스에서 어떠한 협력없이 바로 얻을 수 있는 프로퍼티는 연산 프로퍼티로 위와같이 작성하였고, 해당 인스턴스에서 다른 객체(인스턴스)의 협력이 필요한 부분은 메서드로 작성하였고 나름 객체지향프로그래밍이란 이런게 아닐까하며 코드를 작성하였는데 이에 대한 코멘트를 받아보고싶습니다!

[zdodev]

안녕하세요. @hsw1920

Step2도 빠르게 진행해보셨네요!
요구사항에 대해 깔끔하게 구현해보신 것 같습니다.👍

질문의 내용과 함께 코멘트를 조금 남겨보았습니다.
확인해보시고 의견을 나눠보면 좋을 것 같습니다.😁

[zdodev]

상수를 따로 관리해보셨네요.👍

[zdodev]

생성 메서드를 별도로 구현해보셨네요.👍

[zdodev]

_를 사용하여 프로퍼티 이름을 정의하는 것은종종 사용해볼 수 있을 것 같습니다.
회사나 팀에서 _를 사용한 프로퍼티의 컨벤션을 정해두고 사용하면 더욱 괜찮을 것 같습니다!

[zdodev]

말씀해주신 것처럼 저도 비슷한 기준을 가지고 있습니다!😁
간단한 연산이나 단순 값의 접근이 이뤄지는 경우에는 연산 프로퍼티를 많이 활용해보고 있습니다!
인스턴스의 협력이 필요한 부분에 대해서 메서드를 사용한다는 점도 좋은 기준이라 생각합니다!👍

[zdodev]

strategy를 외부에 노출시키지 않도록 해보셨네요.👍
말씀해주신 것 처럼 이후에 switch의 수정이 발생할 수 있어서 strategy가 주입하도록 구현하는 것도 좋을 것 같습니다.
현재 단계에서는 뷰에서 Discount를 전달하고 있어서 지금과 같은 구조도 좋다는 생각입니다!
뷰 구조를 수정해서 strategy를 더 개선해볼 수 있겠다는 생각을 해보신 것만으로 좋은 고민을 해보신 것 같습니다.👍

[zdodev]

DiscountStrategy 프로토콜을 잘 구현해보셨네요.👍
DiscountStrategy 프로토콜에서 extension을 사용하여 기본 메서드를 구현해보는 것은 어떨까요? 생각이 궁금합니다!

[zdodev]

discountedCost를 대신할 수 있겠네요.👍