- Overview
PLAIN 앱의 전시화면은 이렇게 생겼습니다. (데이터는 가짜입니다.)
* mock data ref
동작 방식은 다음과 같습니다.
1. 서버에서 전시 관련 데이터를 받아온다.
2. 전시 이미지에서 dominant color를 추출한 이후 배경색을 결정한다. (셀 뒤 배경에 활용)
3. 처리된 새로운 전시 데이터를 뷰에 반영한다.
- 기존 코드
ViewModel
import UIKit
import Combine
import ColorThiefSwift
import Kingfisher
private var exhibitions = [ExhibitionModel]() // 원본 전시 데이터
@Published var processedExhibitions = [ProcessedExhibition]() // 후처리 된 전시 객체 배열
@MainActor
private func processExhibitions() async {
var processedExhibitions: [ProcessedExhibition] = []
for e in exhibitions {
guard let posterUrl = e.posterImageUrl,
let url = URL(string: posterUrl) else { return }
let imageView = UIImageView()
let processedExhibition: ProcessedExhibition? = await withCheckedContinuation { continuation in
imageView.kf.setImage(with: url, completionHandler: { _ in
if let dominant = ColorThief.getColor(from: imageView.image ?? UIImage())?.makeUIColor() {
let hueValue = dominant.getHsb().0
let hsbColor = UIColor(hue: hueValue / 360, saturation: 0.08, brightness: 0.95, alpha: 1.0)
continuation.resume(returning: ProcessedExhibition(id: Int(e.id), imageView: imageView, description: e.description, likesCount: e.likesCount, commentCount: e.commentCount, likes: e.likes, backgroundColor: hsbColor))
} else {
continuation.resume(returning: nil)
}
})
}
if let processedExhibition {
processedExhibitions.append(processedExhibition)
}
}
self.processedExhibitions = processedExhibitions
}
ViewController
override func setupBindings() {
viewModel.feedDataSubject
.receive(on: DispatchQueue.main)
.sink { [weak self] (items, scrollToTop) in
guard let self else { return }
self.dataSource.apply(items)
}
.store(in: &subscriptions)
}
앞서 말한 과정을 순서대로 정학하게 처리하고 있는 코드입니다.
구현 완성도 관점에서는 문제가 없지만, 재사용, 유지보수 측면에서는 1점도 아까운 코드입니다.
1. 가독성
들여쓰기가 많아 읽기가 불편합니다. 제가 아닌 다른 사람이 이 코드를 본다면 '뭐 하는 코드지? 왜 갑자기 hsbColor를 만들지?'라는 생각을 하게 될 것입니다. 이는 hsb에 대해 검색을 해본다거나, 저를 찾아온다거나, 제가 하나하나 설명드린다거나 하는 등의 추가적인 작업을 필요로합니다.
즉, 협업 효율성을 저하시키는 코드입니다.
2. SRP
원본 데이터의 이미지를 처리하는 것은 뷰모델의 역할에 어긋납니다. (SRP 위반)
VC에서 subscription을 할 때 처리하거나 새로운 클래스를 만들어서 역할을 분담하는 것이 적절합니다.
3. 성능
이미지를 "순서대로" 받아오고 순서대로 배열에 넣어서 뷰를 업데이트하는 방식은 당연하게도 성능이 좋지 않습니다. 더군다나 제가 작성한 코드는 메인 스레드에서 이루어집니다. 즉, "메인스레드에서 이미지를 순서대로 다운로드"하는 것은 매우 비효율적인 방식입니다. 이미지 다운로드는 여러 스레드에서 수행하되, 모든 이미지가 다운로드 되었을 때 배경 색상을 계산해서 보여주는 방식으로 개선해야 합니다.
이 외에도 부적절한 함수/변수명, 부적절한 역할 부담, Kingfisher 라이브러리를 써서 굳이 imageView에 이미지를 넣는다거나, 후처리된 데이터 모델을 감추지 않고 그대로 publishing한다거나 등등 개발 초기에 짰던 어지러운 코드를 개선할 예정입니다.
- DominantColor 추출
DominantColor를 추출하는 코드는 새로운 클래스를 만드는 것보다 UIImage를 extension하는 것이 재활용성도 더 높고 활용하기 쉬울 것입니다.
import UIKit
import ColorThiefSwift
extension UIImage {
func dominantColor() -> UIColor {
guard let dominantColor = ColorThief.getColor(from: self)?.makeUIColor() else { return UIColor() }
return dominantColor
}
}
만약 ColorThiefSwift 라이브러리를 쓸 수 없는 상황이 오더라도, 해당 코드만 수정하면 되기 때문에 유지보수에 유리합니다.
- 배경색을 결정하는 로직
배경색은 hsb+alpha에서 hue값과 alpha값은 고정하고 saturation 값을 0.08, brightness 값을 0.95로 수정해서 결정합니다.
기존 하드코딩된 부분을 클래스로 빼놓기에는 너무 국소적인 로직(재사용 가능성이 없어보이는 로직)이라는 생각이 들어서 private func으로 빼놓겠습니다.
private func getExhibitionBackgroundcolor(dominantColor: UIColor) -> UIColor {
let hue = dominantColor.getHsb().0
return UIColor(hue: hue / 360, saturation: 0.08, brightness: 0.95, alpha: 1.0)
}
- @Published 대신 PassthroughSubject 활용
@Published의 사용은 캡슐화를 어렵게 만듭니다. 물론, 현실적으로 모든 프로퍼티가 private으로 선언되기는 어렵지만, 해당 데이터는 캡슐화하기에 충분히 중요한 데이터라고 생각하기에 감추는 것이 좋아보입니다.
또한, 값이 변경되면 자동으로 publishing을 하는 @Published의 publishing 시점을 결정하기 위해 빈 배열에 원소를 하나씩 넣었다가 마지막에 한번에 할당했는데, PassthroughSubject 를 활용하면 이럴 필요가 없어집니다.
let processExhibitionSubject = PassthroughSubject<[ProcessedExhibition], Never>()
@MainActor
private func processExhibitions() async {
var processedExhibitions: [ProcessedExhibition] = []
for e in exhibitions {
// ... 로직 ...
}
self.processedExhibitions = processedExhibitions
processExhibitionSubject.send(processedExhibitions)
}viewModel.processExhibitionSubject
.receive(on: DispatchQueue.main)
.sink(receiveValue: { [weak self] processedExhibitions in
guard let self else { return }
self.exhibitionMainDataSource.apply(processedExhibitions)
self.pageControl.numberOfPages = processedExhibitions.count
})
.store(in: &cancellables)
- 중간 점검
Dominant Color 추출, 배경색 계산, @Published→PassthroughSubject 이후 메인 로직의 코드는 다음과 같습니다.
@MainActor
private func processExhibitions() async {
var processedExhibitions: [ProcessedExhibition] = []
for e in exhibitions {
guard let posterUrl = e.posterImageUrl,
let url = URL(string: posterUrl) else { return }
let imageView = UIImageView()
let processedExhibition: ProcessedExhibition? = await withCheckedContinuation { continuation in
imageView.kf.setImage(with: url, completionHandler: { _ in
if let dominantColor = imageView.image?.dominantColor() {
let backgroundColor = self.getExhibitionBackgroundcolor(dominantColor: dominantColor)
continuation.resume(returning: ProcessedExhibition(id: Int(e.id), imageView: imageView, description: e.description, likesCount: e.likesCount, commentCount: e.commentCount, likes: e.likes, backgroundColor: backgroundColor))
} else {
continuation.resume(returning: nil)
}
})
}
if let processedExhibition {
processedExhibitions.append(processedExhibition)
}
}
self.processedExhibitions = processedExhibitions
processExhibitionSubject.send(processedExhibitions)
}
- Kingfisher setImage→retrieveImage
개발 초기에는 Kinfisher에서 이미지를 다운로드하는 함수가 뭔지 몰랐어서 일단 이미지뷰에 이미지를 넣고, 그 이미지를 활용하는 방식으로 구현했습니다. 지금은 이미지를 받아오는 함수를 알기 때문에 이를 활용해서 개선해보겠습니다.
참고로, 이미지 다운 방법도 모른 채로 Kingfisher를 사용한 것은, 알아서 캐싱을 해주기 때문에 이부분에서 편의를 취하고자 그렇게 했습니다 ..
@MainActor
private func processExhibitions() async {
var processedExhibitions: [ProcessedExhibition] = []
for e in exhibitions {
guard let posterUrl = e.posterImageUrl, let url = URL(string: posterUrl) else { return }
let processedExhibition: ProcessedExhibition? = await withCheckedContinuation { continuation in
KingfisherManager.shared.retrieveImage(with: url) { result in
switch result {
case .success(let data):
let dominantColor = data.image.dominantColor()
let backgroundColor = self.getExhibitionBackgroundcolor(dominantColor: dominantColor)
continuation.resume(returning: ProcessedExhibition(id: Int(e.id), image: data.image, description: e.description, likesCount: e.likesCount, commentCount: e.commentCount, likes: e.likes, backgroundColor: backgroundColor))
case .failure(let error):
print(error.localizedDescription)
continuation.resume(returning: nil)
}
}
}
if let processedExhibition {
processedExhibitions.append(processedExhibition)
}
}
self.processedExhibitions = processedExhibitions
processExhibitionSubject.send(processedExhibitions)
}
- 이미지 다운로드 코드 함수화
Kingfisher관련 코드가 보기 불편하니 async 함수로 만들어서 따로 빼주도록 하겠습니다.
@MainActor
private func processExhibitions() async throws {
var processedExhibitions: [ProcessedExhibition] = []
for e in exhibitions {
guard let posterUrl = e.posterImageUrl, let url = URL(string: posterUrl) else { return }
let posterImage = try await getExhibitionPosterImage(with: url)
let dominantColor = posterImage.dominantColor()
let backgroundColor = getExhibitionBackgroundcolor(dominantColor: dominantColor)
let processedExhibition = ProcessedExhibition(id: Int(e.id), image: posterImage, description: e.description, likesCount: e.likesCount, commentCount: e.commentCount, likes: e.likes, backgroundColor: backgroundColor)
processedExhibitions.append(processedExhibition)
}
self.processedExhibitions = processedExhibitions
processExhibitionSubject.send(processedExhibitions)
}
private func getExhibitionPosterImage(with url: URL) async throws -> UIImage {
return try await withCheckedThrowingContinuation { continuation in
KingfisherManager.shared.retrieveImage(with: url) { result in
switch result {
case .success(let data):
continuation.resume(returning: data.image)
case .failure(let error):
continuation.resume(throwing: error)
}
}
}
}
많이 깔끔해졌네요
- 성능 개선
앞서 말했던 것처럼 이미지를 받아오고 처리하고 받아오고 처리하고 .. 이런 방식은 최선의 방법이 아닙니다. 이미지를 동시에 받아오고 전부 받아오면 이미지를 처리할 수 있도록 Task Group을 사용해 개선할 것입니다.
private func processExhibitions() async throws {
var processedExhibitions: [ProcessedExhibition?] = Array(repeating: nil, count: exhibitions.count)
try await withThrowingTaskGroup(of: (Int, ProcessedExhibition?).self, body: { [weak self] taskGroup in
guard let self else { return }
for (idx, exhibition) in exhibitions.enumerated() {
if let posterUrl = exhibition.posterImageUrl, let url = URL(string: posterUrl) {
taskGroup.addTask {
let posterImage = await self.getExhibitionPosterImage(with: url)
guard let posterImage else { return (idx, nil) }
let dominantColor = posterImage.dominantColor()
let backgroundColor = self.getExhibitionBackgroundcolor(dominantColor: dominantColor)
let processedExhibition = ProcessedExhibition(id: Int(exhibition.id), image: posterImage, description: exhibition.description, likesCount: exhibition.likesCount, commentCount: exhibition.commentCount, likes: exhibition.likes, backgroundColor: backgroundColor)
return (idx, processedExhibition)
}
}
}
for try await task in taskGroup {
processedExhibitions[task.0] = task.1
}
})
self.processedExhibitions = processedExhibitions.compactMap{$0}
processExhibitionSubject.send(self.processedExhibitions)
}
private func getExhibitionPosterImage(with url: URL) async -> UIImage? {
return await withCheckedContinuation { continuation in
KingfisherManager.shared.retrieveImage(with: url) { result in
switch result {
case .success(let data):
continuation.resume(returning: data.image)
case .failure:
continuation.resume(returning: nil)
}
}
}
}
이렇게 TaskGroup을 사용해서 동시에 이미지를 받아오고 완료되는 대로 processedExhibitions에 넣어, 전부 완료가 되면 뷰를 업데이트하는 방식으로 성능을 개선했습니다.
'iOS > Refactoring' 카테고리의 다른 글
| ExhibitionMain의 processExhibitions() 함수 리팩토링 (1) | 2024.09.24 |
|---|---|
| MyPageViewModel getAllItems() 함수 개선하기 (0) | 2024.08.10 |
| Domain + Presentation + MVVM 아키텍처 개선 (0) | 2024.08.09 |
| 홈 화면 UX 개선하기 (리사이징, 메모리 캐싱) (0) | 2024.07.27 |
| MyPageProfileEditViewModel 코드 개선하기 (2) (0) | 2024.07.17 |