박스오피스 앱 [STEP3] Harry, Sajae #51
Conversation
1. 어제 날짜값이 스트링값으로 들어가고 있기때문에 향후 수정 예정 2. URL과 Request 만드는 객체 분리해야함
| import Foundation | ||
|
|
||
| /// 참고: https://medium.com/@vyacheslavansimov/swift-utilities-thread-safe-property-5498afc2eb53 | ||
| @propertyWrapper |
There was a problem hiding this comment.
property Wrapper에 대해서 설명해주세요
해당 SynchronizedLock 기능은 property Wrapper로써만 구현될 수 밖에 없는 기능이였나요?
그렇다면 왜 그렇게 생각하셨나요?
NSLock과 extension으로 구현하신 lock.synchronized에 대해서도 설명해주세요~!
There was a problem hiding this comment.
PropertWrapper란?
프로퍼티 래퍼란 계산속성의 게터세터를wrappedValue 라는 계산속성을 사용하여 캡슐화해둔 것 입니다.
특히 위와같은 직렬접근 로직은 여러번 재사용되기 좋은 로직이라 생각되어 프로퍼티래퍼를 선언하여 재사용률을 높이려고 했습니다.
프로퍼티 래퍼를 쓰지 않는다면 아래와 같이 쓸 수 있습니다.
private var _movies = [BoxOfficeDisplayModel]()
private let moviesLock = NSLock()
var movies: [BoxOfficeDisplayModel] {
get {
moviesLock.lock()
defer { moviesLock.unlock() }
return _movies
}
set {
moviesLock.lock()
defer { moviesLock.unlock() }
_movies = newValue
}
}
// GCD 버전
class BoxOfficeViewController {
private var _movies = [BoxOfficeDisplayModel]()
private let queue = DispatchQueue(label: "test")
var movies: [BoxOfficeDisplayModel] {
get {
// 동기적으로 데이터 읽기
return queue.sync { _movies }
}
set {
// 비동기적으로 데이터 쓰기
queue.sync {
self._movies = newValue
}
}
}
}NSLock vs GCD
- 현재 NSLCock은 개념적으로 뮤텍스의 개념을 사용하고 있습니다. 작업이 스레드를 "점유"하여 해당작업만이 락을 풀 수 있습니다.
- GCD는 NSLock과 같이 스레드를 직접관리해주는 것이 아닌 그저 '어디(글로벌,메인)'큐에, '어떤(직렬,동시)'처리를 할지 정해줍니다.
- NSLock의 경우 GCD보다 좀 더 명확하게 코드가 읽힐 수 있습니다. 또한 현재 경우 한 번에 하나의 스레드만 접근하게끔 설정해주는 것이 목표이기 때문에 세마포(이진세마포는 외부에서 풀 수도 있기 때문에)보다는 뮤텍스가 더 적합하다 생각했고 이를 명시적으로 보여주는 NSLock을 선택했습니다.
| var API_KEY: String { get } | ||
| } | ||
|
|
||
| class BaseEnvironment { |
There was a problem hiding this comment.
상속받아 사용하는 곳이 BaseEnvironment 한곳으로 보여요.
상속으로 설계해주신 이유가 궁금해요. 대면 리뷰에서는 상속을 사용하지 않아도 될 것 같다고 말씀해주셨던 것이 기억나서요!
| var errorDescription: String? { | ||
| switch self { | ||
| case .urlError: | ||
| return NSLocalizedString("please check url,", comment: "") |
There was a problem hiding this comment.
NSLocalizedString에 대해서 설명해주세요
String을 단순히 return하는 것과 어떤 차이가 있나요?
There was a problem hiding this comment.
NSLocalizedString 이란?
NSLocalizedString은 사용자의 현재 지역 설정에 맞는 언어로 문자열이 자동으로 변환됩니다. 지정된 키에 대한 로컬라이즈된 문자열을 반환하며, 앱이 다양한 언어와 지역 설정을 지원할 수 있습니다.
let errorMessage = NSLocalizedString("에러가 발생했습니다,", comment: "어디어디가 문제가 생겼으니 확인해주세요")위 코드의 다국어를 지원할 경우 .strings파일 만들고
"에러가 발생했습니다," = "An error has occurred, please check this and that."
키밸류 쌍으로 넣어주기
앱이 영어 설정에서 실행될 때, NSLocalizedString은 .strings 파일 내에서 해당 키에 대한 값을 찾아서 영어 문자열을 반환합니다.
| @@ -0,0 +1,5 @@ | |||
|
|
|||
| enum APIHostType: String { | |||
|
|
||
| import Foundation | ||
|
|
||
| protocol NetworkManagerProtocol { |
There was a problem hiding this comment.
항상 bring할 때만 사용하는 로직으로 보여지지는 않아요 조금 더 범용적인 표현이 사용되면 좋을 것 같아요
There was a problem hiding this comment.
반영커밋: 795a82a
해당 메서드는 단순히 가져오는 것이 아닌 request를 받아 수행하는 목적이 핵심입니다. 이에따라 request수행하는 의도를 살려 performRequest로 수정해주었습니다.
|
|
||
| import Foundation | ||
|
|
||
| final class NetworkManager: NetworkManagerProtocol { |
There was a problem hiding this comment.
NetworkManger의 역할이 애매하다고 느껴져요.
그 이유는 Repository가 하는 역할 때문이라고 판단이 되는데요. Repository에 대한 코멘트는 아래에 작성해둘게요!
There was a problem hiding this comment.
NetworkManager를 삭제하였고, NetworkManager가 수행하던 기능은 Repostiory로 이관 되었습니다.
수정반영: 636d994
|
|
||
| import Foundation | ||
|
|
||
| final class RequestBuilder: RequestBuilderProtocol { |
There was a problem hiding this comment.
해당 타입은 Builder라는 이름으로 설계해주신 이유가 있을까요?
Builder Pattern에 대해서 자세히 설명해주세요
There was a problem hiding this comment.
RequestBuilder는 URLBuilder와 마찬가지로 Builder pattern보다는 호출단의 설정을 통해서 request의 설정값을 가져오는 구조로 변경하였습니다.
수정반영: 636d994
|
|
||
| import Foundation | ||
|
|
||
| protocol JsonDecodeProtocol { |
There was a problem hiding this comment.
Decode에 대한 범용성을 갖춘 protocol을 만들기위해 추상화를 해주신 것 같아요.
하지만 protocol의 네이밍이 범용성을 떨어뜨리는 것 같아요. 좀 더 범용적인 네이밍을 사용해보는 건 어떨까요?
There was a problem hiding this comment.
반영커밋: 795a82a
Swift에는 JSON디코더 뿐만 아니라 PropertyListDecoder, XMLDecoder 혹은 커스텀 디코더등 여러 종류가 있을 수 있습니다. 이를 모두 포함시켜주기 위해 DecoderProtocol로 네이밍을 수정했습니다
| return .success(NetworkResponse(response: httpResponse, data: data)) | ||
| } | ||
|
|
||
| private func printNetworkError(_ error: NetworkError) { |
There was a problem hiding this comment.
에러처리 목적에는 크게 개발자에게 알려주는 목적과 사용자에게 알려주는 목적이 있다 생각합니다.
현재 세분화된 네트워크 에러가 레이어를 넘어갈 때 도메인 에러로 맵핑되면서 에러의 범위가 뭉툭해지고 있습니다.
레포지토리에서 맵핑이 되기전 개발자에게 세분화된 에러를 로그로 알려주기 위해 printNetworkError 함수를 넣어 사용했습니다. 하지만 세션프로바이더에서는 사용하지 않음에도 불필요하게 들어가 삭제했습니다
|
|
||
| import Foundation | ||
|
|
||
| protocol EndPointMakable { |
There was a problem hiding this comment.
EndPoint 란?
-
커뮤니케이션 채널의 끝 지점을 의미한다. 여기서 얘기하고 있는 EndPoint는 API 를 통한 커뮤니케이션의 끝지점으로 URL을 의미한다.
-
프로그램을 구현할 때 URLComponents를 통해서 scheme, host, path, queryItems를 차례대로 조합해서 접근할 URL을 편리하게 만들어주는 방법으로 활용할 수 있다.
-
URL에 담을 수 있는 정보는 scheme, host, path, queryItems 들이 있으므로 해당 정보들을 EndPoint 타입을 통해 url로 조립하여 만들어주고 이외의 정보들 httpMethod, header 등은 EndPoint를 통해 URL을 만든 뒤 이를 활용하여 URLRequest 만들어서 추가적인 정보를 담을 수 있다.
|
|
||
| switch result { | ||
| case .success(let boxOfficeDTO): | ||
| return .success(boxOfficeDTO.boxOfficeResult.dailyBoxOfficeList.map { $0.toEntity() }) |
There was a problem hiding this comment.
Repository Pattern 이란 무엇인가요?
Repository를 설계함에 잇어서 가장 중요하게 생각해야하는 점은 무엇인가요?
NetworkManager의 역할 및 toEntity 메서드의 사용과 함께 생각해서 답변해주시면 좋을 것 같아요!
There was a problem hiding this comment.
Repository Pattern은 보통 비즈니스 로직에서 데이터 베이스나 웹서비스 등의 데이터 저장소에 접근하게 되는데 이 과정에서 여러 문제가 발생할 수 있습니다. 주로 중복되는 코드, 오류를 발생할 가능성이 있는 코드, 오타, 비즈니스 테스트 어려움 등이 있습니다.
Repository는 데이터 소스 레이어와 비즈니스 레이어 사이를 중재합니다.
(1) 레포지터리는 데이터가 있는 여러 저장소(Local Data Source, Remote Data Source)를 추상화하여 중앙 집중 처리 방식을 구현할 수 있습니다.
(2) 데이터를 사용하는 Domain에서는 비즈니스 로직에만 집중할 수 있습니다.
예를 들어, ViewModel에서는 데이터가 로컬 DB에서 오는지, 서버에서 API 응답을 통해 오는 것인지 출처를 몰라도 됩니다. Repository를 참조하여 Repository가 제공해주는 데이터를 이용하기만 하면 됩니다.
따라서 저희 설계에서는 말씀해주신 것 처럼 Repository에서 도메인 단의 종속적인 기능을 처리하게 된다면, 비즈니스 로직의 니즈에 따라서 Repository 로직 또한 수정되어야 했었고, 이를 조금 더 Repository를 재사용하고 범용적으로 사용하기 위해서 NetworkManager, Repository, Usecase의 역할로 책임을 재정의하였습니다.
수정반영: 636d994
|
|
||
| import Foundation | ||
|
|
||
| /// 엔티티 값은 임시로 넣음 |
|
|
||
| import Foundation | ||
|
|
||
| final class BoxOfficeUseCase: BoxOfficeUseCaseProtocol { |
There was a problem hiding this comment.
현재 UseCase가 하고 있는 역할이 따로 없는 것 같아요.
UseCase는 여기서 왜필요하고 어떤 목적을 가지고 있나요?
Repository의 역할과 연관하여 생각해주시면 좋을 것 같아요
There was a problem hiding this comment.
Usecase는 사용자가 원하는 기능을 실질적으로 처리해주는 부분으로, 은행으로 치면 사용자가 대출을 원할 때 대출을 실행주는 기능들이 들어가는 곳입니다. 현재 저희 과제는 박스오피스로 사용자는 영화 정보를 원하며, 이에 영화 정보 가져오는 로직들이 Usecase에 들어가야 한다고 생각합니다.
또한 Repository와 Usecase를 리팩토링 하면서 Repository의 재사용성 확보를 위해
Usecase로 데이터를 전달하는 Mapper를 설계 변경하였습니다.
수정: a38b6e0
|
|
||
| import UIKit | ||
|
|
||
| extension UIViewController { |
There was a problem hiding this comment.
extension으로 구현해주신 의미와 목적이 궁금해요! 설명부탁드릴게요
There was a problem hiding this comment.
- UIAlertController는 UIViewController의 하위클래스로 반드시 뷰컨에서 호출해야합니다.
- 알럿창은 뷰컨에서 반복적으로 쓰이고 각각의 뷰컨에서 알럿창을 구현하면 코드가 중복되기 때문에 확장을 통해 alert창 코드 중복을 낮추었습니다.
- 따로 객체를 만들어쓰면 객체를 생성하고 써야합니다. 하지만 어차피 알럿창은 뷰컨에서 호출되기 때문에 확상을 통해 구현하면 객체생성과정없이 곧바로 사용할 수 있어 편리하다 생각했습니다.
|
|
||
| // 커스텀 뷰 설정 | ||
| func setupBoxOfficeView() { | ||
| boxOfficeCollectionView = BoxOfficeCollectionView(frame: .zero) |
There was a problem hiding this comment.
해당 ViewController는 불필요한 최상위 view를 가지고 있는 것으로 보여요
LoadView에 대해서 학습해보시고
개선할 수 있는 방향성에 대해 설명해주세요
There was a problem hiding this comment.
loadView는 컨트롤러의 뷰가 처음으로 요청될 때 호출되며, 여기서 컨트롤러의 메인 뷰를 직접 생성하거나 설정할 수 있습니다. 뷰컨의 뷰에 직접 할당하고 이를 LoadView()에 호출함으로써 불필요한 제약코드를 삭제했습니다!
| } | ||
| } | ||
|
|
||
| @MainActor |
There was a problem hiding this comment.
MainActor에 대해서 설명해주세요 해당 키워드를 사용하지 않는다면
다르게 MainActor를 사용하는 방식은 어떤게 있나요?
There was a problem hiding this comment.
The main actor is a global actor that describes the main thread:
@globalActor
public actor MainActor {
public static let shared = MainActor(...)
}MainActor is a new attribute introduced in Swift 5.5 as a global actor providing an executor which performs its tasks on the main thread. When building apps, it’s essential to perform UI updating tasks on the main thread, which can sometimes be challenging when using several background threads. Using the @MainActor attribute will help ensure your UI is always updated on the main thread.
해당 키워드는 async-await와 함께 UI가 메인스레드에서 업데이트 되는 것을 보장하기 위해서 사용합니다. 정의를 보면 기본적으로 싱글톤으로 구현되어 있고 Actor로서 Data Race를 방지할 수 있습니다.
따라서 해당 키워드를 사용하지 않는다면 같은 역할을 해주는, 즉 메인스레드에서 비동기 동작을 보장하면서 Data Race를 해결하는 다른 GCD(세마폴 ...), OperationQueue 등 방식으로 처리를 할 수 있습니다.
참고 자료:
There was a problem hiding this comment.
- 메인 엑터는 작업을 메인스레드를 보냄으로써 레이스컨디션없이 UI업데이트를 할 수 있도록 도와줍니다.
- MainActor는 Swift Concurrency로 비교적 최근 문법입니다. 이전에 메인스레드로 UI작업을 돌리는 방법으로는 GCD와 이를 추상화한 opreation Queue를 사용하는 방법이 있습니다
DispatchQueue.main.async {
// UI 업데이트 코드
}OperationQueue.main.addOperation {
// UI 업데이트 코드
}- GCD 의 한계
- GCD 블록 내 코드는 일단 한 번 실행되면 취소할수가 없습니다. (작업 수행전에는 취소가능)
- 단순한 비동기작업은 GCD로 메인스레드로 작업을 돌리는데 적합하나 중간에 작업취소와 같은 상태관리에 있어서 부적합합니다
| var initialSnapshot = NSDiffableDataSourceSnapshot<Section, BoxOfficeDisplayModel>() | ||
| initialSnapshot.appendSections([.main]) | ||
| initialSnapshot.appendItems(loadingPlaceholder, toSection: .main) | ||
| dataSource.apply(initialSnapshot, animatingDifferences: false) |
There was a problem hiding this comment.
애플 문서에서 관련 내용을 찾아본 결과 You can safely call this method from a background queue, but you must do so consistently in your app. Always call this method exclusively from the main queue or from a background queue.
apply()는 main queue와 background queue 에서 모두 안전하게 사용할 수 있는데, 다만 main queue에서 사용하면 main queue에서만 사용하고 background queue에서 사용되면 background queue에서만 사용하라고 명시되어 있습니다.
따라서 현재 저희는 handleFetchResult()를 비동기로 백그라운드에서 처리하고 있으므로 내부에서 호출되는 apply() 또한 애플 문서 가이드에 따라서 background queue에서 동작하게 코드를 수정하였습니다.
수정반영: 05b38f2
참고문서:
https://developer.apple.com/documentation/uikit/uicollectionviewdiffabledatasource/3375795-apply
| $0.spacing = 0 | ||
| } | ||
|
|
||
| leftStackView.addArrangedSubview(rankLabel) |
There was a problem hiding this comment.
가독성이 조금 떨어져보여요 반복문으로 표현되면 더 읽기 편할 것 같아요
| ]) | ||
| } | ||
|
|
||
| func matchRankIntensity(of isNew: Bool, with rankNumber: Int) { |
There was a problem hiding this comment.
overloading과 overriding의 차이점에 대해서 알려주세요
해당 부분은 반드시 overloading을 사용했을 때 장점이 있는 부분이였나요?
There was a problem hiding this comment.
해당 함수명은 오버로딩을 의도했다기 보다는, 어울리는 함수명을 작성하다보니 똑같은 함수명으로 오버로딩이 되었습니다. 딱히 이점이 없어서 다른 이름으로 변경하겠습니다.
-
overriding
A subclass can provide its own custom implementation of an instance method, type method, instance property, type property, or subscript that it would otherwise inherit from a superclass. This is known as overriding. -
overloading
함수의 파라미터가 다르면 같은 이름으로 함수를 정의해서 사용할 수 있는 것
참고자료:
|
|
||
| import UIKit | ||
|
|
||
| final class BoxOfficeCollectionView: UICollectionView { |
There was a problem hiding this comment.
해당 타입이 따로 분리된 이유가 궁금해요!
CollectionView를 가지고 있는 타입에 createLayout이 정의 되었다면 불필요하게 static이 사용될 필요가 없어보여요
There was a problem hiding this comment.
초기화 함수에서 static이 아닌 메서드 호출이 안되어 static을 넣어준 후 호출한뒤 레이아웃 속성을 넣어주었습니다
컬렉션부 레이아웃을 클로저 선언을 통해 불필요한 static을 없애주었습니다!
안녕하세요 시온 @LeeZion94,
박스오피스 앱 STEP3 PR 송부 드립니다.
STEP3 구현 사항 외에 STEP2 통신 구현부 일부를 리팩터링 하였습니다. 리뷰에 참고 부탁드리며 감사합니다.
🔍 What is this PR?
기본과제
1. 네트워크 통신 구현
Step 2에서 구현한 네트워킹 기능을 통해 실제로 상품목록을 API 서버에 요청하여 불러옵니다.
2. ModernCollectionView를 활용한 박스오피스 화면 구현
어제의 박스오피스를 볼 수 있는 화면을 구현합니다.
3. ModernCollectionView 새로고침 구현
리스트를 아래로 잡아끌어서 놓으면 리스트를 새로고침합니다 (당겨서 새로고침)
4. ModernCollectionView 데이터 로딩 화면 구현
처음 목록을 로드할 때, 사용자에게 빈 화면만 보여주는 대신, 로드 중임을 알 수 있게 해주세요.
5. UI 구현
-- 화면 상단에는 날짜를 표기
-- 리스트 형태로 박스오피스 정보 표기
-- 박스오피스 정보의 각 열에 표기할 필수정보
-- 맨 왼쪽에는 영화의 현재 등수를 표기
-- 신규 영화면 등수 아래에 신작이라고 표기
-- 기존 영화면 어제와 비교한 등락을 표기
-- 순위 상승 : 빨간 화살표 + 등락 편차
-- 순위 하락 : 파란 화살표 + 등락 편차
-- 변동 없음 : - 표기
-- 해당 일자의 관객수와 누적 관객수를 표기
-- 숫자가 세 자리 이상 넘어가면 ,를 활용하여 읽기 쉽도록 합니다. 예) 10,000
핵심경험
-- Safe Area을 고려한 오토 레이아웃 구현
-- Collection View의 활용
-- Mordern Collection View 활용
🗂️ 프로젝트 구조
📝 PR Point
1. 데이터 레이어 리팩토링
Repository에서session,URLBuilder,decoder를 주입받아서 쓰기보다NetworkManager를 만들어 레포지토리객체는NetworkManager와URLBuilder두 객체를 통해 데이터를 받아왔습니다2. ModerCollectionView 구현
ColletionView셀 높이 구현에 있어서fractional이나absolute를 넣어 높이를 고정시키보다estimated를 활용해서 셀의 내용에 따라 동적으로 높이를 설정했습니다.3. 새로고침 구현, 로딩 화면 구현
UIRefreshControl을 사용하여 새로고침을 구현해주었습니다.4.
@MainActor사용DispatchQueue.main.async { ... }를 사용해주면 기본적으로 들여쓰기가 되기에 가독성이 떨어진다 판단했습니다await, 메인스레드 작업은@MainActor를 사용하여 가독성을 높일 수 있었습니다.5. RaceCondition 방지를 위한 Property Wrapper 선언
moive데이터는 사용자가 빠르게 연속해서 새로고침 실행과 동시에 서버에서 데이터가 바뀔 경우 raceCondition이 발생할 수 있습니다.lock을 걸고 클로저를 통해 작업 실행후defer문을 통해 락을 풀어주는 코드를 추가했습니다.📸 Screenshot