Conversation
Feature/mission result view#6
- 공통 레이블 설정 값으로 재 수정 작업 진행
🌟feat: 자유항행 - View 제작: 상단 타이머 화면 구현
- 변경된 공통 레이블 설정값으로 변경 작업 진행
[FEAT] 자유항행 - View 제작: 하단 레코드 화면 구현 #9
🐛fix: AlarmListViewCell 메서드 주석처리, ViewController 연결
[FEAT] 자유항행 - View 제작: 상단 및 하단 통합 VC 설계 및 구현 #10
- StopWatch Viewmodel 설계 - 버튼 눌렀을때 시간 동작 기능 구현 - 같은 버튼 눌렀을때 Pause 기능 구현 ref #23
[PR] 상세 기록 화면 컬렉션 뷰 헤더 및 상세보기 버튼 추가 (HomeTab#50)
🌟feat: 홈으로가기 버튼 삭제
Updated README to enhance project description and details.
🐞fix: 잘못된 단위로 표시되던 미션 결과 데이터 수정
✨feat: ActivatedMissionCell에 ProgressView 추가
[PR] 미션 결과 표시 형식 변경 및 버그 수정 (HomeTab#57)
✨feat: CustomMissionCell 삭제 기능 추가
♻️ refactor: 오타 수정, 오류 출력 수정
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 풀리퀘스트는 우주 테마의 시간 관리 앱인 'RocketCall'의 핵심 기능인 알람, 미션(뽀모도로), 자유 항행(스톱워치) 기능을 MVVM 아키텍처와 RxSwift를 기반으로 구현하고, CoreData를 통한 데이터 영속성 및 로컬 알림 관리 로직을 추가합니다. 코드 리뷰 결과, 프로덕션 환경에서 앱 충돌을 유발할 수 있는 fatalError 사용과 특정 인덱스에 의존하는 로직 등 안정성 측면의 개선이 필요합니다. 또한, 코드의 가독성과 유지보수성을 높이기 위해 매직 넘버를 상수로 정의하고, 알림 권한 요청 및 미션 결과 처리 시의 에러 핸들링을 강화할 것을 권장하며, 오타로 인한 컴파일 오류 수정이 필요합니다.
| if let error = error as NSError? { | ||
| fatalError("Unresolved error \(error), \(error.userInfo)") | ||
| } |
There was a problem hiding this comment.
fatalError를 사용하여 앱을 강제 종료하는 것은 프로덕션 환경에서 데이터 손실이나 좋지 않은 사용자 경험을 유발할 수 있습니다. 영구 저장소를 로드하지 못하는 경우, 사용자에게 오류를 알리고 앱을 안전하게 종료하거나 복구 흐름을 제공하는 것이 더 안정적입니다.
| // Replace this implementation with code to handle the error appropriately. | ||
| // fatalError() causes the application to generate a crash log and terminate. You should not use this function in a shipping application, although it may be useful during development. | ||
| let nserror = error as NSError | ||
| fatalError("Unresolved error \(nserror), \(nserror.userInfo)") | ||
| } |
There was a problem hiding this comment.
saveContext에서 오류 발생 시 fatalError를 호출하면 앱이 비정상적으로 종료될 수 있습니다. 데이터 저장 실패는 다양한 원인(예: 디스크 공간 부족)으로 발생할 수 있으며, 이 경우 앱을 강제 종료하기보다는 오류를 기록하고 사용자에게 상황을 알리는 것이 더 안정적인 방법입니다.
|
|
||
| do { | ||
| // 무음모드 일때도 소리나게 하기 | ||
| try AVAudioSession.sharedInstance().setCategory(.playback, mode: . default) |
There was a problem hiding this comment.
mode: . default에 불필요한 공백이 있어 컴파일 오류가 발생합니다. mode: .default로 수정해야 합니다.
| try AVAudioSession.sharedInstance().setCategory(.playback, mode: . default) | |
| try AVAudioSession.sharedInstance().setCategory(.playback, mode: .default) |
| guard let self else { fatalError("Error: self is nil") } | ||
| switch itemIdentifier { | ||
| case .activatedMission(let mission): | ||
| guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: ActivatedMissionCell.id, for: indexPath) as? ActivatedMissionCell else { fatalError("ActivatedMissionCell dequeueReusableCell error") } |
There was a problem hiding this comment.
셀 캐스팅 실패로 인한 앱 충돌을 방지하기 위해, fatalError를 호출하는 대신 빈 셀을 반환하도록 수정하세요. 이는 추후 더 견고한 오류 처리 및 사용자 알림을 구현하기 위한 계획의 일환입니다.
| guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: ActivatedMissionCell.id, for: indexPath) as? ActivatedMissionCell else { fatalError("ActivatedMissionCell dequeueReusableCell error") } | |
| guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: ActivatedMissionCell.id, for: indexPath) as? ActivatedMissionCell else { return UICollectionViewCell() } |
References
- To prevent application crashes from cell casting failures during dequeuing, return an empty cell instead of calling fatalError, with a plan to implement more robust error handling and user notifications later.
| center.requestAuthorization(options: options) { granted, error in | ||
| } |
There was a problem hiding this comment.
requestAuthorization의 완료 핸들러에서 error를 처리하지 않고 있습니다. 권한 요청 중에 오류가 발생할 수 있으며, 이를 무시하면 잠재적인 문제를 진단하기 어려워집니다. 오류를 로깅하거나 적절히 처리하는 것이 좋습니다.
center.requestAuthorization(options: options) { granted, error in
if let error = error {
print("Notification authorization error: \(error.localizedDescription)")
}
}| var dateComponents = DateComponents() | ||
| dateComponents.hour = alarm.hour | ||
| dateComponents.minute = alarm.minute | ||
| dateComponents.second = i * 9 |
There was a problem hiding this comment.
매직 넘버 9가 코드에 직접 사용되어 의도를 파악하기 어렵습니다. 이 값을 let 상수로 정의하고 의미 있는 이름을 부여하면 코드 가독성과 유지보수성이 향상됩니다. 예를 들어, notificationRepeatInterval과 같은 이름으로 정의할 수 있습니다.
| content.interruptionLevel = .timeSensitive // 방해 금지여도 알람 | ||
|
|
||
| // 2. 시간 설정 | ||
| let trigger = UNTimeIntervalNotificationTrigger(timeInterval: 300, repeats: false) |
There was a problem hiding this comment.
매직 넘버 300이 다시 알림(snooze) 기능에 사용되고 있습니다. 이 값을 snoozeTimeInterval과 같은 이름의 상수로 정의하여 코드의 명확성을 높이는 것이 좋습니다.
| let trigger = UNTimeIntervalNotificationTrigger(timeInterval: 300, repeats: false) | |
| let trigger = UNTimeIntervalNotificationTrigger(timeInterval: 5 * 60, repeats: false) // 5분 |
| } | ||
|
|
||
| // 24초 딜레이 | ||
| DispatchQueue.global().asyncAfter(deadline: .now() + 27.0) { |
There was a problem hiding this comment.
매직 넘버 27.0이 코드에 직접 사용되어 그 의미를 알기 어렵습니다. 이 값은 알람이 반복될 때 다시 예약되기까지의 지연 시간으로 보입니다. 이 값을 상수로 정의하고 명확한 이름을 부여하여 코드의 가독성을 높이는 것이 좋습니다.
| private func showMissionResult(resultId: UUID) { | ||
| selectedIndex = 2 | ||
| // 네비게이션 컨트롤러 꺼냄 | ||
| guard let missionNavigationController = viewControllers?[2] as? UINavigationController else { return } |
There was a problem hiding this comment.
viewControllers 배열의 특정 인덱스(2)에 의존하여 UINavigationController를 가져오고 있습니다. 탭 바의 순서가 변경될 경우 이 코드는 예기치 않게 동작하거나 실패할 수 있습니다. 타입 기반으로 뷰 컨트롤러를 찾는 것이 더 안정적입니다.
| guard let missionNavigationController = viewControllers?[2] as? UINavigationController else { return } | |
| guard let missionNavigationController = viewControllers?.first(where: { ($0 as? UINavigationController)?.viewControllers.first is MissionViewController }) as? UINavigationController else { return } |
| let payload = try coreDataManager.fetchMissionResult(of: resultId) | ||
| missionResultView.configure(with: payload) | ||
| } catch { | ||
| // TODO: 오류 처리 로직 구현 | ||
| missionResultView.configure(with: samplePayload) |
There was a problem hiding this comment.
사용자에게 오류를 표시할 때는 일반적인 메시지 대신 오류 유형에 따른 구체적이고 의미 있는 메시지를 제공해야 합니다. 현재처럼 샘플 데이터를 사용하여 오류를 숨기는 대신, 적절한 오류 알림을 표시하세요.
References
- When displaying errors to the user, provide specific and meaningful messages based on the error type instead of a generic message.
5 participants
고생많으셨습니다