こんにちは、たなたつです。全社横断でアプリ開発の課題解決をしています。
先日開催されたtry! Swift Tokyo 2024のLINEヤフー企業ブースでは、Code Review Challengeを行いました。Code Review Challengeとは、Bad CodeをGood Codeにするための公開コードレビューです。参加者に技術への関心を持ってもらうこと、外部の方々からのレビューにより社員の学びにつなげることを目的に、過去のイベントで行っていた取り組みを今回のイベントでも実施しました。
本記事では、今回出題したCode Review Challengeの5問目の解説をします。
出題コード
import UIKit
import CoreImage
package final class ImageProcessor {
package static let shared = ImageProcessor()
private init() {}
package func applySepiaTone(_ image: UIImage) -> UIImage {
let ciImage = CIImage(image: image)
let filter = CIFilter(name: "CISepiaTone")!
filter.setValue(ciImage, forKey: kCIInputImageKey)
filter.setValue(0.6, forKey: kCIInputIntensityKey)
return filter.outputImage?.uiImage ?? image
}
}
private extension CIImage {
var uiImage: UIImage {
let context = CIContext()
let cgImage = context.createCGImage(self, from: extent)!
return UIImage(cgImage: cgImage)
}
}こちらはCore Imageフレームワークを用いた画像処理を扱うコードです。
短いコードの中に多くの改善点がありますが、注目すべき3点について今回は解説をします。
- シングルトンは可能な限り避ける
- Built-inのCIFilterはタイプセーフなAPIを使う
- CIContextは使い回す
1. シングルトンは可能な限り避ける
アプリのさまざまな箇所から利用される機能を実装する時に、シングルトンは非常に簡単にそれを実現できる手段になります。しかし、使用を避けられる場合はできる限り避けるべきです。
シングルトンには下記の問題がよく発生します。
a. アプリの他のグローバルな状態に依存しやすく、テストしにくくなる
b. Actor等で排他制御が必要になる
a はアプリの状態管理が複雑化するきっかけになるため、品質を保つためのコードレビューの負担も大きくなります。
b に関しては、例えば、下記のコードはSwift 6ではエラーになります。
// .enableUpcomingFeature("GlobalConcurrency")を有効にしてSwift 5.10の環境で実行
final class Model {
static let shared = Model() // Static property 'shared' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor; this is an error in Swift 6
}これを回避するためには、classをactorにしたり、@MainActorやglobal actorでConcurrency safeにしたりする必要があります。つまり、インスタンスが一つしか作れないということは、排他制御について考慮すべき事項が増え、実装や実行時パフォーマンスに影響が出やすくなるということです。
どのように改善するかは場合によりますが、画像処理が必要な箇所が局所的なアプリの場合はシンプルにシングルトンをやめ、ViewModel等の利用箇所でインスタンスを保持するのが良いでしょう。
2. Built-inのCIFilterはタイプセーフなAPIを使う
Built-inのCIFilterのほとんどにはタイプセーフなAPIが用意されており、import CoreImage.CIFilterBuiltinsのようにimportすることで、このように実装できます。
// 古い実装方法
let filter = CIFilter(name: "CISepiaTone")!
filter.setValue(ciImage, forKey: kCIInputImageKey)
filter.setValue(0.6, forKey: kCIInputIntensityKey)
filter.outputImage
// タイプセーフな新しい実装方法
let filter = CIFilter.sepiaTone()
filter.inputImage = ciImage
filter.intensity = 0.6
filter.outputImage昔から存在するAPIに関する情報は古くなっていることがあるので、新しい公式ドキュメントやサンプルコードなども参照しましょう。
3. CIContextは使い回す
CIContextは画像処理のコンテキストを保持するオブジェクトです。この中にはレンダリングに関する多くの情報が含まれており、使い回すほうが効率的な処理が可能です。
例えば、下記の処理をシミュレータ環境で実行した場合、約9倍の差が生まれました。
// 0.044 sec
for _ in 0..<10 {
let context = CIContext()
let filter = CIFilter.sepiaTone()
filter.inputImage = ciImage
filter.intensity = 0.6
print(filter.outputImage)
}
// 0.005 sec
let context = CIContext()
for _ in 0..<10 {
let filter = CIFilter.sepiaTone()
filter.inputImage = ciImage
filter.intensity = 0.6
print(filter.outputImage)
}CIContextやCIImageはimmutableなので、複数スレッドでインスタンスを使い回すこともできますが、CIFilterはmutableのため、排他制御が必要です。Built-inのCIFilterは作り直してもパフォーマンスには大きく影響しないため、排他制御するよりは都度作り直すと良いでしょう。
その他の想定レビュー
詳細な解説は省略しますが、想定していた改善点をいくつか紹介します。
エラーハンドリング
- 条件によってクラッシュする場合は
!を使うのはやめましょう。- サポートしているOSや状況によってクラッシュが発生しない場合に、意図的に使うことは良いです。
??でエラーを隠して、正常ではない処理が実行されてしまう実装はやめましょう。guard等でOptionalをアンラップし、エラーとしてthrowするほうが、ユーザーに対して適切にエラー表示しやすいです。
可読性向上
- 汎用性の高い
CIImageのextensionは別のファイルに記載するほうが良いです。private extensionは同一の処理が複数箇所に実装されてしまう原因にもなるため、汎用性の高いものは別ファイルにinternal extensionで実装するほうが良いことがあります。
CIImageをUIImageとして取得するプロパティは軽い処理ではないため、computed propertyは避けたほうが良いです。- UIKitは
UIImageのためにしかimportしていないため、import class UIKit.UIImageと記述して明確にしても良いです。 ImageProcessorというクラス名はあまりにも汎用的なため、用途を明確にした名前にすると良いです。- 汎用的なクラス名を使うと、アプリのアップデートとともに次第にさまざまな機能が実装されてしまい、クラスが肥大化することになります。
改善例
これらを踏まえて改善した実装はこちらです。
今後もフィルターの種類が増えていくことを想定し、拡張しやすい仕組みも導入してみました。
// ImageFilter.swift
import class UIKit.UIImage
import CoreImage.CIFilterBuiltins
package struct ImageFilter {
private let context = CIContext()
package init() {}
package func apply(_ image: UIImage, with filters: [any ImageFiltering]) throws -> UIImage {
guard var outputImage = CIImage(image: image) else {
throw Error.invalidImage
}
for filter in filters {
outputImage = try filter.apply(outputImage)
}
return try outputImage.renderToUIImage(with: context)
}
}
package extension ImageFilter {
protocol ImageFiltering {
func apply(_ image: CIImage) throws -> CIImage
}
enum Error: Swift.Error {
case invalidImage
case filterError
}
}// ImageFilter+SepiaTone.swift
import CoreImage
package extension ImageFilter {
struct SepiaToneFilter: ImageFiltering {
package func apply(_ image: CIImage) throws -> CIImage {
let filter = CIFilter.sepiaTone()
filter.inputImage = image
filter.intensity = 0.6
guard let outputImage = filter.outputImage else {
throw Error.filterError
}
return outputImage
}
}
}
package extension ImageFilter.ImageFiltering where Self == ImageFilter.SepiaToneFilter {
static var sepiaTone: Self { ImageFilter.SepiaToneFilter() }
}// CIImage+UIKit.swift
import CoreImage
import UIKit
enum CIImageUIKitError: Error {
case failedToCreateCGImage
}
extension CIImage {
func renderToUIImage(with context: CIContext) throws -> UIImage {
guard let cgImage = context.createCGImage(self, from: extent) else {
throw CIImageUIKitError.failedToCreateCGImage
}
return UIImage(cgImage: cgImage)
}
}最後に
多くの方にCode Review Challengeに参加していただいて、とても嬉しかったです。ありがとうございます！
Core ImageフレームワークはiOS 5時代からの歴史のあるフレームワークなので、古い情報で実装してしまったり、シンプルなAPIがゆえにパフォーマンスに影響のある実装をしてしまうケースをよく見かけます。
また、?? などで誤ったエラーハンドリングをし、どこで問題が起きているのかわかりにくくなったり、不要なシングルトンの利用でSwift Concurrency等を使った同時並行処理時に問題が起きやすいコードを実装してしまった経験はないですか？
今回はそれらの啓蒙も含めて、問題を考えてみました。何か少しでも、コード品質の改善につながったら嬉しいです。