부스트캠프 6기 iOS 코드 리뷰어 지원 PR #201
Conversation
(boostcamp-2020#25 boostcamp-2020#26) ActivityScene TotalView 연결 및 Date PickerView 구현
(boostcamp-2020#97) feat: running split scene 구현
(boostcamp-2020#91) feat: Location accuracy에 따른 값 전송 유무 로직 추가
(boostcamp-2020#88) Motion Type에 따라 자동으로 러닝을 일시정지 및 재시작한다
…e <--> EditProfileScene)
(boostcamp-2020#191) fix: 다크모드 및 라이트모드 toggle시 annotation이 바뀌는 버그 수정
…/179 (boostcamp-2020#136, 161, 179) 및 코드 컨벤셔닝
fix: Calorie 누적 오류 Int -> Double
(boostcamp-2020#196)fix: pausedWorkout 사운드 문제
(boostcamp-2020#198) feat: split detail 데이터 바인딩
| init(navigationController: UINavigationController) { | ||
| self.navigationController = navigationController | ||
| navigationController.setNavigationBarHidden(true, animated: true) | ||
| print("[Memory \(Date())] 🌈Coordinator🌈 \(Self.self) started") | ||
| } |
There was a problem hiding this comment.
release에 print문이 들어가면 I/O 작업이기 때문에 성능에 영향을 줄 수 있습니다.
#if DEBUG
print("[Memory \(Date())] 🌈Coordinator🌈 \(Self.self) started")
#endif이런식으로 디버깅 모드를 구분할 수 있습니다.
디버깅 print문 출력을 담당하는 객체를 두는것도 좋은 방법일 것 같습니다.
| static func saveImageDataToDocumentsDirectory(fileName: String, imageData: Data) { | ||
| let url = URL.documents.appendingPathComponent(fileName) | ||
| do { | ||
| try imageData.write(to: url) | ||
| } catch { | ||
| print(error) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
에러를 핸들링 할때 print문 보다는 오류를 사용자에게 보여주는것이 UX측면에서 좋습니다:)
| var period: String { | ||
| guard let hourMinSec = self.hourMinSec else { return "알수없는 시간대" } | ||
| switch hourMinSec.hour { | ||
| case 22 ... 24, 0 ..< 3: | ||
| return "야간" | ||
| case 3 ..< 6: | ||
| return "새벽" | ||
| case 6 ..< 12: | ||
| return "오전" | ||
| case 12 ..< 18: | ||
| return "오후" | ||
| case 18 ..< 22: | ||
| return "저녁" | ||
| default: | ||
| return "" | ||
| } | ||
| } |
There was a problem hiding this comment.
이렇게 Text를 관리하는 경우, 사용자 언어설정에 따라 영어나 다른 언어도 지원하고 싶다면 Localizing에 대해서 공부해 보시는것도 좋습니다 :)
| extension UINavigationController { | ||
| // 사용하는 뷰컨에서 viewDidDisappear 시 removeFromParent로 없애줄 것 | ||
| func setStatusBar(backgroundColor: UIColor) -> UIView { | ||
| let statusBarFrame: CGRect | ||
| if #available(iOS 13.0, *) { | ||
| statusBarFrame = view.window?.windowScene?.statusBarManager?.statusBarFrame ?? CGRect.zero | ||
| } else { | ||
| statusBarFrame = UIApplication.shared.statusBarFrame | ||
| } | ||
| let statusBarView = UIView(frame: statusBarFrame) | ||
| statusBarView.backgroundColor = backgroundColor | ||
| view.addSubview(statusBarView) | ||
| return statusBarView | ||
| } | ||
| } |
There was a problem hiding this comment.
이런식으로 statusBar를 만들고, 다른 뷰컨트롤러에서 viewDidDisappear시 이 setStatusBarView를 removeFromParent로 지워줘야한다면 view를 반환하지말고 tag로 이를 구분해 자체적으로 UINavigationController extension내에 clearStatusBar()를 두는건 어떨까요?
| extension UIStackView { | ||
| static func make( | ||
| with subviews: [UIView], | ||
| axis: NSLayoutConstraint.Axis = .horizontal, | ||
| alignment: Alignment = .fill, | ||
| distribution: Distribution = .fill, | ||
| spacing: CGFloat = 0 | ||
| ) -> UIStackView { | ||
| let view = UIStackView(arrangedSubviews: subviews) | ||
| view.axis = axis | ||
| view.alignment = alignment | ||
| view.distribution = distribution | ||
| view.spacing = spacing | ||
| return view | ||
| } | ||
| } |
There was a problem hiding this comment.
이런식으로 stackView를 만드니 굉장히 깔끔하네요 👍
| import UIKit | ||
|
|
||
| class ActivityListHeaderView: UICollectionReusableView { | ||
| lazy var categoryLabel = makeValueLabel() |
There was a problem hiding this comment.
lazy var를 쓰신곳이 많이 보입니다. 이 경우 문제가 되진 않지만 lazy var는 thread safe하지 않으니 멀티 스레드 환경 동시 접근에서는 주의가 필요합니다 :)
| let paceLabel = UILabel() | ||
| let changeLabel = UILabel() | ||
|
|
||
| var viewModel: RunningSplitCellViewModelType? |
There was a problem hiding this comment.
cell에 viewModel이 있는데 어떤 이유일까요?🙂
|
|
||
| override func viewDidAppear(_ animated: Bool) { | ||
| super.viewDidAppear(animated) | ||
| viewModel?.inputs.viewDidLoad() |
There was a problem hiding this comment.
여기 보면 viewModel viewDidLoad가 viewDidAppear에서 불리고 있는데, 뭔가 이상하지 않나요?
| private func bindViewModel() { | ||
| guard let viewModel = viewModel else { return } | ||
| viewModel.outputs.userLocationSubject | ||
| .receive(on: DispatchQueue.main) |
There was a problem hiding this comment.
어떤곳은 RunLoop.main이고 어떤곳은 DispatchQueue.main인데 이 둘의 차이를 아시나요?😊
| final class GoalValueView: UIView { | ||
| private lazy var goalValueLabel: UILabel = makeGoalValueLabel() | ||
|
|
||
| var tapAction: (() -> Void)? |
There was a problem hiding this comment.
이 부분도 publisher로 만들면 좋을것 같아요:)
No description provided.