【part 3】AppApp のリファクタリング過程紹介【SwiftLint導入後のWarning解消編】

はい。どうも、Reoです。

ごりごりブログを書くマンになっています。

AppAppリファクタリングシリーズの part3です！まだ SwiftLint 編です！長いですねー。

環境

• Xcode11.3.1
• Swift 5.1.3
• iOS13.3

初回 → AppApp のリファクタリングを始めます！【SwiftLint 導入編】

前回 → 【part 2】AppApp のリファクタリング過程紹介【SwiftLint導入後のError解消編】

リポジトリ→ uruly/AppApp

今回の目標

今回は SwiftLint を導入したことで発生した Warning を解消していこうと思います。

前回で Error 解消はしたので、ようやくビルドができる状態になりました。やったね。

.swiftlint.ymlの中身

前回と変えてないです。

disabled_rules:
  - force_cast
  - force_try

included:
  - AppApp

excluded:
  - Carthage

line_length: 200
large_tuple: 5

file_length:
  warning: 500
  error: 1200

nesting:
  type_level:
    warning: 2

identifier_name:
  excluded:
    - id
    - URL
cyclomatic_complexity: 20

Warning 解消するぞい！

さて。

目に見えている Warning は60個なんだけれど、多分もっとあります。知ってるぞ。

頑張っていこう。

Closure Parameter Position

Closure Parameter Position Violation: Closure parameters should be on the same line as opening brace. (closure_parameter_position)

もっと前のプロジェクトで、UIAlertController を多分どこかからコピペして使っていて、それをさらにコピペして使っているとこんなことになる。NSLog…

let alertController = UIAlertController(title: "Appを削除します。", message: "", preferredStyle: .actionSheet)
let otherAction = UIAlertAction(title: "Appを削除する", style: .default) {
    _ in NSLog("はいボタンが押されました")
    self.collectionView.deleteAppData()
}
let cancelAction = UIAlertAction(title: "キャンセル", style: .cancel) {
    _ in NSLog("いいえボタンが押されました")
}

alertController.addAction(otherAction)
alertController.addAction(cancelAction)

self.present(alertController, animated: true, completion: nil)

Warningが出ているのはクロージャの _ in の部分で、改行して書くなよ！っていうだけ。

なんかこれだけでもいっぱいあるので、全箇所修正してきます。

上記の修正後

let alertController = UIAlertController(title: "Appを削除します。", message: "", preferredStyle: .actionSheet)
alertController.addAction(UIAlertAction(title: "Appを削除する", style: .default) { [weak self] _ in
    self?.collectionView.deleteAppData()
})
alertController.addAction(UIAlertAction(title: "キャンセル", style: .cancel))

present(alertController, animated: true, completion: nil)

Multiple Closures with Trailing Closure

Multiple Closures with Trailing Closure Violation: Trailing closure syntax should not be used when passing more than one closure argument. (multiple_closures_with_trailing_closure)

クロージャが続く（以下の例だとanimations, completion がクロージャ）の時には、省略しないでラベルをつけてねということ。

// 修正前
UIView.animate(withDuration: 0.2, animations: {
    self.alpha = 0.0
}) { (_) in
    self.removeFromSuperview()
}

// 修正後
UIView.animate(withDuration: 0.2, animations: {
    self.alpha = 0.0
}, completion: { (_) in
    self.removeFromSuperview()
})

私はこのアニメーションのクロージャ内でselfを弱参照にすべきかが未だにわからない…ので調べました。

You don’t (always) need [weak self] – Flawless iOS – Medium

こちらの記事を読む限り、今回のは大丈夫そう。これはちょっとあとで全体じっくり読まねば…！

Identifier Name

Identifier Name Violation: Variable name should be between 3 and 40 characters long: ‘vc’ (identifier_name)

SwiftLint のデフォルト設定だと、一文字だとErrorですが、2文字はWarningになります。

vc -> viewController に変更していきます。

ちなみに仕事をして以来、hogeVC 派から hogeViewController 派になりました。

For where

For Where Violation: `where` clauses are preferred over a single `if` inside a `for`. (for_where)

for文内で if文を使っている場合

// 修正前
for object in objects {
    if object.name == name {
        return true
    }
}

こういう場合には where を使いましょうねと言われます。

// 修正後
for obj in objs where obj.name == name {
    return true
}

はい。
SwiftLint を入れて初めてこの構文を知った人も多いのではないでしょうか。私もその1人です。

