택배송장앱 [STEP 1] Caron #40
Conversation
|
안녕하세요. @Qussk ! 현재 PR 대상 브랜치가 main으로 설정되어 있습니다! 해당 브랜치를 본인의 브랜치로 변경해주실 수 있을까요? |
|
넵 다시 제출 했습니다.!!! OCP까지 일단 해봤는데 리뷰 부탁드립니다!!! |
There was a problem hiding this comment.
안녕하세요. Caron!
리뷰어 소대입니다!
쉽지 않은 내용이었는데, 택배송장앱 Step1, 2 진행하시느라 고생하셨습니다!
먼저 본문에 적어주신 질문에 대한 제 생각은 다음과 같습니다.
**그리고 프로토콜을 정의하는 것 - 인터페이스분리법칙
클래스를 따로 분리 - 단일책임원칙
이후 의존성 주입이 되어있는곳을 역전시키는것?- 의존관계역전원칙
이라고 이해한게 맞은건지 궁금하고,
프로토콜을 정의하거나 클래스를 분리하는 것이 인터페이스 분리나 단일책임원칙 이라고 보기는 어려울 것 같습니다!
필요한 상황에 따라 혹은 그에 맞게 인터페이스를 분리하거나, 단일책임원칙을 준수하도록 할 때, 어떠한 기법이나 프로그래밍 방식이 필요한 지 확인해보면 좋을 것 같습니다!
개인적으로 DIP나 OCP가 무엇인지 보는 것보다 DIP와 OCP를 안지켰을 때 생기는 단점을 알아봤을 때 더 이해하기 쉬었던 것 같습니다!
관련한 내용에 대해서 DM을 주셔도 좋을 것 같습니다~!
다른 질문은 코멘트에 궁금증과 함께 의견을 남겨보았습니다!
같이 의견 나눠보면 좋을 것 같습니다!😁
| @@ -7,10 +7,13 @@ | |||
| import UIKit | |||
|
|
|||
| class ParcelOrderViewController: UIViewController, ParcelOrderViewDelegate { | |||
| private let parcelProcessor: ParcelOrderProcessor = ParcelOrderProcessor() | |||
|
|
|||
| private var parcelProcessor: ParcelProcessor = ParcelProcessor() | |||
There was a problem hiding this comment.
parcelProcessor 프로퍼티를 ParcelProcessor 타입으로 선언하는 대신,
ParcelInformationProcessProtocol 타입으로 선언하는 것은 어떨까요?
DIP 내용을 적용할 수 있지 않을까 싶습니다!😁
| class ParcelProcessor: ParcelInformationProcessProtocol & ParcelInformationPersistenceProtocol { | ||
| func process(parcelInformation: ParcelInformation, onComplete: (ParcelInformation) -> Void) { | ||
| // 데이터베이스에 주문 저장 | ||
| save(parcelInformation: parcelInformation) | ||
|
|
||
| onComplete(parcelInformation) | ||
| } | ||
|
|
||
| func save(parcelInformation: ParcelInformation) { | ||
| print("발송 정보를 데이터베이스에 저장했습니다.") | ||
| } | ||
| } |
There was a problem hiding this comment.
ParcelProcessor 타입을 새로 정의하신 이유가 있을까요?
이미 정의한 ParcelOrderProcessor 타입을 사용하는 것은 어떨까요?🙂
| protocol ParcelInformationProcessProtocol { | ||
| func process(parcelInformation: ParcelInformation, onComplete: (ParcelInformation) -> Void) | ||
| } | ||
|
|
||
| class ParcelInformation { |
There was a problem hiding this comment.
ParcelInformation 타입은 class와 struct 중 어떤 것으로 선언하는 것이 좋을 지 궁금합니다!
그리고 Step1의 요구사항 중 객체미용체조 7원칙을 적용해서 ParcelInformation 타입의 프로퍼티를 묶어서 분리해보는 것은 어떨까요?
관련성이 있는 요소를 찾아서 새로운 타입으로 조합해보면 좋을 것 같습니다!🙂
| } | ||
|
|
||
| class ParcelOrderProcessor { | ||
|
|
||
| class ParcelOrderProcessor : ParcelInformationPersistenceProtocol { |
There was a problem hiding this comment.
ParcelOrderProcessor 클래스가 ParcelInformationPersistenceProtocol 프로토콜을 직접 채택하신 이유가 있을까요?
프로토콜을 직접 채택하는 대신, 현재 소유한 프로퍼티에게 저장 역할을 전달하는 것은 어떨까요? 프로퍼티와 save 메서드를 통해서 저장 역할을 다른 타입에게 전달할 수 있을 것 같습니다!
|
|
||
| class ParcelOrderProcessor : ParcelInformationPersistenceProtocol { | ||
|
|
||
| private var databaseParcelInformationPersistence: DatabaseParcelInformationPersistence = DatabaseParcelInformationPersistence() |
There was a problem hiding this comment.
databaseParcelInformationPersistence 프로퍼티가 구체 타입인 DatabaseParcelInformationPersistence에 의존하고 있네요!
이렇게 DatabaseParcelInformationPersistence 타입에 직접 의존하면, 변화에 대응하기 어렵고 수정이 빈번해질 수 있습니다!
구체 타입에 의존하기 보다는 프로토콜(인터페이스)에 의존하도록 하고, init 메서드를 통해 구체 타입에 대한 의존성을 주입 받도록 구현(DIP)해보는 것은 어떨까요?
구체 타입에 의존하기 보다는 인터페이스에 의존하는 것에 대한 내용을 확인해보면 좋을 것 같습니다!😁
| class DatabaseParcelInformationPersistence { | ||
| func save(parcelInformation: ParcelInformation) { | ||
| // 데이터베이스에 주문 정보 저장 | ||
| print("발송 정보를 데이터베이스에 저장했습니다.") | ||
| } | ||
| } |
There was a problem hiding this comment.
저장 역할을 할 수 있는 구체 타입을 잘 정의해주셨네요!👍 프로토콜(인터페이스)를 따르도록 해보면 좋을 것 같습니다!
| let parcelProcessor: ParcelProcessor = ParcelProcessor() | ||
|
|
||
| let viewController: ParcelOrderViewController = ParcelOrderViewController() | ||
| let viewController: ParcelOrderViewController = ParcelOrderViewController(parcelProcessor: parcelProcessor) |
| enum Discount: Int { | ||
| case none = 0, vip, coupon | ||
|
|
||
| var strategy: DiscountStrategy { | ||
| switch self { | ||
| case .none: | ||
| return NoDiscount() | ||
| case .vip: | ||
| return VIPDiscount() | ||
| case .coupon: | ||
| return CouponDiscount() | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
discountedCost 프로퍼티의 내용이 이전과 동일합니다!
discountedCost 프로퍼티의 내용을 잘 정의해주신 discount 프로퍼티를 활용해보는 것은 어떨까요?
deliveryCost 값을 직접 사용하기 보다는 discount 프로퍼티에 전달하여 해결할 수 있을 것 같습니다!🙂
| protocol DiscountStrategy { | ||
| func applyDiscount(deliveryCost: Int) -> Int | ||
| } |
|
|
||
| class NoDiscount: DiscountStrategy { | ||
| func applyDiscount(deliveryCost: Int) -> Int { | ||
| return 0 |
|
안녕하세요! @Qussk ! |
|
안녕하세요 다시 요청 드려봅니다....!! 검토 부탁드립니다. 그리고 객체 미용 체조도 적용해보려고 노력중인데요..! 3원칙 - 원시값과 문자열의 포장에서 case문을 최대한 안쓰는 방향으로 이야기하고 있는데요. enum Discount: Int { } |
| class YouthDiscount: DiscountStrategy { | ||
| func applyDiscount(deliveryCost: Int) -> Int { | ||
| return deliveryCost / DiscountList.youth | ||
| } |
| } | ||
|
|
||
| class ParcelOrderProcessor { | ||
|
|
||
| let databaseParcelInformationPersistence: DatabaseParcelInformationPersistence? |
There was a problem hiding this comment.
databaseParcelInformationPersistence 프로퍼티를 DatabaseParcelInformationPersistence 구체 타입으로 정의하신 이유가 있을까요? DIP를 수행하기 위해서 프로토콜의 타입으로 선언되어야 하지 않을까 싶습니다!
There was a problem hiding this comment.
DIP와 연결되는 부분이라 정의하신 ParcelInformationPersistence 프로토콜을 활용해보면 좋을 것 같습니다!
| enum DiscountList { | ||
| static let vip = 5 * 4 | ||
| static let coupon = 2 | ||
| static let youth = 3 |
| var strategy: DiscountStrategy { | ||
| switch self { | ||
| case .none: | ||
| return NoDiscount() | ||
| case .vip: | ||
| return VIPDiscount() | ||
| case .coupon: | ||
| return CouponDiscount() | ||
| case .youth: | ||
| return YouthDiscount() | ||
| } | ||
| } |
There was a problem hiding this comment.
지금과 같은 switch-case문도 꼭 제거하지 않고 괜찮은 코드라고 생각입니다
그런데 Discount 타입의 수정이 발생할 수 있어 switch-case문을 제거하는 것도 좋은 생각인것 같습니다!👍
이후에 개선해보는 것도 좋을 것 같습니다!
안녕하세요. 제가 개인적인 해외일정과 시차문제 때문에 수업에 완벽하게 참여하고 있지 못하지만 저장된 영상을 보면서 이해해나갈것이며 일정에 따라 시간나는 대로 최대한 연습을 해보는 중입니다.
[STEP 1] , 까지 밖에 해보지 못했는데 아래 내용이 잘 이해되지가 않아서 ㅠㅠ 제대로 못한것 같습니다.
“DatabaseParcelInformationPersistence를 직접 소유하는 대신에 ParcelInformationPersistence을 소유하도록 변경” 하라는 부분이 DatabaseParcelInformationPersistence의존성 주입을 해제하고 ParcelInformationPersistence라는 클래스를 새로 만들어서 의존성 주입후 init으로 의존성 역전을 하라는 말일까요 ?…
[SOLID DIP 적용] 부분입니다!!
**그리고 프로토콜을 정의하는 것 - 인터페이스분리법칙
클래스를 따로 분리 - 단일책임원칙
이후 의존성 주입이 되어있는곳을 역전시키는것?- 의존관계역전원칙
이라고 이해한게 맞은건지 궁금하고,
의존성 역전시키는게 상위타입이 하위 타입에 의존하지 않도록 바꾸는 거같은데 객체지향 때문인건가요 ????? (정확히 이해가 안돼서요.)
답변 부탁드립니다. 감사합니다. ㅠ
영상올라오면 꼭 보겠습니다!!