부스트캠프 모바일 6기 코드 리뷰어 지원 #200
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 데이터 바인딩
| } | ||
|
|
||
| func showActivityDetailViewController() { | ||
| // TODO: detailVM 생성 실패시 처리가 필요함!!! |
There was a problem hiding this comment.
TODO가 해결된건지 아직 처리해야하는게 남은건지 모르겠네요.
두 경우 다 일단 이 주석이 guard 문 내부에 있는게 좀 더 위치가 정확할거 같아요.
closeSignal.send() 가 처리를 한건지 더 추가로 해야될게 있는지 모르겠어요.
| if times > 0 { | ||
| return String(format: "%02d:%02d", times, minutes) | ||
| } | ||
| return String(format: "%02d:%02d", minutes, seconds) |
There was a problem hiding this comment.
if 문을 사용할때 논리적으로 확실하게 모든 케이스를 커버한다는 생각으로 작성하기위해 항상 else문이 있다고 생각하고 작성하는 것이 좋습니다.
if문만 있는거랑 if {} else {} 가 짝으로 있는건 완전히 다른 의도의 코드입니다.
| guard let image = UIGraphicsGetImageFromCurrentImageContext() else { return UIImage() } | ||
| return image |
There was a problem hiding this comment.
guard문은 보통 메소드 상단에서 메소드 내의 처리를 하기전에 조건을 걸어 탈락시키는 방어코드를 사용할때 활용하는게 좋습니다.
틀린 문법은 아니지만 guard문 위에 너무 많은 코드가 있어서 제가 처음 봤을땐 추후에 에러를 유발할 수 있는 메소드로 보입니다.
| if Date.isSameWeek(date: range.start, dateOfWeek: date) { | ||
| return "이번 주" | ||
| } else if | ||
| let lastWeekDate = Calendar.current.date(byAdding: .day, value: -7, to: date), | ||
| Date.isSameWeek(date: range.start, dateOfWeek: lastWeekDate) | ||
| { | ||
| return "저번 주" | ||
| } | ||
|
|
||
| if Date.isSameYear(date: range.start, dateOfYear: date) { | ||
| return range.start.toMDString + "~" + range.end.toMDString | ||
| } |
There was a problem hiding this comment.
모든 조건을 다 커버하는지 의심이 듭니다.
depth가 깊어지더라도 else문을 명확하게 넣어서 한눈에 모든 조건이 커버되도록 수정하는게 더 좋을 것 같습니다.
| if startIndex < endIndex { | ||
| distance = runningStates[endIndex].distance - runningStates[startIndex].distance | ||
| } else { | ||
| distance = 0 | ||
| } |
| motion.startDeviceMotionUpdates(using: .xMagneticNorthZVertical) | ||
|
|
||
| timer = Timer(fire: Date(), interval: 1.0 / 100.0, repeats: true, | ||
| block: { _ in |
There was a problem hiding this comment.
block 내부의 강한 self 캡처링으로 인해 위험해보이는 코드입니다.
클로저의 캡처링과 순환참조에 대해 확인하고 넘어가시기 바랍니다.
| } | ||
|
|
||
| func startCountingSteps() { | ||
| if isActive { return } |
There was a problem hiding this comment.
여긴 guard문을 쓰는게 더 의도가 잘 드러날 것 같습니다
Issue Number
Close #
변경사항
새로운 기능
작업 유형
체크리스트