Skip to main contentSkip to footer
Search
LINEヤフー Tech Blog

LINEヤフー株式会社のサービスを支える、技術・開発文化を発信しています。

【Code Review Challenge】解説６問目：SDKのコードで気をつけること　 #try! Swift Tokyo 2024

こんにちは。iOSエンジニアの松原です。普段はLINEヤフー社内でログ収集を行うSDKやLINEアプリの開発をしています。

先日開催されたtry! Swift Tokyo 2024のLINEヤフー企業ブースでは、Code Review Challengeを行いました。Code Review Challengeとは、Bad CodeをGood Codeにするための公開コードレビューです。参加者に技術への関心を持ってもらうこと、外部の方々からのレビューにより社員の学びにつなげることを目的に、過去のイベントで行っていた取り組みを今回のイベントでも実施しました。本記事では、Code Review Challengeの6問目の解説をします。

出題したコード

出題したコードは以下です。

// This code is part of the SDK code.
// This code defines a data model and a data manager to handle a unique identifier (UUID) for a user.
// The DataManager class provides functionality to retrieve a stored UUID from UserDefaults, validate it,
// clear it, or generate a new one if none exists or on demand.
 
import Foundation

public struct DataModel {
    let uuid: UUID
}
 
public class DataManager {
    static let shared = DataManager()
    private let userDefaultsKey = "myUniqueIdentifier"
 
    public init() {}
 
    var uniqueIdentifier: DataModel {
        get{
            let defaults = UserDefaults.standard
            let uuidString = defaults.string(forKey: userDefaultsKey)
            var uuid: UUID
            if let validUUIDString = uuidString, let extractedUUID = UUID(uuidString: validUUIDString) {
                uuid = extractedUUID
            } else {
                uuid = UUID()
                defaults.setValue(uuid.uuidString, forKey: userDefaultsKey)
                defaults.synchronize()
            }
            return DataModel(uuid: uuid)
        }
    }
 
    public func clearStoredIdentifier() {
        let defaults = UserDefaults.standard
        defaults.removeObject(forKey: userDefaultsKey)
        defaults.synchronize()
    }
 
    public func validateIdentifier(_ identifier: DataModel) -> Bool {
        let uuidRegex = "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$"
        let uuidString = identifier.uuid.uuidString
        let regex = try! NSRegularExpression(pattern: uuidRegex)
        let range = NSRange(location: 0, length: uuidString.utf16.count)
        return regex.firstMatch(in: uuidString, options: [], range: range) != nil
    }
 
    public func generateNewIdentifier() -> DataModel {
        let newUUID = UUID()
        UserDefaults.standard.setValue(newUUID.uuidString, forKey: userDefaultsKey)
        return DataModel(uuid: newUUID)
    }
}

このコードではUUIDを持ったDataModelと、DataModelの保存やその他の機能をUserDefaultsを利用して提供するSDKを想定しています。

コードを見てすぐに思いつく問題点としては「try!などエラー処理が甘い」「シングルトンのアクセス修飾子が足りずアプリから呼べない」などがあります。

// Bad pattern.
let regex = try! NSRegularExpression(pattern: uuidRegex)
let range = NSRange(location: 0, length: uuidString.utf16.count)
return regex.firstMatch(in: uuidString, options: [], range: range) != nil

// Good pattern.
do {
    let regex = try NSRegularExpression(pattern: uuidRegex)
    let range = NSRange(location: 0, length: uuidString.utf16.count)
    return regex.firstMatch(in: uuidString, options: [], range: range) != nil
} catch {
    print("Invalid regex: \(error.localizedDescription)")
    // Bellow code is better than print for logging.
    // Logger.standard.error("Invalid regex: \(error.localizedDescription)")
    return false
}
// Bad pattern.
static let shared = DataManager()

// Good pattern.
public static let shared = DataManager()

また、仕様に関する問題点として以下の項目が挙げられます。

  • UUIDは生成時に検証が行われるためvalidationIdentifierメソッドは必要ない点
  • iOS12よりUserDefaultsのsynchronize()メソッドはDeprecatedとなっており必要ない点

出題コードの問題点

Code Review Challengeでは先ほど挙げた細かいリファクタリング候補に加え、設計に関する指摘が多く挙げられました。

  • シングルトンは使うべきでない
  • DataManagerクラスはActorを使っても良さそう
  • UserDefaultsをDI(Dependency Injection)できるようにするべき
  • メソッドの命名がわかりにくく、何をするメソッドなのか曖昧
  • SwiftのAPIガイドラインに従うべき

本記事では特に多く寄せられた指摘について解説したいと思います。

Actorを利用する

今回のコードはThreadセーフに作られておらず、複数スレッドから同時に実行されるとデータの不整合が起きる可能性があります。例として、uniqueIdentifierの実行中にclearStoredIdentifierやgenerateNewIdentifierが実行されることで、uniqueIdentifierの戻り値とUserDefaultsの値が不定となる可能性があります。解決策としてDispatchQueueを使いThreadセーフにする方法が考えられますが、ここではActorを使った解決策を解説したいと思います。ActorはSwift5.5から導入された機能でThreadセーフな機能を実装するのに向いています。

