Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#178] 테스트코드 작성 전 코드 리팩토링 #197

Merged
merged 49 commits into from
Oct 20, 2023

Conversation

Parkjju
Copy link
Collaborator

@Parkjju Parkjju commented Oct 3, 2023

상세 내용

테스트코드 작성하는 시스템을 선제적으로 적용해보려고 이것저것 문서들을 참고해봤는데, 현재 코드로는 쉽지가 않은 것 같다고 판단이 서서 우선 완료함쪽 코드 리팩토링을 전체적으로 진행했습니다.

테스트코드 작성이 어렵다고 판단한 이유로 몇가지 이유가 있는데, 정리하면 다음과 같았습니다

  1. 뷰 컨트롤러 인스턴스 생성시 뷰모델을 주입하는 방식으로 구현되어 있지 않음. (let VC = NewViewController(viewModel: viewModel) )이러한 형태로 생성자 함수에서 뷰모델 속성을 할당하도록 구현해야 테스트코드 작성에 용이하다고 하네요. 아직 테스트코드 작성방법은 자세히 모르지만, 일단 목업 뷰모델을 생성하여 뷰컨에 전달하는 방식이 기반이 된다는 것만 알고 있습니다
  2. 뷰컨과 뷰모델의 관계가 N : 1로 맺어져도 되지만 뷰마다 새로운 뷰모델에 대한 관계를 맺어주는게 특정 뷰에 대한 테스트를 더 잘 작성할 수 있다고 생각했습니다 👍
  3. Input & Output 형태가 deprecated 되었다는 의견이 있었는데.. 레퍼런스 프로젝트 코드를 전체적으로 훑어보니 Input & Output 기반의 뷰모델 정의가 훨씬 직관적이고 뷰모델이라는 패턴을 잘 사용하는 것이라고 생각했습니다. 입출력이 명확하게 정의되어야 코드 테스트 이후 UI테스트까지 매끄럽게 이어질거라 생각했어요 👍

전체적인 리팩토링은 다음 레포를 참조했습니다.

리팩토링 1. Input & Output 정의

ViewModelType이라는 프로토콜을 새로 정의했습니다. 뷰모델 클래스들은 해당 프로토콜을 채택합니다.

protocol ViewModelType {
    associatedtype Input
    associatedtype Output

    func transform(input: Input) -> Output
}
class MyViewModel: ViewModelType {
  struct Input { } 
  struct Output { }

  func transform(input: Input) { return Output() } 
}

위와 같은 형태로 구현됩니다. 기본적으로 인풋 & 아웃풋 & 트랜스폼의 절차는

  1. 뷰에서 반응형으로 표현하기 위한 인풋을 정의 (ex - API요청 이후 리액티브하게 화면 표기를 변경해주고 싶은데, 어떤 타이밍에 요청을 날릴 지에 대한 정의)
  2. 뷰모델 인풋을 기반으로 어떤 데이터를 보여줄지 아웃풋을 정의 (ex - 테이블뷰 셀에 바인딩해줄 데이터에 대한 정의)
  3. 데이터 처리 및 정제 과정을 transform 함수에서 정의

완료함 메인화면 기준으로 인풋을 정의해보면 다음과 같은 것들이 있습니다.

  1. viewDidAppear - 완료함 진입 인풋 트리거이후 API 요청
  2. selection - 테이블뷰 선택 이벤트
  3. retrieveNextPageTrigger - 해당 뷰에 대해서만 적용되는 특수한 상황인데, viewDidLoad 함수가 네비게이션 컨트롤러에 새로운 뷰를 푸시 & 팝 할때 두 상황 모두 호출이 되어버려서 내부적으로 로직을 추가해두었습니다.

viewDidAppear를 기준으로 설명하면, 트랜스폼을 통한 데이터의 정제 과정이 아래와 같이 이루어집니다.

func transform(input: Input) -> Output {
    let count = BehaviorRelay(value: 0)
    
    input.viewDidAppear
        .flatMapLatest { [unowned self] () -> Observable<BaseModel<RetrospectCount>> in
            return self.requestEnabledRetrospectCount().asObservable()
        }
        .subscribe(onNext: {
            count.accept($0.data.count)
        })
        .disposed(by: bag)

        return Output(retrospectCount: count)
}

이렇게 되면 회고작성 가능한 갯수가 뷰 컨트롤러에 아웃풋을 통해 전달되고 해당 데이터를 뷰에 바인딩해주면 됩니다.

