[Refactor] 뷰잇 모델 수정 #275
[Refactor] 뷰잇 모델 수정 #275
Conversation
- 사용하지 않는 Like는 삭제
WalkthroughViewit의 좋아요 표현을 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 사용자
participant VC as ViewController
participant VM as ViewModel
participant UC as LikeUseCase
participant Repo as Repository/Mapper
U->>VC: 좋아요 토글 액션
VC->>VM: toggleLike(viewit)
VM->>UC: (viewit.isLiked ? unlike(viewit) : like(viewit))
UC->>UC: viewit.like()/viewit.unlike() %% 변경된 내부 변이
UC-->>VM: AnyPublisher<Viewit?, WableError>
VM->>VM: viewitList 업데이트 (스냅샷)
VM-->>VC: UI 갱신 데이터
VC-->>U: 하트 상태 및 카운트 반영
note right of Repo: 도메인 모델은 isLiked/likeCount로 평탄화됨
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift (2)
138-141: Relay의 in-place 변경은 구독자에게 전파되지 않습니다 — send로 갱신하세요
viewitListRelay.value[index] = viewit는 내부 배열만 수정하고 이벤트를 발행하지 않아 UI가 갱신되지 않을 수 있습니다. 배열을 복사/수정 후send로 내보내세요.Apply this diff:
- .sink { [weak self] index, viewit in - self?.viewitListRelay.value[index] = viewit - } + .sink { [weak self] index, viewit in + guard let self = self else { return } + var list = self.viewitListRelay.value + list[index] = viewit + self.viewitListRelay.send(list) + }필요하시면 동일 패턴을 사용하는 다른 지점도 함께 패치해 드리겠습니다.
172-173: 동일 이슈: 페이지 추가/삭제 시에도 in-place 변경 대신 send 필요아래 두 곳도 배열을 직접 변경만 하고 발행하지 않습니다. 동일하게 수정해주세요.
- .sink { [weak self] in self?.viewitListRelay.value.append(contentsOf: $0) } + .sink { [weak self] items in + guard let self = self else { return } + var list = self.viewitListRelay.value + list.append(contentsOf: items) + self.viewitListRelay.send(list) + }- .sink { [weak self] index, _ in - self?.viewitListRelay.value.remove(at: index) - } + .sink { [weak self] index, _ in + guard let self = self else { return } + var list = self.viewitListRelay.value + guard list.indices.contains(index) else { return } + list.remove(at: index) + self.viewitListRelay.send(list) + }Also applies to: 219-220
Wable-iOS/Data/Mapper/ViewitMapper.swift (1)
14-16: 날짜 파싱이 항상 nil이 될 가능성: 빈 dateFormat, 로케일 미설정, 모호한 KST 사용dateFormat이 빈 문자열이며 locale 미설정 상태라 디바이스 로케일에 의존합니다. 또한 "KST" 약어는 모호할 수 있습니다. 백엔드가 ISO-8601(RFC3339) 문자열을 준다면 ISO8601DateFormatter로 교체하세요.
아래처럼 교체를 제안합니다:
- let dateFormatter = DateFormatter() - dateFormatter.dateFormat = "" - dateFormatter.timeZone = TimeZone(abbreviation: "KST") + let isoFormatter = ISO8601DateFormatter() + isoFormatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds]- let time = dateFormatter.date(from: viewit.time) + let time = isoFormatter.date(from: viewit.time)만약 서버가 타임존 정보가 없는 KST 로컬 문자열(예: "yyyy-MM-dd HH:mm:ss")을 준다면 DateFormatter를 사용하되 다음을 권장합니다:
dateFormat명시,locale = en_US_POSIX,timeZone = TimeZone(identifier: "Asia/Seoul"),calendar = .gregorian.Also applies to: 22-22
🧹 Nitpick comments (7)
Wable-iOS/Domain/UseCase/Viewit/LikeViewitUseCase.swift (1)
19-27: 반환 타입을 Optional에서 비-Optional로 단순화 고려성공 시 항상
Viewit를 반환하므로AnyPublisher<Viewit, WableError>로 단순화하면 호출부에서nil분기 제거가 가능합니다. 호출부의catch { .just(nil) }패턴도 자연스럽게 사라집니다.Also applies to: 29-37
Wable-iOS/Domain/Entity/Viewit.swift (2)
26-27: 불변식 보강 제안: likeCount 음수 방지
Likable의 구현(혹은Viewit확장)에서unlike()호출 시likeCount가 0 아래로 내려가지 않도록 최소값을 보장해 주세요.예:
mutating func unlike() { guard likeCount > 0 else { isLiked = false; return } likeCount = max(0, likeCount - 1) isLiked = false }
13-13: Diffable 안정성 검토: Hashable/Equatable 동작 정의현재 자동 합성된
Hashable/Equatable은 모든 프로퍼티를 비교합니다. 좋아요 상태/카운트 변경 시 항목이 “다른 아이템”으로 간주되어 삭제/삽입 애니메이션이 발생할 수 있습니다. 아이덴티티를id로만 두고 싶다면==/hash(into:)를id기반으로 커스터마이징하거나, 현 상태를 유지하되 원하는 애니메이션 정책을 명확히 해주세요.적용 예:
-struct Viewit: Identifiable, Hashable, Likable { +struct Viewit: Identifiable, Hashable, Likable { ... } +extension Viewit { + static func == (lhs: Viewit, rhs: Viewit) -> Bool { lhs.id == rhs.id } + func hash(into hasher: inout Hasher) { hasher.combine(id) } +}주의: 위처럼 아이덴티티만
id로 두면applySnapshot만으로는 내용 변경이 리로드되지 않을 수 있어snapshot.reconfigureItems([item])등의 보완이 필요합니다.Wable-iOS/Data/RepositoryImpl/ViewitRepositoryImpl.swift (1)
119-121: Mock 데이터 스키마 업데이트 확인
isLiked/likeCount로의 전환이 일관되게 반영되었습니다. 목 데이터도 새 도메인에 맞습니다.일부 줄에 불필요한 공백(예:
isLiked: false,)이 있어 제거하면 포맷이 깔끔해집니다.Also applies to: 134-136, 149-151, 164-166, 179-181
Wable-iOS/Data/Mapper/ViewitMapper.swift (3)
20-22: 빈 문자열로 URL 생성 지양 — 옵셔널 체이닝으로 안전 파싱
URL(string: optional ?? "")는 데이터 이슈를 숨기고 불필요한 객체 생성을 유발합니다. 옵셔널에 대해 flatMap으로 안전 파싱하세요.- let thumbnailURL = URL(string: viewit.viewitImage ?? "") - let videoURL = URL(string: viewit.viewitLink ?? "") + let thumbnailURL = viewit.viewitImage.flatMap(URL.init(string:)) + let videoURL = viewit.viewitLink.flatMap(URL.init(string:))
25-29: 불리언 분기 간소화가독성을 위해 삼항 연산자로 단순화 가능합니다.
- let postStatus: PostStatus - if viewit.isBlind { - postStatus = .blind - } else { - postStatus = .normal - } + let postStatus: PostStatus = viewit.isBlind ? .blind : .normal
32-45: ViewitMapper 매핑 검증 및 선택적 방어 적용
- likedNumber와 도메인의 likeCount는 둘 다 non-optional Int로 타입이 일치합니다. 서버가 음수를 반환할 가능성이 없다면 그대로 사용해도 무방하나, 음수 방지를 위해 선택적으로 clamp 적용을 권장합니다.
예시)- likeCount: viewit.likedNumber + likeCount: max(0, viewit.likedNumber)- time은 DTO의 String을 Date?로 파싱하여, 실패 시 nil이 전달됩니다. 도메인에서 Date?로 정의된 것이 의도된 동작인지 비즈니스 요구사항을 재확인하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
Wable-iOS.xcodeproj/project.pbxproj(0 hunks)Wable-iOS/Data/Mapper/ViewitMapper.swift(1 hunks)Wable-iOS/Data/RepositoryImpl/ViewitRepositoryImpl.swift(5 hunks)Wable-iOS/Domain/Entity/Like.swift(0 hunks)Wable-iOS/Domain/Entity/Viewit.swift(2 hunks)Wable-iOS/Domain/UseCase/Viewit/LikeViewitUseCase.swift(2 hunks)Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift(1 hunks)Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift(1 hunks)
💤 Files with no reviewable changes (2)
- Wable-iOS.xcodeproj/project.pbxproj
- Wable-iOS/Domain/Entity/Like.swift
🔇 Additional comments (5)
Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift (1)
125-128: Likable 기반 토글 분기 전환, 방향성 좋습니다
viewit.isLiked로 분기해like/unlike를 호출하는 흐름이 새 모델에 잘 맞습니다.Wable-iOS/Domain/UseCase/Viewit/LikeViewitUseCase.swift (2)
23-24: 도메인 변경 반영 OK
likedViewit.like()호출로 Viewit의 Likable 적용을 올바르게 사용하고 있습니다.
33-34: unlike 처리도 일관성 있게 잘 반영됨
unlikedViewit.unlike()처리, 문제 없습니다.Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift (1)
117-119: cell 바인딩 필드 변경 적절
isLiked/likeCount로의 치환이 새 모델과 일치합니다. 구성 순서도 자연스럽습니다.Wable-iOS/Domain/Entity/Viewit.swift (1)
13-13: Viewit의 Likable 채택 👍도메인 의도와 잘 맞습니다.
| if viewit.isBlind { | ||
| postStatus = .blind | ||
| } else { | ||
| postStatus = .normal | ||
| } |
There was a problem hiding this comment.
여기는 삼항연산자 사용하면 조금 더 직관적일 것 같은데 if-else로 처리하신 이유가 있을까요??
There was a problem hiding this comment.
그러게요? 허허허 수정하겠습니다.
| .map { | ||
| var unlikedViewit = viewit | ||
| unlikedViewit.like.unlike() | ||
| unlikedViewit.unlike() |
There was a problem hiding this comment.
이건 그냥 저도 리팩토링하면서 느꼈던 의문점인데 toggle()로 구현하지 않고 like()와 unlike()를 분리하신 이유가 있을까요?
아무래도 안정성 때문일까요..?
There was a problem hiding this comment.
조금 시간이 지나서 자세히 기억나지 않네요.
금요일에 말씀드리도록 하겠습니다.
👻 PULL REQUEST
📄 작업 내용
✅ 이번 PR에서 이런 부분을 중점적으로 체크해주세요!
잠깐 확인하고 갈까요?
들여쓰기를 5번 이하로 준수했는지, 코드 가독성이 적절한지 확인해주세요.
한 줄당 120자 제한을 준수했는지 확인해주세요.
MARK 주석이 정해진 순서와 형식에 맞게 작성되었는지 확인해주세요.
반복되는 상수 값이 있는지, 있다면 Constant enum으로 분리되어 있는지 확인해주세요.
삼항 연산자가 길어질 경우 적절히 개행되어 있는지 확인해주세요.
조건문에서 중괄호가 올바르게 사용되었는지 확인해주세요.
라이브러리 import가 퍼스트파티와 서드파티로 구분되고 알파벳순으로 정렬되었는지 확인해주세요.
용량이 큰 리소스나 호출되지 않을 가능성이 있는 프로퍼티에 lazy var가 적절히 사용되었는지 확인해주세요.
메모리 누수 방지를 위한 weak 참조가 필요한 곳에 적용되었는지 확인해주세요.
도메인 로직과 UI 로직이 적절히 분리되어 있는지 확인해주세요.
🔗 연결된 이슈
Summary by CodeRabbit