今回のコードのDataManagerをActorに準拠させてみます。DataManagerクラスのclassをactorに変更します。

import Foundation

public struct DataModel {
    let uuid: UUID
}

public actor DataManager {
    public let userDefaultsKey = "myUniqueIdentifier"

    public func getUniqueIdentifier() -> DataModel {
        let defaults = UserDefaults.standard
        let uuidString = defaults.string(forKey: userDefaultsKey)
        var uuid: UUID
        if let validUUIDString = uuidString, let extractedUUID = UUID(uuidString: validUUIDString) {
            uuid = extractedUUID
        } else {
            uuid = UUID()
            defaults.setValue(uuid.uuidString, forKey: userDefaultsKey)
        }
        return DataModel(uuid: uuid)
    }

    public func clearStoredIdentifier() {
        let defaults = UserDefaults.standard
        defaults.removeObject(forKey: userDefaultsKey)
    }

    public func generateNewIdentifier() -> DataModel {
        let newUUID = UUID()
        UserDefaults.standard.setValue(newUUID.uuidString, forKey: userDefaultsKey)
        return DataModel(uuid: newUUID)
    }
}

呼び出し元のコードを以下のようにすることでThreadセーフに呼び出すことができます。

await dataManager.clearStoredIdentifier()

DispatchQueueを使うことでもThreadセーフにすることは可能ですが、Actorを用いることでより簡単にThreadセーフにできます。Swift6ではStrict Concurrencyが導入される予定であるため、コンパイル時のデータ競合に対するチェックが厳しくなります。Swift6の対応をしていく上でもSwift Concurrecyのキャッチアップが重要となってきます。

DI(Dependency Injection)できるようにする

今回のコードで利用しているUserDefaultsはstandardを固定しているため、単体テストを実施する際に本番のデータ領域を利用することとなり、本番データを上書きしてしまう危険性があります。そこで以下のようにコードを修正し、外部から利用するUserDefaultsとそのKeyをDIできるようにします。

public class DataManager {
    private let userDefaults: UserDefaults
    private let userDefaultsKey: String

    init(userDefaults: UserDefaults = UserDefaults.standard, userDefaultsKey: String = "myUniqueIdentifier") {
        self.userDefaults = userDefaults
        self.userDefaultsKey = userDefaultsKey
    }

    var uniqueIdentifier: DataModel {
        get {
            let uuidString = userDefaults.string(forKey: userDefaultsKey)
            var uuid: UUID
            if let validUUIDString = uuidString, let extractedUUID = UUID(uuidString: validUUIDString) {
                uuid = extractedUUID
            } else {
                uuid = UUID()
                userDefaults.setValue(uuid.uuidString, forKey: userDefaultsKey)
            }
            return DataModel(uuid: uuid)
        }
    }

    public func clearStoredIdentifier() {
        userDefaults.removeObject(forKey: userDefaultsKey)
    }

    public func generateNewIdentifier() -> DataModel {
        let newUUID = UUID()
        userDefaults.setValue(newUUID.uuidString, forKey: userDefaultsKey)
        return DataModel(uuid: newUUID)
    }
}

通常利用時は以下のようにすることで元のコードと同じ挙動をすることができます。

let dataManager = DataManager()

単体テスト時は以下のようにすることで本番のUserDefaultsを利用することなく動作のテストを実施できます。

let userDefaultsForTest: UserDefaults = UserDefaults(suiteName: "forUnitTest")!
let userDefaultsKey = "myUniqueIdentifier"
let dataManager = DataManager(userDefaults: userDefaultsForTest, userDefaultsKey: userDefaultsKey)

UserDefaults(suiteName:)を利用することでデフォルト以外の名前でデータベース名を利用してUserDefaultsを利用できます。

init(suiteName:)（外部サイト）

おまけ

try! Swift Tokyoの会場で指摘される方がおり大変驚きましたが、もう一点SDK開発者として知っておかないといけない情報としてPrivacy Manifestに関する項目があります。WWDC2023で発表されたPrivacy ManifestではSDKがどのようにデータを利用しているのか、特定のAPIをどのような理由で利用しているのかApp開発者に明示できるようになりました。Required Reason APIに指定されたAPIを利用する場合は利用する理由とともにPrivacy Manifest内に記載する必要があり、記載しない場合APIが利用できなくなるとAppleから示されています。UserDefaultsはこのRequired Reason APIに該当しており、今回の問題で出題したコードを実際にSDKとして利用するためにはPrivacy Manifestに記載する必要があります。今後もこのようなプライバシー施策が出される可能性もあるため、WWDCなどAppleの公式発表を注視していく必要があります。

Describing use of required reason API（外部サイト）

おわりに

今回初めてCode Review Challengeの問題作成を行いましたが、難易度の違う問題をちりばめていきながら1からコードを作成することは大変難しいと思いました。しかし、当日いろいろな方からレビューをいただき、さまざまな観点でコードについて深く考察することができ新たな視点に気づくなど大変有意義な時間となりました。今回得られた知見を業務に活かしつつ、今後もこのような機会があれば問題作成を実施したいと感じました。