[Swift] いつprivateメソッドに切り出すか

長すぎるメソッドは明らかに分割するべきですが、だからと言って何でも分割すればいいというものではありません。自分が普段使っている考え方を挙げてみます。

メソッドの数を増やしすぎない

一般論として、極端に言うと、ひとつのクラス内に15行のメソッドが10個ある場合と、3行のメソッドが50個の場合では前者の方が良いことが多いです。メソッドが多すぎると認知負荷が増大し、可読性を悪化させます。一方、長すぎるメソッドは問題ですが1画面に収まる程度であれば通常問題になりにくいです。それ以上に分割する場合は全体のバランスを考えた方がいいです。

短すぎる処理を切り出さない

以下の2つのメソッドは理解しやすい名前を持っていますが、コードを直接読んだほうがずっと理解が早いです。このような場合はメソッドに切り出すよりもベタ書きしてあったほうが有利です。同じ処理が複数箇所に重複して存在する場合でさえ、2〜3行程度の十分短い処理ならばベタ書きのほうが有利である可能性があります。ただし、重要なロジックが含まれる場合はたとえ1行でも重複は避けた方が良いでしょう。

private func backToPreviousScreen() {
    self.navigationController?.popViewController(animated: true)
}

private func showDetailViewController() {
    let vc = DetailViewController()
    self.navigationController?.pushViewController(vc, animated: true)
}

一箇所からしか実行されない処理

以下のコードでは、viewDidLoad内で行うべき処理の一部がsetUpSomethingメソッドに切り出されています。他の箇所からsetUpSomethingが呼ばれることはありません。このような切り出し方にはいくつかのデメリットがあります。

override func viewDidLoad() {
    /* 何らかの初期化処理 */
    setUpSomething()
}

private func setUpSomething() {
    /* 何らかの初期化処理 */
}

viewDidLoad内に存在するコードはどのようなタイミングで実行されるのかiOSプログラマーなら誰でもすぐに理解することができるという大きなメリットがありますが、setUpSomethingはそうではありません。setUpSomethingを理解するためにはファイル内を検索し、viewDidLoad内からのみ呼ばれていることを確認する手間が発生します。メソッドに切り出すことによって、処理のひとかたまりに適切な名前をつけることができるというメリットはありますが、コメントで代用が可能な場合もあります。

privateメソッドの動作保証

クラスにprivateメソッドを書くということは「クラス内部に向けてメソッドを提供する」という意味であり、基本的に「そのメソッドはクラス内のどこから呼ばれても正しく振る舞う」ように書かなければなりません。（同様に、publicメソッドを書いたら、そのメソッドは基本的にいつどこから呼ばれても正しく振る舞う必要があります。）意識的に実践している人は多くないかもしれませんが、これによってプログラムの堅牢性が飛躍的に上がります。

以下のコードはこのルールに違反している例です。

private func showMessage(_ message: String) {
    let label = UILabel(frame: CGRect(x: 10, y: 10, width: 100, height: 30))
    label.text = message
    /* その他labelの設定をする */
    self.view.addSubview(label)
}

このメソッドはおそらく一度だけしか実行されないことを前提においており、複数回実行された場合は重複したUILabelオブジェクトが生成され期待していない結果となります。現在の仕様で1度しか実行されないからといって、それを暗黙の前提にしたコードを書いてはいけません。これはバグの温床となります。この例の場合「既にlabelが存在するならば再利用する」などの改善が考えられます。

以下の例を見てみましょう。

private var hoge: Int?

func doSomething() {
    doSomeWorkA()
    doSomeWorkB()
}

private func doSomeWorkA() {
    // 何らかの処理
    self.hoge = /* 処理の結果 */
}

private func doSomeWorkB() {
    // 何らかの処理
    // print(self.hoge!)
}

doSomethingの実装が長くなったため、doSomeWorkAとdoSomeWorkBに分割しました。このふたつのメソッドはdoSomething以外から呼ばれることはありません。ただし、一度切り出したならばdoSomeWorkBはdoSomeWorkAの直後以外のタイミングで呼ばれる可能性を考慮するべきです。そのように書けないならばメソッドの切り出しを避けて長いdoSomethingメソッドを許容したほうが良い可能性があります。

おまけ: 副作用のないprivateメソッドを書く

上で「privateメソッドはクラス内のどこから呼ばれても正しく振る舞うべき」であると書きました。副作用をなくすことで、privateメソッドに切り出したままこれを簡単に達成できる場合があります。前述したshowMessageメソッドのもうひとつの改善例を挙げます。

private func makeMessageLabel(_ message: String) -> UILabel {
    let label = UILabel(frame: CGRect(x: 10, y: 10, width: 100, height: 30))
    label.text = message
    /* その他labelの設定をする */
    return label
}

// 使用例
override func viewDidLoad() {
    /* ここで他の初期化処理をする */
    self.view.addSubview(makeMessageLabel("message"))
}

この例ではaddSubviewを呼び出し側に任せることにより問題を回避しています。このコードではlabelの重複が起こりえないことが簡単に読み取れます。