// 완료함 뷰컨
func bindViewModel() {
    // MARK: - 리팩토링 코드
    let input = CompletionViewModel.Input(viewDidAppear: viewDidAppearTrigger)
    let output = viewModel.transform(input: input)
    
    // 1. 회고 작성가능 갯수 카운트 레이블 바인딩
    
    output.retrospectCount
        .map { count -> NSAttributedString in
            let stringValue = "\(count)개의 목표 회고를 작성할 수 있어요!"
            let attributedString: NSMutableAttributedString = NSMutableAttributedString(string: stringValue)
            attributedString.setColorForText(textForAttribute: "\(count)개의 목표 회고", withColor: .pointPurple)
            return attributedString
        }
        .bind(to: alertBox.label.rx.attributedText)
        .disposed(by: disposeBag)

리팩토링 2. 네트워크 함수 Single Trait 적용

현재 네트워크 요청은 Observable<Result<Success, Failure>>를 기반으로 처리하여, 구독자가 onNext 내에서 switch - case 로 failure 케이스 내에서 에러처리를 진행하고 있습니다.

이렇게 되면 사실상 에러가 반환된 데이터임에도 next 이벤트로 방출되어 에러처리가 어렵다는 문제가 있습니다. Single Trait을 사용하면 이러한 문제를 쉽게 해결 가능합니다.

리팩토링 과정중이라, 기존 코드를 모두 수정할 수는 없어서 실험적으로 완료함에만 적용하기 위해 APIService, APISession 프로토콜에 함수 하나를 추가했습니다.

protocol APIService {
    func request<T: Codable> (_ request: APIRouter) -> Observable<Result<T, APIError>>
    func requestSingle<T: Codable> (_ request: APIRouter) -> Single<T>
}

func requestSingle<T: Codable>(_ request: APIRouter) -> Single<T> {
        return Single<T>.create { observer -> Disposable in
                let request = API.session.request(request, interceptor: APIInterceptor()).responseDecodable { (response: DataResponse<T, AFError>) in
                    guard let statusCode = response.response?.statusCode else {
                        observer(.failure(APIError.unknown))
                        return
                    }

                    guard (200 ... 399).contains(statusCode) else {
                        observer(.failure(APIError.http(status: statusCode)))
                        return
                    }

                    guard let decoded = response.data?.decode(T.self) else {
                        observer(.failure(APIError.decode))
                        return
                    }

                    observer(.success(decoded))
                    return
                }

                return Disposables.create {
                    request.cancel()
                }
            }
    }

requestSingle 함수가 Single 기반으로 네트워크를 요청하는 함수인데, 뷰모델에서 ServiceGoalList와 같이 API 요청 함수들을 모아둔 프로토콜을 채택하기 때문에 ServiceGoalList 프로토콜에서 옵저버블을 리턴하는 함수가 아닌 Single을 리턴하는 함수를 추가합니다.

protocol ServicesGoalList {
  func requestAllGoalsWithSignle(lastGoalId: Int, goalStatusParameter: GoalStatusParameter) -> Single<BaseModel<GoalResponse>>
}

이렇게 되면 옵저버블에서 에러처리를 할때 onNext에서 하는것이 아닌 onError에서 처리할 수 있게 되어 코드 직관성이 더 좋아질거라 생ㄱ각합니다.

기타

리팩토링 적용은 제가 독립적으로 맡은 부분에만 적용되도록 구성해놓아서 다른 곳에 영향은 없는 상태입니다!
뷰모델 정의시 Input & Output 기반으로 생각하면 서로 코드 리뷰에도 도움이 많이 될 것 같아서 PR 확인 & 레퍼런스로 달아둔 문서들 읽어보시공 자유롭게 코멘트 주세요 !

Reference

  1. 완료함 리팩토링 과정 - 본인 블로그 게재
  2. RxSwift sample app
  3. RxSwift + MVVM: how to feed ViewModels

셀프 체크리스트

  • Merge 하는 브랜치가 올바른가?
  • 코딩 컨벤션을 준수하는가?
  • PR과 관련없는 변경사항이 없는가?
  • 내 코드에 대한 자기 검토가 되었는가?
  • 오른쪽의 Development에서 이슈와 연결하였는가?

@Parkjju Parkjju added refactor 코드 개선, 리팩토링 jun jun(박경준)의 이슈 labels Oct 3, 2023
@Parkjju Parkjju requested a review from EunsuSeo01 October 3, 2023 12:21
@Parkjju Parkjju self-assigned this Oct 3, 2023
@EunsuSeo01
Copy link
Collaborator

확인했습니다! Input Output 쓰는 거 좋은 것 같아요~ 수고 많으셨습니다!

@Parkjju Parkjju merged commit 8ab2e18 into develop Oct 20, 2023
@Parkjju Parkjju deleted the 178-refactor/input branch October 20, 2023 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jun jun(박경준)의 이슈 refactor 코드 개선, 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[test/tutorial] 테스트코드 튜토리얼 문서를 참조하여 완료함에 적용한다.
2 participants