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

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

【Code Review Challenge】解説3問目：実行タイミングには気をつけて #try! Swift Tokyo 2024

こんにちは。Yahoo!カーナビでiOSアプリ開発を担当しているhasekenです。

先日開催されたtry! Swift Tokyo 2024のLINEヤフー企業ブースでは、Code Review Challengeを行いました。

Code Review Challengeとは、Bad CodeをGood Codeにするための公開コードレビューです。

参加者に技術への関心を持ってもらうこと、外部の方々からのレビューにより社員の学びにつなげることを目的に、過去のイベントで行っていた取り組みを今回のイベントでも実施しました。

本記事では、Code Review Challengeの3問目の解説をします。

出題コード

import UIKit
import StoreKit

// Check if there is a promotional app at the launch of the application by querying the server.
// If there is a promotional app, display PromotionStoreViewController modally.
final class AppTopViewController: UIViewController {
    override func viewDidLoad() {
        super.viewDidLoad()

        Task {
            await downloadPromotionAppInfo(url: URL(string: "https://example.com/promotionapp/info")!)
        }
    }

    private func downloadPromotionAppInfo(url: URL) async {
        guard let (data, response) = try? await URLSession.shared.data(from: url) else { return }

        guard let httpResponse = response as? HTTPURLResponse, httpResponse.statusCode == 200 else {
            print("Invalid response")
            return
        }

        // json = ["appId": AppID of the promotional app (String),
        //         "appTitle": Name of the promotional app (String),
        //         "appIconImage": Image data of the promotional app (Data)]
        if let jsonObject = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
            let viewController = PromotionStoreViewController()
            viewController.appId = jsonObject["appId"] as? String
            viewController.appTitleLabel.text = jsonObject["appTitle"] as? String
            if let imageData = jsonObject["appIconImage"] as? Data {
                viewController.appIconImageView.image = UIImage(data: imageData)
            }
            self.present(viewController, animated: true)
        }
    }

    // The rest of the code is omitted
}

final class PromotionStoreViewController: UIViewController {
    var appId: String?
    @IBOutlet var appTitleLabel: UILabel!
    @IBOutlet var appIconImageView: UIImageView!
    @IBOutlet var showAppStoreButton: UIButton!

    @IBAction private func didTapPresentStoreViewButton(sender: Any) {
        let storeViewController = SKStoreProductViewController()
        let parameters: [String: Any] = [SKStoreProductParameterITunesItemIdentifier: appId]
        storeViewController.loadProduct(withParameters: parameters) { status, error in
            if status {
                self.present(storeViewController, animated: true)
            } else {
                print(error!.localizedDescription)
            }
        }
    }
}

こちらのコードは、プロモーションしたいアプリをユーザーに紹介するアプリのコードです。

本アプリを起動した時に、プロモーションしたいアプリがあるかをdownloadPromotionAppInfoにてサーバーに問い合わせ、もしある場合はPromotionStoreViewControllerを表示するものです。

PromotionStoreViewControllerでは、StoreKitのSKStoreProductViewControllerを表示します。

このコードではいくつかの問題を仕込んでみました。

強制アンラップを行う、Taskが保持されず通信がキャンセルできないなどの問題もありますが、その中でも3点ほどUIKitに関する以下の3点をこの記事では説明をしたいと思います。

  • UIの変更はMain Threadで行う
  • viewDidLoad実行前にIBOutletにアクセスしてしまうとクラッシュする
  • viewDidAppear実行前にpresentすると画面が表示されない

1.UIの変更はMain Threadで行う

UIの変更を行う場合、Main Threadで行うことが推奨されています。

（UIViewControllerもMainActorになっていたりします）

@MainActor
class UIViewController : UIResponder

https://developer.apple.com/documentation/uikit/uiviewcontroller （外部サイト）

本コードでは、クロージャー内で表示を実行する場合があります。

final class AppTopViewController: UIViewController {
     ...
    private func downloadPromotionAppInfo(url: URL) async {
        ... 
        if let jsonObject = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
            let viewController = PromotionStoreViewController()
            viewController.appId = jsonObject["appId"] as? String
            viewController.appTitleLabel.text = jsonObject["appTitle"] as? String
            if let imageData = jsonObject["appIconImage"] as? Data {
                viewController.appIconImageView.image = UIImage(data: imageData)
            }
            self.present(viewController, animated: true)
        }
        ...
}

...

これらを処理する場合、Main Threadで処理すると以下となります。

// downloadPromotionAppInfo(url:)に@MainActorをつける
@MainActor
private func downloadPromotionAppInfo(url: URL) async {
    ...
}

// DispatchQueue.main.asyncでpresentを実行する
private func downloadPromotionAppInfo(url: URL) async {
    ...
    if let jsonObject = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
        let viewController = PromotionStoreViewController()
        ...
        DispatchQueue.main.async {
            self?.present(viewController, animated: true)
        }
    }             
}

2.viewDidLoad実行前にIBOutletにアクセスしてしまうとクラッシュする

サーバー通信完了後、PromotionStoreViewControllerを表示しますが、その前にPromotionStoreViewControllerが保持するIBOutletにアクセスしています。

