0

以下のSwiftコードのネストを浅くしたいのですが、いい案はないでしょうか?

task = URLSession.shared.dataTask(with: URL(string: url)!) { (data, res, err) in
    if let obj = try! JSONSerialization.jsonObject(with: data!) as? [String: Any] {
        if let items = obj["items"] as? [[String: Any]] {
            self.repo = items
            DispatchQueue.main.async {
                self.tableView.reloadData()
            }
        }
    }
}

4 件の回答 4

1

はじめまして、URLSessionで非同期なので、タイミングによってはselfが解放されている可能性まで考えてのコードです。
guard文の多用になってあまりすっきりはしていないかも知れませんが、あえて guard let a = b, let c = dの様な一つのguard文で複数のチェックを行う事は今回は避けています。
エラー処理の構文まで含めると1ネスト減らすことが出来ていて、
ロジック自体は2ネスト減らすことができそうです。

guard let url: URL = URL(string: url) else {
    print("URL not constructed")
    return
}
task = URLSession.shared.dataTask(with: url) { [weak self] (data, res, err) in
    guard let weakSelf = self else {
        print("self already deallocated")
        return
    }
    guard let jsonData: Data = data else {
        print("received data not exist"
        return
    }
    guard let obj = try? JSONSerialization.jsonObject(with: data!) as? [String: Any] {
        print("JSON Serialization failed")
        return
    }
    guard let items = obj["items"] as? [[String: Any]] else {
        print("items is not much type")
        return
    }
    weakSelf.repo = items
    DispatchQueue.main.async { [weak self] in
        guard let weakSelf = self else {
            print("self is already deallocated")
            return
        }
        weakSelf.tableView.reloadData()
    }
}

try!は続くメソッドや関数が例外を発生させた時にエラーでアプリケーションそのものが終了してしまうため、try?というエラーが起きた時はnilを返すtryに書き替えてあります。
これにより、objというローカル変数はそれ以降必ずあることが保証されるので、guard文によるアンラップチェックを多用してデーターが十全であることをアンラップが必要な度にしつこいほど確認・保証する作りになっています。これによりなにをアンラップした時にエラーになったか?、が続く処理ですぐにかけるので、エラーの発生箇所と対処箇所がすぐ近くに書けるのもメリットの一つだと思います

また、URLSessionにデーターをリクエストしているので、指定したURLからデーターが帰ってきた時には、呼び出したクラスが解放しようとした後かも知れない場合も考慮し、クロージャーの先頭に
[weak self] (引数があればここに書く) inという「このブロックの中ではselfを弱参照してメモリーリークを防ぐ」と共に、
guard let weakSelf = self else { return }
という一文で弱参照しているselfが解放されていない(nilでない)ことも確認しています。

コード全体のネストは一段しか減らせていないですが、オプショナル型のアンラップ時のnilによるアプリのクラッシュを可能な限り防ぐ方向に振っています。

2
  • Swift初心者でguardも知りませんでした。ありがとうございます。メモリーリーク、try?問題も抱えていたので、いくつも解決できました!!
    – zunda
    Commented 2020年6月30日 7:45
  • do{} catch A {} catch B {} catch {}も、エラー発生箇所と対処箇所が離れる反面、エラーが起きていない時のアルゴリズムをすっきり書けるというメリットがありますので、!?なしのtrythrowと適宜使い分けて下さい
    – Чайка
    Commented 2020年6月30日 7:56
1

ネストを減らすと言う意味では、guard文の採用も検討された方が良いでしょう。

task = URLSession.shared.dataTask(with: URL(string: url)!) { (data, res, err) in
    guard let obj = try! JSONSerialization.jsonObject(with: data!) as? [String: Any],
        let items = obj["items"] as? [[String: Any]]
        else {
            print("bad data")
            return
    }
    self.repo = items
    DispatchQueue.main.async {
        self.tableView.reloadData()
    }
}

なお、コード例の中でtry!を使用していますが、JSONSerialization.jsonObject(with:)は、入力となるデータに1バイトでもおかしな点があれば簡単に失敗するので、try!を使用するとそこでアプリがクラッシュしてしまいます。「強制tryは確実に安全と分かっている場合以外は使ってはいけない」と覚えておいた方が良いと思います。

別質問とも関わりますが、その他の危険な強制アンラップも全部避け、エラーチェックもきちんとやるとこんな感じになります。「ネストを減らす」と言うご質問の趣旨からは外れることになりますが、URLSessionは様々な理由でエラー終了しますので、(今は練習用のコードなのかもしれませんが、例えそうだとしても)必要なエラー処理を省略すべきではありません。

guard let theUrl = URL(string: url) else {
    print("bad URL string")
    return
}
task = URLSession.shared.dataTask(with: theUrl) { (data, res, err) in
    do {
        if let error = err {
            throw error
        }
        guard let data = data else {
            print("data is nil")
            throw MyError.dataNil
        }
        guard let obj = try JSONSerialization.jsonObject(with: data) as? [String: Any],
            let items = obj["items"] as? [[String: Any]]
            else {
                throw MyError.badData
        }
        self.repo = items
        DispatchQueue.main.async {
            self.tableView.reloadData()
        }
    } catch {
        print(error)
        //必要であればエラー時の処理を追加
    }
}

(どこかにこんなものを定義してある前提のコードです。)

enum MyError: Error {
    case dataNil
    case badData
}
4
  • guard内のthrowはreturnも兼ねてくれているのでしょうか?
    – zunda
    Commented 2020年6月30日 8:22
  • @zunda さん、何やらコメントをいただきかけたようですが、コードの一部を示されただけでは何を言いたいのかがよく分かりません。その部分のコードに何か疑問点があるでしょうか?
    – OOPer
    Commented 2020年6月30日 8:33
  • guardではreturnが必要だと思うのですが、そこをthrowは補ってくれているということなのでしょうか?という疑問でした。 guard let data = data else { print("data is nil") return //throw MyError.dataNil }
    – zunda
    Commented 2020年6月30日 8:36
  • 「returnが必要」というのは正しい覚え方ではないですね。正しくは「処理を中断するような文が必要」と言えます。(戻り値を返さない)メソッドやクロージャの中ならreturnが適当なこともありますし、私が示したdo-catchの中に書くのであればthrow 〜でも構わないわけです。「絶対にエラーが発生しないはず」という場合には、fatalError()を記述して、(エラーが出ないはずなのにエラーが出たら)アプリをわざとクラッシュさせるというコーディングをすることもあります。
    – OOPer
    Commented 2020年6月30日 8:45
0

一つネストを減らせました。

参考:Swift 1.2のif let複数宣言と例外処理

task = URLSession.shared.dataTask(with: URL(string: url)!) { (data, res, err) in
    if let obj = try! JSONSerialization.jsonObject(with: data!) as? [String: Any],
       let items = obj["items"] as? [[String: Any]] {
           self.repo = items
           DispatchQueue.main.async {
               self.tableView.reloadData()
           }
    }
}
0

回答してくださったのを統合した感じでコーディングしてみました。

そしてErrorに関するMyError.swiftのファイルを別に作りました。

わかったこと

  • try?文は基本「guard else」とセットで使うこと
  • Error処理をちゃんとすること
guard let url: URL = URL(string: url) else {
    print("URL not constructed")
    return
}
            
task = URLSession.shared.dataTask(with: url) { [weak self] (data, res, err) in
    do {
        if let error = err {
            throw error
        }
                
        guard let weakSelf = self else {
            print("self already deallocated")
            return
        }

        guard let jsonData = data else {
            print("data is nil")
            throw MyError.dataNil
        }
        guard let obj = try? JSONSerialization.jsonObject(with: jsonData) as? [String: Any] else {
            print("JSON Serialization failed")
            throw MyError.badData
        }

        guard let items = obj["items"] as? [[String: Any]] else {
            print("items is not much type")
       throw MyError.badData
        }
        weakSelf.repo = items
        DispatchQueue.main.async { [weak self] in
            guard let weakSelf = self else {
                print("self is already deallocated")
                return
            }
            weakSelf.tableView.reloadData()
        }
    }catch {
            print(error)
    }
}

1
  • 1
    try?を使用すると、「投げられたエラー情報を捨ててしまう」ことになるので、本当に捨ててしまって良いかどうかはよく考えた方が良いでしょう。また、URLSession.shared.dataTask(with:)DispatchQueue.main.asyncも循環参照を作る心配はないので、[weak self]を使う必要性(「循環参照を避ける」という意味での必要性)はありません。
    – OOPer
    Commented 2020年6月30日 8:31

この質問に回答するには、ログインする必要があります。

求めていた回答ではありませんか? のタグが付いた他の質問を参照する。