Conversation
async let -> TaskGroup
Summary of ChangesHello @dev-domo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 사용자 인증 및 온보딩 경험을 개선하고, WeatherKit 통합을 통해 위치 기반 기능을 강화하는 데 중점을 둡니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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
이번 PR은 로그인 및 사용자 데이터 처리 로직을 대대적으로 리팩토링하는 내용을 담고 있습니다. 전반적으로 서버 API를 통해 상태를 관리하도록 변경하고, UserDefaults 의존성을 줄인 점이 인상적입니다. 특히 Apple 로그인 시 사용자 이름을 가져와 닉네임 설정 단계를 건너뛰는 기능과, UITableView의 드래그 앤 드롭 및 스와이프 액션 로직을 프로토콜과 익스텐션으로 분리하여 코드 중복을 줄인 점은 매우 훌륭한 개선입니다. 몇 가지 추가 개선 제안 사항을 리뷰 코멘트로 남겼습니다. 오류 처리 방식과 코드 중복 제거에 관한 내용이니 확인 부탁드립니다.
| func getMemberName() async throws -> MemberNameEntity { | ||
| do { | ||
| let memberName = try await fetchMemberName() | ||
| return memberName | ||
| } catch { | ||
| return .stub() | ||
| } | ||
| } |
There was a problem hiding this comment.
getMemberName() 함수가 오류 발생 시 .stub()을 반환하도록 구현되어 있습니다. 이 방식은 API 호출 실패나 다른 오류가 발생했을 때 실제 원인을 파악하기 어렵게 만들고, 사용자에게는 기본값("워리")이 보여 혼란을 줄 수 있습니다. throws 키워드를 사용하는 함수인 만큼, catch 블록에서 오류를 처리하기보다는 상위 호출 지점으로 오류를 다시 던져(rethrow) UI/ViewModel 레벨에서 적절한 오류 처리(예: 에러 메시지 표시)를 할 수 있도록 하는 것이 더 견고한 설계입니다.
func getMemberName() async throws -> MemberNameEntity {
let memberName = try await fetchMemberName()
return memberName
}| guard let accessToken = keyChainService.load(key: .accessToken) else { | ||
| BeforeGoingLogger.error(BeforeGoingError.accessTokenMissing) | ||
| return .stub() | ||
| } |
There was a problem hiding this comment.
accessToken이 없을 경우, 오류를 던지는 대신 .stub()을 반환하고 있습니다. 토큰이 없는 것은 인증이 필요한 요청에서 명백한 오류 상황이므로, BeforeGoingError.accessTokenMissing 오류를 던져서 호출자가 이 상황을 명확히 인지하고 처리하도록 하는 것이 좋습니다. 현재 구현은 오류를 숨기고 기본값을 반환하여 잠재적인 버그를 유발할 수 있습니다.
guard let accessToken = keyChainService.load(key: .accessToken) else {
BeforeGoingLogger.error(BeforeGoingError.accessTokenMissing)
throw BeforeGoingError.accessTokenMissing
}| private func isAgreedTerms(accessToken: String) async throws -> Bool { | ||
| do { | ||
| let _ = try await networkService.request( | ||
| endPoint: TermsAPI.getTerms(accessToken: accessToken), | ||
| responseType: TermsResponseDTO.self | ||
| ) | ||
| return true | ||
| } catch { | ||
| return false | ||
| } | ||
| return isCompleted | ||
| } |
There was a problem hiding this comment.
isAgreedTerms(accessToken:) 함수가 TermsRepository에도 동일하게 구현되어 있어 코드 중복이 발생했습니다. 약관 관련 로직은 TermsRepository에서 관리하는 것이 역할 분리 측면에서 더 적절해 보입니다. TermsRepository의 isAgreedTerms 함수를 public으로 변경하고, AuthRepository에서는 TermsInterface를 통해 해당 함수를 호출하여 사용하는 방식으로 리팩토링하면 중복을 제거하고 코드 유지보수성을 높일 수 있습니다.
| try await withThrowingTaskGroup(of: Void.self) { group in | ||
| group.addTask { [weak self] in | ||
| try await self?.requestDate() | ||
| } | ||
|
|
||
| group.addTask { [weak self] in | ||
| try await self?.updateWeatherInformation(date: DateUtil.getCurrentDate()) | ||
| } | ||
|
|
||
| try await group.waitForAll() | ||
| } |
There was a problem hiding this comment.
withThrowingTaskGroup의 else 블록에서 try await group.waitForAll()을 호출하고 있습니다. withThrowingTaskGroup은 클로저의 실행이 끝나면 모든 자식 작업(child task)이 완료될 때까지 암시적으로 기다리므로, 이 코드는 불필요합니다. 해당 라인을 제거하여 코드를 더 간결하게 만들 수 있습니다.
try await withThrowingTaskGroup(of: Void.self) { group in
group.addTask { [weak self] in
try await self?.requestDate()
}
group.addTask { [weak self] in
try await self?.updateWeatherInformation(date: DateUtil.getCurrentDate())
}
}| private func createSwipeAction(deleteAction: UIContextualAction) -> UISwipeActionsConfiguration { | ||
| let config = UISwipeActionsConfiguration(actions: [deleteAction]) | ||
| config.performsFirstActionWithFullSwipe = false | ||
| return config | ||
| let swipeActionsConfig: UISwipeActionsConfiguration = { | ||
| let config = UISwipeActionsConfiguration(actions: [deleteAction]) | ||
| config.performsFirstActionWithFullSwipe = false | ||
| return config | ||
| }() | ||
| return swipeActionsConfig | ||
| } |
There was a problem hiding this comment.
createSwipeAction 함수가 즉시 실행 클로저(immediately-executed closure)를 사용하도록 변경되었는데, 이로 인해 코드가 이전보다 더 길어지고 가독성이 다소 저하되었습니다. 특별한 이유가 없다면, 이전처럼 상수를 선언하고 프로퍼티를 설정한 뒤 반환하는 간결한 형태로 되돌리는 것이 좋겠습니다.
private func createSwipeAction(deleteAction: UIContextualAction) -> UISwipeActionsConfiguration {
let config = UISwipeActionsConfiguration(actions: [deleteAction])
config.performsFirstActionWithFullSwipe = false
return config
}
|
1 participant
✅ PR 타입
반영 브랜치
refactor/#99-reject -> dev
✨ 변경 사항
📂 관련 이슈
#99