final class AppTopViewController: UIViewController {
     ...
    private func downloadPromotionAppInfo(url: URL) async {
        ... 
        if let jsonObject = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
           ...
            viewController.appTitleLabel.text = jsonObject["appTitle"] as? String
            if let imageData = jsonObject["appIconImage"] as? Data {
                viewController.appIconImageView.image = UIImage(data: imageData)
            }
            self.present(viewController, animated: true)
        }
        ...
}

final class PromotionStoreViewController: UIViewController {
    ...
    @IBOutlet var appTitleLabel: UILabel!
    @IBOutlet var appIconImageView: UIImageView!
    ...
}

ただし、IBOutletはviewDidLoadが終わるまで生成されずnilとなります。

viewDidLoadはpushViewControllerかpresentされた時に呼ばれますが、今回IBOutletは強制アンラップで記述されているため、今回viewDidLoad前にIBOutletにアクセスしてしまい、アプリがクラッシュします。

この問題に対応すると、例えば以下となります。

// IBOutletをprivateにし、viewDidLoadでパラメータを設定する。
final class AppTopViewController: UIViewController {
     ...
    private func downloadPromotionAppInfo(url: URL) async {
        ... 
        if let jsonObject = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
           ...
            viewController.appTitle = jsonObject["appTitle"] as? String
            if let imageData = jsonObject["appIconImage"] as? Data {
                viewController.appIconImage= UIImage(data: imageData)
            }
            self.present(viewController, animated: true)
        }
        ...
}

final class PromotionStoreViewController: UIViewController {
    ...
    var appTitle: String?
    var appIconImage: UIImage?
    @IBOutlet private var appTitleLabel: UILabel!
    @IBOutlet private var appIconImageView: UIImageView!
    ...
    override func viewDidLoad() {
        super.viewDidLoad()

        appTitleLabel.text = appTitle
        appIconImageView.image = appIconImage 
    }
}

3.viewDidAppear実行前にpresentすると画面が表示されない

本コードでは、サーバー通信を行い、通信結果を見てPromotionStoreViewControllerを表示しています。

ただし、presentはviewDidAppearより前に実行してしまうと、表示したいUIViewControllerが表示されなくなってしまいます。

今回、サーバー通信が終わるとすぐにpresentを実行するため、viewDidAppearが実行されたかどうかは、例えばネットワークのスピード起因などによってタイミングが変わってしまいます。

そのためアプリ動作時にpresentされるタイミングが都度変わってしまい、バグが仕込まれてしまうため、通信開始 or presentするタイミングを変えるなどが必要です。

その他、自分が想定していたレビュー内容は下記です。

  • tryで実行される箇所がある。do-catchを行っていないため、エラーハンドリング処理を実装するのが良い。
  • JSONSerializationを行っている。Codableを用いることがことが考えられる。（Decoder.decodeが使える、Optionalに強くなるなるなどのメリット）
  • サーバー側のJSONファイルが不正な場合に、JSONの中身が想定通りに返ってこないことが考えられる。その場合、”appId”および”appTitle”は値が入らない可能性があり、表示したPromotionStoreViewControllerでの表示内容や動作に影響が出るため、エラーハンドリング処理を実装するのが良い。
  • “appId”はSKStoreProductViewControllerを表示するために必須パラメータとなるため、optionalではない方が良さそう。
  • didTapPresentStoreViewButtonの引数のsenderは利用していないため、記述する必要はなさそう。
  • didTapPresentStoreViewButtonにて、IBActionの引数を記述する場合、Anyではなく型を指定するのが良さそう（今回はUIButton）。

ここまでが自分が想定していたレビュー指摘の一部解答です。

みなさまいかがでしたか。
会場では非常にたくさんの方々にレビューをいただきました！
（問題が見えなくなるほどのレビューをいただき、非常に嬉しかったです！）

Code Review Challengeの３問目のコードに付箋がたくさんはられている写真

たくさんのレビューをいただく中で、いくつか自分が想定していなかったレビューもありました。
その中で1つご紹介させていただけたらと思います。

async/awaitを行うメソッドの名前に動詞を入れない

今回、サーバーから情報を取得するためのメソッドdownloadPromotionAppInfoを用意しました。

このメソッドには”download”と動詞が先頭についている形です。
しかし、AppleのAPIを見てみるとasyncのついたメソッドの名称に動詞が入っていない場合があります。例えばURLSessionのdata(for:delegate:)などが挙げられます。

https://developer.apple.com/documentation/foundation/urlsession/3767352-data （外部サイト）

AppleのAPIの書き方に倣うならば、動詞をつけない方が良いのではないか？　というレビュー指摘は自分にとって予想外だったため、大変勉強になりました。

おわりに

今回のサンプルコードについては社内でレビューをたくさんしてくださった方々がいました。
この方々のレビューがなければ、もっと質の悪いサンプルコードになってしまっていたと思います。
改めて感謝を述べさせていただきたいです。
サンプルコードを企業のブースに掲載し、来場者の方にレビューいただくというのは自分にとって初めての経験でした。
初めは「自分のコードのレベルが低すぎたりしないか？」、「レビュー指摘が全然来なかったらどうしよう」といった不安がありましたが、ふたを開けてみると非常にたくさんの方々が立ち止まってくださり、仲間と相談したりなどして多くのレビュー指摘の付箋を貼ってくださいました。
また一つ、カンファレンスへの新しい参加手段を見つけられたような気がして、非常に嬉しかったです。