[Swift] コードレビュー方法・ベストプラクティス（その１）
コードレビューは、簡潔なコードベースを維持するために重要なだけでなく、異なるコーディングスタイル（同僚のコーディングスタイル）を学ぶ方法としても重要だと考えている。さらに、優れたコードレビューでは、PMやQAがテスト中に発見できないような問題を発見することができる。この記事は、iOSのレビューでチェックすべき項目を示すことを目的としてる、、、(理想的な世界では)。
不要な「import」が利用されているか
不要な import を使用しているかどうかを確認してください。
ビルドが壊れることはないが、どのライブラリにどのフレームワークが含まれているかを確認した方が良い。例えば、 Foundation はSwiftUIとUIKitに含まれている。なのでSwiftUIを使ったら、 Foundation も import する必要がない。
レビューの前
import UIKit
import SwiftUI
import Foundation
class ViewController: UIViewController { ... }
レビューの後
import UIKit // FoundationはUIKitに含まれます。SwiftUIは使われていない。
class ViewController: UIViewController { ... }
型推論をさせているか
型推論を使用しているかどうかを確認してください。
Swiftで型推論を行うと、DoubleをIntとして解釈したり、FloatをDoubleとして解釈したりすることがあるので、変数を宣言するときには型を明示的に宣言することが最善である。特に数値の場合は、BoolやString、コレクション型よりも特に重要。
レビューの前
let myNumber = 0
let myDouble = 0.0 // 0.0じゃないとIntとして推論
レビューの後
let myNumber: Int = 0 // Int
let myDouble: Double = 0 // Double
protocol/delegateをextensionにリファクターしたか
デリゲートを以下のようにextensionとして分離しているか確認してください。必須ではないけど、読みやすくなる。
レビューの前
class HomeViewController: UIViewController, UITableViewDelegate, UITableViewDataSourceDelegate {
override func viewDidLoad() {
super.viewDidLoad()
...
}
func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
tableView.deselectRow(at: indexPath, animated: true)
...
}
func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
...
}
}
レビューの後
class HomeViewController: UIViewController {
override func viewDidLoad() {
super.viewDidLoad()
...
}
}
extension HomeViewController: UITableViewDelegate {
func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
tableView.deselectRow(at: indexPath, animated: true)
...
}
}
extension NotificationViewController: UITableViewDataSource {
func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
...
}
}
誤字があるか
当然だけど、誤字があるかどうか慎重に確認してください。
レビューの前
@IBOutlet var filtreButten: UIButton!
レビューの後
@IBOutlet var filterButton: UIButton!
高階関数を活用しているか（map, flatMap, reduce, filterなど）
Swiftの高階関数はとても便利だと思う。必須ではないが、読みやすくなると思う。
レビューの前
let words: [String] = ["room", "home", "train", "green", "heroe"]
var uppercasedWords: [String] = [String]()
for word in words {
uppercasedWords.append(word.uppercased())
}
レビューの後
let words: [String] = ["room", "home", "train", "green", "heroe"]
let uppercasedWords = words.map({ word in
return word.uppercased()
})
オプショナルを強制的アンラップしたか（@IBOutletを除く）
オプショナルを強制的アンラップすることは推奨されていない（クラッシュするリスクが上がるため）。
アンラップしたかどうかを確認すること。下記のようにアンラップするのは簡単でセーフコードになる。
レビューの前
var name: String? = nil
print("\(name!) letters")
レビューの後
// if let (条件が必要であれば)
// iOS16以下
if let unwrapped = name {
print(unwrapped)
} else {
print("nil")
}
// iOS16以降
if let name {
print(name)
} else {
print("nil")
}
// guard let
guard let unwrapped = name else { return }
print(unwrapped)
APIがdeprecatedか
Xcodeで警告が出るはずだが、要確認。
特に、SwiftUIのメソッドがすぐdeprecatedになる。
レビューの前
protocol yourDelegate: class { // Xcodeで警告が出るはず
...
}
レビューの後
protocol yourDelegate: AnyObject {
...
}
またこの続きがあるので、その２もお楽しみに👨🏫
Spacely, Inc. App Div.
Dean Thompson
参考
Discussion