부스트캠프 6기 iOS 코드 리뷰어 지원

[cozzin]

No description provided.

[cozzin]

Suggested change

	var appCoordinator: AppCoordinator?
	private var appCoordinator: AppCoordinator?

사소한 부분이지만 접근제어를 가능한 닫아두는 것이 유지보수에 좋습니다.
예를들어 외부에서 appCoordinator에 접근해서 변경한다면 일관성 있는 동작을 보장하기 어려워집니다.

[cozzin]

Suggested change

	return showMainFlow()
	showMainFlow()

여기서 return 을 해도 컴파일 에러는 발생하지 않지만
return 된 Void 값을 사용하지 않기 떄문에 return을 제거하는 것이 가독성에 좋아보입니다.

[cozzin]

Suggested change

	var navigationController: UINavigationController
	let navigationController: UINavigationController

BasicCoordinator init 시에 navigationController를 할당하고 같은 객체를 계속 사용하게 되는데요.
더 이상 변경이 없음을 명확하게 알려주기 위해서 let 을 쓰는게 좋을 것 같아요.

[cozzin]

릴리즈된 앱에서 print 로그가 출력되는 것은 무의미하고 때로는 성능저하가 있을 수 있기 때문에 DEBUG 시에만 출력되도록 설정하는 것이 좋습니다. 아래와 같은 트릭을 사용하면 간편하게 개선할 수 있습니다.

public func print(_ items: Any..., separator: String = " ", terminator: String = "\n") {
    #if DEBUG
    Swift.print(items, separator: separator, terminator: terminator)
    #endif
}

조금 더 심화된 학습을 해보고 싶다면 Logger를 직접 만들어보는 것도 좋습니다.
log Level에 따라서 메세지를 다르게 출력할 수 있습니다.
verbose, debug, info, warnings, error 단계로 나눠서 출력하고 error 시에는 리모트 서버 또는 디바이스 파일에 기록을 남기는 방법도 있습니다.
참고: https://github.com/emaloney/CleanroomLogger

[cozzin]

Suggested change

	func setGoal(_ goalInfo: GoalInfo?) {
	func setGoal(_ goalInfo: GoalInfo) {

Optional이 아니어도 컴파일 에러는 없네요.
가능하면 init 시점에 GoalInfo를 주입하는 것으로 리팩토링 해봐도 좋을 것 같아요.

[cozzin]

Suggested change

	var runningInfoSubjects = [
	RunningInfoTypeSubject(RunningInfo(type: .time)),
	RunningInfoTypeSubject(RunningInfo(type: .pace)),
	RunningInfoTypeSubject(RunningInfo(type: .averagePace)),
	RunningInfoTypeSubject(RunningInfo(type: .kilometer)),
	]
	var runningInfoTapAnimationSignal = PassthroughSubject<Int, Never>()
	var initialAnimationSignal = PassthroughSubject<Void, Never>()
	var resumeAnimationSignal = PassthroughSubject<Void, Never>()
	var showPausedRunningSignal = PassthroughSubject<Void, Never>()
	let runningInfoSubjects = [
	RunningInfoTypeSubject(RunningInfo(type: .time)),
	RunningInfoTypeSubject(RunningInfo(type: .pace)),
	RunningInfoTypeSubject(RunningInfo(type: .averagePace)),
	RunningInfoTypeSubject(RunningInfo(type: .kilometer)),
	]
	let runningInfoTapAnimationSignal = PassthroughSubject<Int, Never>()
	let initialAnimationSignal = PassthroughSubject<Void, Never>()
	let resumeAnimationSignal = PassthroughSubject<Void, Never>()
	let showPausedRunningSignal = PassthroughSubject<Void, Never>()

프로토콜에서 var 로 되어있어서 그대로 유지하는 경우가 많은데요.
{ get } 으로만 프로퍼티 제약이 걸려있는 경우에 let 을 사용해도 충분한 상황이 있습니다.

[cozzin]

현재는 closeSubscription에 직접 접근해서 변경하도록 되어있는데요.
BasicCoordinator에서 private var closeSubscription 로 변경하고
아래의 함수와 비슷한 역할을 하는 함수가 있으면 더 명확할 것 같아요.

private func store<T>(coordinator: BasicCoordinator<T>) {
    childCoordinators[coordinator.identifier] = coordinator
}

[cozzin]

Suggested change

	private var viewModel: RunningPageViewModelTypes?
	private let viewModel: RunningPageViewModelTypes

init 시점에 viewModel이 주입되기 떄문에 optional이 아닌 타입으로 사용 가능합니다.

[cozzin]

viewDidLoad 이후에도 view.bounds 가 변경될 가능성이 있습니다.
viewDidLayoutSubviews 나 다른 lifecycle에서 계산해주는 것도 좋을 것 같아요.

[cozzin]

UITableViewDataSource 구현을 따로 분리한 것도 좋은 시도 같아요 👍
ViewController 코드가 과도하게 증가하는 것을 막아줄 수 있겠네요.