[Swift]あらためて基本に立ち返ろう。Swiftソースコードでありがちなアンチパターンとそのリファクタリングポイント

#Swift

中安佑一

2021.07.07

この記事は公開されてから1年以上経過しています。情報が古い可能性がありますので、ご注意ください。

最初に

CX事業本部モバイル事業部の中安です。まいどです。

今日はクラスメソッドの創立記念日ということで、ブログを1つ投稿していこうと思います。

最近はお仕事で社内外誰かのコーディングしたソースコードを見る機会が多かったのですが、その中でソースコード自体が整理されていなかったり、Swiftらしくない記法が見受けられたり、気になるポイントが何箇所か出てきました。

この気になるポイントはたとえばコードレビューの場であれば、アウトプットとして伝えることはできると思うのですが、そうでない場合は自分の中で収まってしまうので、頭の中で何が気になるのかを整理しておきたいなと思いました。

この記事に書くことはある種「あたりまえ」なことを書いていくことになりますが、 Swift初学者の方の参考にしていただいたり、コードレビュー時の観点にしていただけると幸いかと思います。

では、色々と例を出していきましょう。

名前の記法が統一されていない

意外なことに命名記法が統一されていないというソースコードが見受けられることがあります。また、別言語の慣習がSwiftにも使われていて、Swiftらしいソースコードを阻害していることもあります。

肌感としてよく見るなと思うのが「定数はすべて大文字で書く」というものです。すなわち、定数はアッパースネークケースで書きましょうというものです。 Javaの世界ではよく使われますし(その流れでKotlinでも使われます)、Web用スクリプトの世界でもよく見かけますが、Swiftではあまり見かけません。 (サードパーティのライブラリの中でたまに見かけますが…)

Swiftの命名については様々なサイトに書いてたりしますが、複雑に難しく書いてあったりします。しかし、基本的な考え方はシンプルだと思います。

「型になりうるものはアッパーキャメルケース。ならないものはローワーキャメルケース」

この2種類しかないと思っておいてよいと思います。

■ アッパーキャメルケースを使う場面

クラス名、構造体名(struct)、プロトコル名、列挙型名(enum)、タイプエイリアス等です。

class Hoge: Fuga

func hoge() -> Fuga

上記のFugaのように : や -> の右側につくものにローワーキャメルケースが来たら違和感を覚えるようにしておきましょう。

■ ローワーキャメルケースを使う場面

変数名、定数名、列挙型の要素名、メソッド名、プロパティ名、ラベル等です。

static let constantString = "Hoge"

冒頭で話したとおり、定数も上記のようにローワーキャメルケースであるべきかと思います。

ネストが深い

これもよく見かけことのあるアンチパータンと言えます。 if-elseを大量に使うことによって可読性が下がっているソースコードです。

プログラマさんであればネストを増やさないことは常に気をつけているはずなのですが、実際にはネストだらけのソースコードを書いてしまい、後から見返すと自分で読み返すのも嫌なくらいのものが出来上がってることもあります。

if user.age >= 20 {
    if user.isStudent {
       // something
    } else {
       // something
    }
} else {
    // something
}

ひとつのメソッド内では、意識的にネストは1深層程度に留める努力をしたいものです。

if user.age < 20 {
    // something
    return
}

if user.isStudent {
    // something
    return
}
// something

他にもguard文などを使用して、ネストの深度を下げていきましょう。

get〜は使わない

SwiftのAPIデザインガイドラインでは「副作用に応じてメソッドに名前を付ける」とされています。

副作用は、メソッドの呼び出しによってオブジェクトの状態、または別のオブジェクトの状態が変化することを指しますが、 副作用があれば名前を動詞に、副作用がなければ名前を名詞にすることが推奨されています。

俗に「ゲッタ/セッタ (getter/setter)」と呼ばれるようなメソッドの名前を付ける場合、これもまたJavaの世界などではよく見られますが、「get〜」「set〜」という名前を選択することが多いです。

しかし、Swiftでは前述のような命名慣習があるため、副作用のない取得系メソッドに動詞はつけません。

これはiOSの標準クラスでも見受けられます。

ボタンの例でいうと

let button = UIButton()
// ボタンタイトルの取得
let title = button.title(for: .normal)
// ボタンタイトルの設定
button.setTitle("タイトル", for: .normal)

このとき

// ボタンタイトルの取得(これは良くない例)
let title = button.getTitle(for: .normal)

という名前にはしないということです。

なぜなら、setTitle(for:)はbuttonの状態を変更する副作用があるのに対して、 title(for:)はbuttonの状態に影響を与えないからです。

名前だけで戻り値が分かるようにする

Bool値

ifやwhileなどの構文において分岐を判別するのはBool型であり、非常によく使われます。しかし、よく見かけるアンチパターンなソースコードの場合「それ、Boolが返ってくるんだ」と思わせるものも多いです。

`is〜`

Swiftでは判別のためのBoolを返すメソッドや変数にはis〜を先頭につけることが推奨されています。たとえばUIView系のhiddenやselectedなどは、isHiddenやisSelectedなどに途中で統一されました。

これでクラスを使用する場合に、別の開発者にBool値が返ることを名前だけで明示することができ、リーダブルなソースコードになります。

特殊ケース `has〜`