Unused Optional Binding

Unused Optional Binding Violation: Prefer `!= nil` over `let _ =` (unused_optional_binding)

これはちょっとだけ好きじゃなかったりします。

// 修正前
if let _ = hoge {
    // if 文内で hoge を使わない
}
// 修正後
if hoge != nil {
    // 処理
}

if let を使う時は、ちゃんとその値を使おうね、そうじゃなかったら != nil を使いましょうねってことです。

Class Delegate Protocol

Class Delegate Protocol Violation: Delegate protocols should be class-only so they can be weakly referenced. (class_delegate_protocol)

そもそもの protocol の使い方が良くないのはあるんですが。

protocol BasePageViewControllerDelegate {
    var basePageVC: BasePageViewController { get }
}

とりあえずこの Warning は

protocol BasePageViewControllerDelegate: AnyObject {
    var basePageVC: BasePageViewController { get }
}

で修正。

class ではなく AnyObject を使う理由に関しては以下が参考になります。

Swift で Class-Only Protocol を定義する – Qiita

勉強したてのプロトコルを頑張って使っているのだけれど、結構しんどいですね。

Notification Center Detachment

Notification Center Detachment Violation: An object should only remove itself as an observer in `deinit`. (notification_center_detachment)

これは NotificationCenter を Remove する場所が違うよってことです。

// 修正前
override func viewDidDisappear(_ animated: Bool) {
    super.viewDidDisappear(animated)
    NotificationCenter.default.removeObserver(self)
}

// 修正後
deinit {
    NotificationCenter.default.removeObserver(self)
}

NotificationCenterのことはRxSwiftを初めてようやくわかった気になっています。それまではチンプンカンプンでした。

Implicit Getter

Implicit Getter Violation: Computed read-only properties should avoid using the get keyword. (implicit_getter)

getter しかないのに get なんて書かんでええねん。

// 修正前
var navigationHeight: CGFloat {
    get {
        if UIDevice.current.model.range(of: "iPad") != nil {
            return 40.0
        } else {
            return 80.0
        }
    }
}

// 修正後
var navigationHeight: CGFloat {
    if UIDevice.current.model.range(of: "iPad") != nil {
        return 40.0
    } else {
        return 80.0
    }
}

うむ。だいぶ疲労が出てきた。残り29個です。

スペースがどうのとかいうのはもう飛ばします。

Function Body length

Function Body Length Violation: Function body should span 40 lines or less excluding comments and whitespace: currently spans 45 lines (function_body_length)

この関数、ちょっと Fat じゃない？というもの。

全てを直せるかは微妙ですが、大抵の場合は直せるはずです。

AppApp の場合は、Subview の配置をコードで行なっているところが多いです。

なのでコードで書いていた部分をIBでの実装に移植する等を行えば解決できます。

この辺はちょっとがっつり直さないとダメなので、一旦保留にします。

ShareExtensionの方も…

何かがおかしいと思ったら、ShareExtensionの方でSwiftLintが効いていない…というか設定するのを忘れていました。

.swiftlint.yml の included に追記します。

included:
  - AppApp
  - ShareFromAppStoreExtension

つらみですね…

特に目新しいものはなかったのでサクッと直しました…全然サクッとじゃなかったけど…

ルールの見直し

ここまで作業して残った Warning は

• Function Body Length
• File Line Length
• Type Body Length
• Function Parameter Count

です。

なので今後はこれを無くしていくようにリファクタリングを行なっていきます。

また、タプルの許容範囲を広くしているので（large_tuple: 5）そこもデフォルトでいいようにしていく予定です。デフォルトでは 3つを超えるとエラーが出ます。

一番厄介なのは、Function Parameter Count な気がしています。関数の引数制限が結構厳しいです。

これらをなくしていくのは、現段階ですぐに行えるものでもないので、徐々にやっていきます。

次回

SwiftLint編は一応今回で終わりです。

次回は、SwiftLintが教えてくれないことについて触れながら作業をしていこうと思います。

おわり

リファクタリングは本来まとめてやるものではなく、気付いた時にコツコツ直していくことが大切なものだと思います。（って何かの記事か本で見た。）

今回解消できなかったものも、随時解消できる時にやっていこうと思います。

記事を書くことによって倍以上の時間がかかっているわけですが、わからないところをちゃんと調べようという気にはなれるので、意味ないことはないはず…。

ではまた次回。多分明日です。