「凛としてSwift (血闘編ノ弐)」からの続きです。最終回です。
・redundant_discardable_let
関数の返り値を読み捨てるのに、letを使うのは冗長だよ、です。
返り値を読み捨てていることの是非はともかく、letいらないの？知りませんでした。
let _ = daoFooDB.updateFooSortKey(foos)_ = daoFooDB.updateFooSortKey(foos) //え、アンダースコアだけでいいの？まじか。
・statement_position
ステートメントの開始位置がよくない、です。
このケースでは、そのステートメントの前に空白がないよ、というわけです。opening_braceと同じような意味合いというか、対になる感じでしょうか。
if xMax <= 15.1 {
//…省略
}else if xMax <= 30.1 { //←elseの前にスペースがない
//…省略
}else if xMax <= 60.1 { //←elseの前にスペースがない
//…省略
}else { //←elseの前にスペースがない
//…省略
} if xMax <= 15.1 {
//…省略
} else if xMax <= 30.1 { //←elseの前にスペースを入れました
//…省略
} else if xMax <= 60.1 { //←elseの前にスペースを入れました
//…省略
} else {
//…省略
}
・empty_count
配列が空かを判断するのには、isEmptyを使いましょう、ですね。
if practices.count > 0 { //←配列が空か？
//…省略
} if !practices.isEmpty { //isEmptyを使用する
//…省略
}
※2018/12/29追記
countという変数名を使うと引っかかってしまうみたいなので注意です。
countという変数名を使うと引っかかってしまうみたいなので注意です。
func hoge(count: Int) {
if count == 0 { //← empty_countエラー？
//…省略
} else {
//…省略
}
}
func hoge2(work: Int) {
let count = work
if count == 0 { //← empty_countエラー？
//…省略
} else {
//…省略
}
}
・variable_name
変数名,定数名にアンダースコアが入っているものがエラーになりました。
まぁ、アンダースコアはワイルドカードとして単体で意味があるので、変数には使用しない方がいいかもしれません。大文字のスネークケースの方が定数っぽいのに、と思うのはジジイだからでしょうか。
let SQL_PRACTICE_SELECT = "SELECT id, sort_key, name FROM practice WHERE id = ?;"let kSqlPracticeSelect = "SELECT id, sort_key, name FROM practice WHERE id = ?;"
・force_cast
強制キャストは、失敗すると実行エラーになるので禁止ね、です。
そこでどうするかですが、そのキャストの失敗の重大性をどう見るかで変わってきそうです。起こりうる失敗として処理を続けるようなケースと、もう後続の処理を行う必要のないような重大なものです。
まず、後続の処理をハンドリングするために、キャストの成功時と失敗時の振り分けをif文で行う方法です。
ダウンキャストに失敗したとき、as!演算子だと実行時エラーになりますが、as?演算子を使ったときはnilを返します。これを利用してif let ～ 文と組み合わせて、ダウンキャスト失敗時の処理をelseに落とします。
func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
let cell = tableView.dequeueReusableCell(withIdentifier: "MenuTableViewCell", for: indexPath)
self.updateCell(cell as! MenuTableViewCell, indexPath: indexPath) //←強制キャスト
return cell
} func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
let cell = tableView.dequeueReusableCell(withIdentifier: "MenuTableViewCell", for: indexPath)
if let c = cell as? MenuTableViewCell {
self.updateCell(c, indexPath: indexPath) //セルの表示項目の設定
return c
} //←この例ではelseがありませんが、もし失敗時の処理が必要ならここに。
return cell
}
これならキャストの成功/失敗で何かしらの処理を行って、後続の処理に流すことができますね。また、その値が複数の型のうちのどれかである可能性があるような場合など、if文でどの型かを判別して、それぞれ処理するのにも便利です。
※)2017/06/19追記
いくつかの型を判別してそれぞれ処理する時には、switch文でパターンマッチングを使った方が綺麗ですね。
※)2017/06/19追記
いくつかの型を判別してそれぞれ処理する時には、switch文でパターンマッチングを使った方が綺麗ですね。
func fooBar(hoge: Any) {
switch hoge {
case let f as Foo:
print("name is \(f.name)")
case let f as Bar:
print("name is \(f.name)")
default:
print("something wrong?")
}
}
次に、後続の処理をキャンセルしたい場合には、guard-let文でするりと抜けるのがスマートです。letで定義した定数が、guard文以降でも値を保持したまま使用可能という、まさにこのためにあるような便利機能です。
guard文の中にelseの処理は書けますが、thenの処理は書けません。また、elseの処理の中にはそのブロック内の後続処理に流れないようなreturn(もしくはbreakやthrowなど)が必須です。if文のように分岐するというより、elseに入ったらもうブロック内の後続には流さない、以降の処理は条件が真であることを保証する、まさに「ガード」ですね。
@IBAction func sliderVolumeValueChanged(_ sender: AnyObject) {
guard let slider = sender as? UISlider else {
//メッセージを出すとかね。
return //←これは必須で、関数sliderVolumeValueChangedを抜けています
}
volume = slider.value //←guard-let文で定義されたsliderが使用できる！
//…省略
}
ちなみに、これをif letで書くと、ネストも深くなりますし、なんだか冗長です。
@IBAction func sliderVolumeValueChanged(_ sender: AnyObject) {
if let slider = sender as? UISlider {
volume = slider.value
//…省略
} else {
//メッセージを出すとかね。
}
}
ただ、このguard文の書式ですが言語的にはどうなんでしょうか。ここだけ定数のローカルスコープの例外っていうのは、なにやらモヤモヤします。規則性って大切です。例外ってのはできるだけ少ない方が人に優しいと思います。elseも、条件が偽である処理であることを示すためにあるのでしょうが、thenが書けないならelseも必要なさそうです。別の何かスマートな書式を定義しても良かったんじゃないかとは思います。言語を設計しているエラい人はそのあたりどう考えているんでしょうね？
それはさておき、ここで例に出したのはInterface Builderから生成されたアクションです。強制キャストに失敗することはありえなさそうです。
どうせキャストに失敗しないんだし、いっそのこと必要な型で受け取ってしまえば、とパラメータの型を変更してもちゃんと動きます。Lintのワーニングも出ません。でも、これは単に暗黙のうちに強制キャストをしているだけですし、むしろ強制キャストを隠してしまっているという点で、本末転倒で悪手じゃないかと思いますがどうでしょう？
@IBAction func sliderVolumeValueChanged(_ sender: UISlider) { //← AnyObjectをUISliderに変更
volume = sender.value //音量
//…省略
}
これをするくらいなら、// swiftlint:disableを使って、明示的にLintの例外とする方が健全だと思います。
@IBAction func sliderVolumeValueChanged(_ sender: AnyObject) { //← AnyObjectをUISliderに変更
// swiftlint:disable force_cast
let slider = sender as! UISlider
volume = sender.value //音量
//…省略
}
まぁ、問題なく動けば良いといえば、いいんですけどね。:P
・force_try
エラーをthrowする文は、ちゃんとdo-catchでエラーをハンドリングしなさい、です。
エラーをthrowする文は、ちゃんとdo-catchでエラーをハンドリングしなさい、です。
func playHoge(player: AVAudioPlayerNode, sourceFile: AVAudioFile, engine: AVAudioEngine) {
player.scheduleFile(sourceFile, at: nil, completionHandler: nil)
try! engine.start() //←エラーを投げる可能性あり
player.play()
} func playHoge(player: AVAudioPlayerNode, sourceFile: AVAudioFile, engine: AVAudioEngine) {
player.scheduleFile(sourceFile, at: nil, completionHandler: nil)
do {
try engine.start()
player.play()
} catch {
print("something wrong? \(error)")
}
}・unused_optional_binding
使わないならletでバインドする必要ないよね？、です。
使わないならletでバインドする必要ないよね？、です。
if let _ = h as? Hoge {
//hはHoge型だよ
} else {
//hはHoge型じゃないよ
} if h as? Hoge != nil {
//hはHoge型だよ
} else {
//hはHoge型じゃないよ
}ただ上記の例のように、単に型を判断したいだけならtype(of: T)を使った方がいいのは間違いないです。
if type(of: h) != Hoge.self {
//hはHoge型だよ
} else {
//hはHoge型じゃないよ
}
♪♪♪
以降のLintエラー、ワーニングは、// swiftlint:disableを使って明示的に回避しました。
ちなみに// swiftlint:disableに複数指定するときは空白を開けて、// swiftlint:disable cyclomatic_complexity function_body_lengthと書くようです。
・function_parameter_count
関数のパラメータの数が多すぎ、です。
Objective-Cとの絡みもあり、ちょっと今は直せないと判断しました。
@objc(calcIntonationGap:variety1:variety2:variety3:variety4:variety5:)
static func calcIntonationGap(_ stimmung: Stimmung, variety1: StimmungVariety1, variety2: StimmungVariety2, variety3: StimmungVariety3, variety4: StimmungVariety4, variety5: StimmungVariety5) -> [Double] {
//…省略
return intonationGap
} @objc(calcIntonationGap:variety1:variety2:variety3:variety4:variety5:)
// swiftlint:disable function_parameter_count
static func calcIntonationGap(_ stimmung: Stimmung, variety1: StimmungVariety1, variety2: StimmungVariety2, variety3: StimmungVariety3, variety4: StimmungVariety4, variety5: StimmungVariety5) -> [Double] {
//…省略
return intonationGap
}
・cyclomatic_complexity
これ、なんて訳せばいいのかよくわからなかったのでググったら、ウィキペディアの「循環的複雑度」が一番に出てきました。初めて聞いた単語(笑)。ざっくり言うと「分岐多すぎ」ですか。これも厄介で、五線譜を表示する処理ではどうしてもif文やswitch文の塊になるので避けられません。
・type_body_length
構造体(enumも、かな？)の長さが、長すぎ。
・function_body_length
関数の長さが、長すぎ。
・file_length
ファイルの長さが、長すぎ。
この辺りは.swiftlint.ymlを修正して調整してもいいですが、少し頑張ればなんとかなりそうなので、あえて// swiftlint:disableで手が空くまでペンディングにしました。いずれにしても、// swiftlint:disableを使って回避した箇所は、定期的に棚卸しする必要があるでしょう。
これでLintのエラー、ワーニングが全て消えました。おつかれさまでした。
「帰ってきた凛としてSwift（くるくる編）」に続きます。
♪♪♪
こちらもどうぞ。
凛としてSwift
凛としてSwift (血闘編ノ壱)
凛としてSwift (血闘編ノ弐)
帰ってきた凛としてSwift（くるくる編）