「〜を持っているか」というBool値の場合は、has〜を先頭に付けます。

/// (たとえば配列内に)指定した要素があるかどうかを返す
func hasElement(_ element: Any) -> Bool

isHaveElementよりも英語的には直感的に読めると思います。

特殊ケース `should〜`

「〜をするべきか」というBool値の場合は、should〜を先頭に付けます。

/// 指定したインデックスをハイライト表示すべきかどうかを返す
func shouldHighlight(at index: Int) -> Bool

何を判定しているのかを書く

例えば下記であると、配列のインデックス範囲外の値が来てもエラーにならないように、あらかじめインデックスの範囲内かどうかの判定をして安全に値を取り出しています。

if 0 <= index, index < list.indices.count {
    let element = list[index]
}

これを以下のように変数に入れておき、変数名で何を判定しているかを明示しておけば後で読んでも理解が早くなると思われます。 (場合によりますが)

let isSafeRange = (0 <= index, index < list.indices.count)

if isSafeRange {
    let element = list[index]
}

この程度のものであれば最初の書き方でも良いと思いますが、これが複雑な判定になってくると、if文に様々な条件が書かれているだけで混乱のもとになります。「何を判定しているのか」を明示しておくことは、自分のためにも後で読む他人のためにも優しくなると思います。

オブジェクト

前項で副作用の有無で名詞と動詞に分けるという話をしました。

オブジェクト自体が影響を受ける場合(副作用がある場合)は動詞の原型を、オブジェクト自体が影響を受けない場合は過去分詞(たいていの場合は 〜ed)とすることで影響度を明示することが大事です。

// employees自体がソートされる
employees.sort()

// ソートされたものが返却され、employees自体は影響されない
let sortedEemployees = employees.sorted()

このあたりも標準APIが使い分けているので参考にすべきかと思います。

ラベルについて

メソッドのラベルはある程度の英語センスが試されますが、まずは標準のAPIのシグネチャを参考するのがよいかもしれません。

前置詞

たとえば、配列のインデックスを引数で指定する場合

func remove(index: Int)

employees.remove(index: 0)

としても意味は伝わりそうですが

func remove(at index: Int)

employees.remove(at: 0)

このようなシグネチャにすることでソースコードが英文として読みやすくなることもあります。

また、以下のような複数の同格の引数に対する前置詞については、メソッド名自体に前置詞をつけるパターンが標準のAPIには見受けられます。

func move(to x: CGFloat, y: CGFloat)
↓
func moveTo(x: CGFloat, y: CGFloat)

読みやすさの観点からリファクトしてみてもいいかもしれません。

ラベル省略

そもそもメソッド名自体に与えるべき引数が表現されている場合は、ラベルを省略することを検討したいところです。

func addEmployee(employee: Employee)

list.addEmployee(employee: employee)

こうなってくると、employeeという文字が何回出てくるんだということになります。言われてみると違和感があるのですが、こういうソースコードはよく見かけます。

func addEmployee(_ employee: Employee)

list.addEmployee(employee)

Employeeを追加することは分かっているので、ラベルは省略しても読みやすさは変わらないと思います。

また、複数の引数が同格、または区別しない場合はラベルを省略することを検討します。

func max(a: Int, b: Int) -> Int
↓
func max(_ a: Int, _ b: Int) -> Int

文脈の中で使用者にとって a と b は関係ないので、省略すべきというロジックです。

$0...の扱い

クロージャでは短縮引数として$0 $1 を使用することができますが、この利用は1行で収まる程度の中だけで行ったほうがいいと思います。

let names = users.map { $0.name }

そうではなく、たとえばクロージャの中で何行にも渡って書かなければならない時は、引数には名前をつけておくべきかと思います。

let names = users.map {
    let sei = $0.sei
    let mei = $0.mei
    return "\(sei) \(mei)"
}
↓
let names = users.map { user in
    let sei = user.sei
    let mei = user.mei
    return "\(sei) \(mei)"
}

「$0の中身って何だっけ」となる時点でバッドコードだと思うので、その場合はリファクトしていきましょう。

スコープを曖昧にしない

こちらも意外なことにスコープが意識されていないというソースコードを見ることがあります。 (おおよそは意識されていますが)

クラスの三原則は「継承」「隠蔽化(カプセル化)」「ポリモーフィズム」と言われます。その中でも「隠蔽化」の観点により、クラスの外部からアクセスできるものは制限されるべきかと思います。

class HogeViewController: UIViewController {
    
    @IBOutlet weak var button: UIButton!
    
    @IBAction func didTapButton() {
        
    }
}

ビューコントローラが管理するボタンに対して外部からアクセスすることがないはずですが、この書き方であると外部からdidTapButton()が呼び出せることになっていますし、 buttonの参照にもアクセスすることができてしまいます。

プロパティやメソッドは書き出しの時点では原則プライベートにしておくことが大事です。

class HogeViewController: UIViewController {
    
    @IBOutlet private weak var button: UIButton!
    
    @IBAction private func didTapButton() {
        
    }
}

定義するクラスによってまちまちではありますが、ことビューコントローラに関してはパブリックになるプロパティやメソッドはほとんどないはずです。パブリックが多いソースコードになっている場合は設計の見直しが必要だと思います。