はじめに
- Swift with iOSなコードをレビューする時に僕が気をつけて見ているところをざっくりまとめました
- 今年からSwift始めます!って人が読めばクラッシュしやすいコードを書きにくくなるはず
- ロジック面についてもちゃんと確認しましょう
勘所
AnyObject使ってる
不適切に使われてる(型を消すとか)場合には絶対に直させる。型を消したい場合は protocol
+ typealias
とかgenerics
でなんとかできる場合が多い。というか AnyObject
を使ってるコードでは、型を消す必要がない場合が多い。
配列にindex指定してアクセスしている
let item = items[index]
とか。基本的にはindex
など使わずにアクセスするのが一番なので、書き直せないか確認する。
書き直せない場合はロジック自体の設計を変えられないか、もしくははみ出ない処理がちゃんと書かれているかを確認している。
Force Unwrapしてる
let width = optionalView!.frame.width let dic = unknownAnyObject as! [String : String] someFuction(optionalData!)
クラッシュ原因ほとんどはこれ。安易にForce Unwrapすべきではない。
if let width = optionalView?.frame.width { // この中で `witdh` を使う } if let dic = unknownAnyObject as? [String : String] { // この中で `dic` を使う } if let data = optionalData { someFunction(data) }
ちなみに、StoryBoard系とかCell系とかの呼び出しには使っても良い。
let storyboard = UIStoryboard(name: "WelcomePage", bundle: nil) let viewController = storyboard.instantiateInitialViewController() as! WelcomePageViewController
なぜなら、これらが無かった時にエラーハンドリングできない。できても画面が真っ白になるとかそういう感じなので、デバッグ中やQA中にクラッシュしてもらって素早く直した方が良い。
非同期処理の中で self
呼んでるのに weak
or unowned
になってない
self
呼んでるのに weak
or unowned
になってないlet queue = dispatch_queue_create("com.example.task", DISPATCH_QUEUE_SERIAL) dispatch_async(queue) { self.someFunction(self.someData) }
非同期処理中では self
が先に消えてることがあり、アクセスするとクラッシュする場合がある。
let queue = dispatch_queue_create("com.example.task", DISPATCH_QUEUE_SERIAL) dispatch_async(queue) { [weak self] self?.someFunction(self?.someData) }
weak
にしておくと、block内のself
がOptionalとして扱われるので、安心して使える。
追記(7月5日21時)
これは誤りです。非同期処理中のself
は強参照で確保されているため、self
が先に消えることはないため、weak
やunowned
にする必要はありません。ブコメ等でご指摘いただいた皆様、ありがとうございました。
delegate
が weak
じゃない
protocol SomeDelegate: class { func updateItem() } class SomeClass { var delegate: SomeDelegate? }
とか宣言されてる。ようこそ循環参照の村へという感じになる。 delegate
には大体親の ViewController
から self
を渡すので、これを weak
にしておかないと SomeClass
のインスタンスは永遠に開放されない。「なんで deinit()
呼ばれないんだろ?」と思ったらだいたいこれが原因。
protocol SomeDelegate: class { func updateItem() } class SomeClass { weak var delegate: SomeDelegate? }
ちゃんとweak
にしようなっ!
nilチェックを !=
でしてる
if optionalValue != nil { awesomeMethod(optionalValue!) }
Obj-Cっぽい。Swiftでこういう書き方をしなければいけない場面はほぼない。
if let value = optionalValue { awesomeMethod(value) }
と書ける。もし、nil
かどうかだけ見たくてblock内では利用しない場合は
if let _ = optionalValue { // do something }
と書くのが良い。
guardによる早期リターンをしてない
if someFlag { // 10行ぐらいの処理 } else { // 1行の処理 }
10行の処理
って書いてるけど、だんだん10行とかでは済まない長さになり、ネストが深くなり、可読性が下がる。可読性が下がるとエンバグしやすくなる。エンバグすると人が死ぬ。
guard someFlag else { // 1行の処理 return } // 10行の処理
と素早くreturn
してのが良い。
繰り返し処理の中で guard
や if
で return
してる
for item in items { guard item.someFlag else { return } }
guard
慣れしてくるとうっかりミスる奴。return
ではなくてbreak
とか continue
じゃないとダメな場合があるので、気をつけて見ている。
??
で書けるところを if
で分岐している
let x: CGFloat let y: CGFloat if let view = optionalView { x = view.frame.x y = view.frame.y } else { x = 0 y = 0 }
大体の場合においてコードが長くなって可読性が下がる。optionalValue
が nil
だった場合にエラーハンドリングしたい場合はこの書き方で良いが、値が欲しくてデフォルト値も決まっている場合は
let x: CGFloat = optionalView?.frame.x ?? 0 let y: CGFloat = optionalView?.frame.y ?? 0
と書くと良い。
min(x: T, y: Y) や max(x: T, y: Y) を使ってない
if view.frame.width > 540 { width = 540 } else { width = view.frame.width }
view
のサイズが540より大きかったら540にして、そうじゃないならview
のサイズを使うというやつ。こういう処理をする時はだいたい高さや縦横比なんかの計算が混ざるので、上記みたいにシンプルに終わらず if
のネストが始まってつらい。
let width = min(540, view.frame.width)
こうするとシュッと終わる。