Objective-C のコードレビューチェックリスト

  • 673
    Like
  • 3
    Comment
More than 1 year has passed since last update.

はじめに

本稿は Juri Pakaste 氏による Cocoa review checklist (commit fff5703)の翻訳です。他人の Objective-C のコードをレビューするとき注意する点、また普段のコーディングで心がけるべき点についてまとめられています。

なお、原文のタイトルは Cocoa review checklist となっていますが、内容が Cocoa に限らない範囲のトピックをカバーしているため、本稿のタイトルは「Objective-C の〜」としました。

誤訳の指摘や例の補足を歓迎します。

コードレビューチェックリスト

コードの見た目とコード以外の問題

不要な #import@class 宣言を消す

#import をソートする

.m ファイルの中では、対応する .h ファイルの #import を最初の行に書く。空行をはさんで、ソートされた他の #import を書く。

Xcode のグループをソートする

Xcode のプロジェクトツリーをソートされた状態に保つ。

メソッドをうまく並べる

メソッドを合理的に並べる。#pragma mark でメソッドをグルーピングする。

訳注: #pragma mark MarkName ディレクティブを記述すると、 Xcode のメソッド一覧に MarkName が表示される。 #pragma mark - でメソッド一覧に分割線を挿入できる。

コンパイラ警告をなくす

-Werror (少なくともリリースビルドでは)と他の警告フラグを有効にする。

静的解析の警告をなくす

標準のスタイルに合わせてコードをフォーマットする

プロジェクト内で一貫したコーディングスタイルを定め、それに従う。 clang-formatUncrustify などのフォーマッタを使うことにしたなら、レビュー前にそれが実行されているか確かめる。

スペルと文法

文法の委細にこだわる必要はないが、明らかな誤字や文法の誤りは残さないようにする。

xcconfig を使うならプロジェクトのビルド設定を変更しない

訳注:ビルド設定は、プロジェクト内に記述するほか、 xcconfig という外部ファイルにも書くことができる。 xcconfig を使うことにしたなら、ビルド設定は一貫してそちらに記述する。

画像は正しいサイズで、最適化もかける

ImageOptim などの画像最適化ツールを使う。画像が最適化されているかどうかレビュアーが判別できないときは、画像を追加した人に確認する。または、問題が起きる可能性もあるが、最適化ツールを自分で走らせてみる。

すべての画像に等倍と2倍の解像度のバージョンを用意する

2倍のバージョンは縦横のサイズが正しく元の2倍になるようにする。

テスト

失敗するテストがないようにする

ビジネスロジックのユニットテスト

プロジェクトにユニットテストを採用しているなら、少なくとも UI や IO に関係しない新規のコードにはテストが必要。

機能テスト

プロジェクトに KIF などの機能テストを採用しているなら、 UI に関係する新規のコードにはテストが必要。

ドキュメンテーションとライセンス

すべてのドキュメンテーションを現状のコードに一致させる

現状で依存しているライブラリのライセンスを、ソフトウェアライセンス表示画面に反映する

Changelog を更新する

変更点を Changelog ファイルに記録しているなら、バージョン管理システムのすべてのフィーチャーブランチで、そのブランチでの変更点を Changelog ファイルに残す。

Cocoa コーディング

Apple の命名規約に従う

変数やメソッドの命名は Apple の Coding Guidelines for Cocoa に従うべきである。

メソッドの間接呼び出し(デリゲートや通知)は読みやすく

Cocoa にはメソッドを間接的に呼び出す手段がいくつかある。読みやすさの順に並べると:

  1. 直接メソッドを呼ぶ
  2. デリゲート
  3. NSNotificationCenter
  4. レスポンダチェイン
  5. KVO
  6. Cocoa バインディング

リストの上にあるものほど読みやすいので、下のものより優先して使う。ただしメソッド間の結びつきを強くしすぎないこと。また一貫性に気を配る。

スレッドよりもキューを使う

フレームワークがシングルスレッドからのアクセスを要求しているのでなければ、GCD や NSOperationQueue を使う。

通知を送受信するスレッドにポリシーを設ける

NSNotificationCenter はスレッド1つにつき1インスタンス作る。 -defaultCenter はメインスレッドからのみ使う。正しいスレッドで通知することを送信側の責任とする。

オブザーバを正しく登録・登録解除する

通知や KVO のオブザーバは必要に応じて登録し、不要になったら登録を解除する。とくに -dealloc の中で解除を忘れない。

イベントハンドラとロジックを分離する

通知のハンドラメソッドやアクションメソッドに複雑なロジックを書かない。

絶対にメインスレッドをブロックしない

ディスク IO やネットワーク IO 、複雑な計算処理がメインスレッドをブロックしないようにする。

NSStringFromEnum / NSStringFromStruct を使う

(未訳、これのことか)

-description メソッドを書く

すべてのクラスは、デバッグに役立つ情報を -description メソッドで返すようにする。

オブジェクトの生成は最小限にし、不要になったらすぐ解放する

ループの中で一時オブジェクトを生成しているなら、それが本当に必要か考慮する。 @autoreleasepool に追加できないか検討する。

int より enumenum より NS_ENUM

関連する整数値を列挙するなら、 intenum より NS_ENUM マクロを使う。

訳注:例:

typedef NS_ENUM(NSInteger, MyEnum) {
    MyEnumA,
    MyEnumB,
    MyEnumOther,
};

- (void)foo {
    MyEnum myEnum = MyEnumA;
}

訳注: NS_ENUM で定義された列挙型の変数を switch 文で分岐すると、 case 節がすべての値を網羅しているかコンパイラがチェックしてくれる。

訳注:ビットマスクの定義には int より NS_OPTIONS を使うとよい:

typedef NS_OPTIONS(NSInteger, MyFlag) {
    MyFlagNone = 0,
    MyFlagA = 1 << 0,
    MyFlagB = 1 << 1,
};

Cocoa の命名規約に従う

(未訳)

nonnull 属性

NULL でないポインタを受け取る関数には __attribute__((nonnull)) 属性をつける。

訳注:例:

void foo(void *p) __attribute__((nonnull)) {
}

objc_requires_super 属性

オーバーライドされたとき子クラスのメソッドから呼び出されなければならないメソッドには __attribute__((objc_requires_super)) 属性をつける。訳注:最近の SDK では NS_REQUIRES_SUPER も使える。

訳注:例:

/*** 親クラス内 ***/
- (void)foo:(id)arg __attribute__((objc_requires_super)) {
    
}
// または
- (void)foo:(id)arg NS_REQUIRES_SUPER {
    
}

/*** 子クラス内 ***/
- (void)foo:(id)arg {
    [super foo]; // これを呼ばないと警告が出る
}

pureconst

グローバルな状態を変更しない関数には __attribute__((pure)) 属性をつける。グローバルな状態を参照することもない関数には __attribute__((const)) 属性をつける。

訳注: __pure__const も使える。

国際化

ユーザーに向けて表示される文字列は NSLocalizedString などの仕組みで国際化する。対応する文字列を strings ファイルに追加する。

シングルトンには dispatch_once()

シングルトンは本当に必要なときだけ使う。その場合、 dispatch_once() で初期化を1回だけ実行する。

frame と bounds

frame プロパティと bounds プロパティの違いを意識し、使い分ける。訳注:frameとboundsの違いなどを参照のこと。

コード全体の品質

クラスの役割はシンプルに、同じコードは一箇所に

クラスの責任を明確に定義する。分割すべきクラスはそうする。類似したコードが複数の場所に散らばっているなら、一箇所にまとめることを考える。

よい名前をつける

命名は合理的に。

どこにでもアサーションを使う

通知のハンドラメソッドは、期待するスレッドで実行されているかアサーションする。

nil にすべきでないものは、 nil でないことをアサーションする。他の値についても同様。

共有された状態は最小限に、交換可能に

共有された状態は必要なもののみにする。ミュータブルであるべきか検討する。シングルトンよりも依存性注入を使う。

まず関数、次にクラスメソッド、最後にインスタンスメソッド

ロジックはなるべく関数で実装する。そのほうが読みやすい。インスタンス変数にアクセスするほどではないが、クラス名を名前空間として使い一箇所にまとめたいロジックは、クラスメソッドで実装する。

関数で複雑なオブジェクトを操作しない

巨大なビジネスオブジェクトを関数の引数にしない。

できれば簡潔に、どんなときでも理解しやすく

簡潔さは重要だが、理解しやすさはそれに増して大事。どこに線を引くか決めて、それを守る。

短いが読みやすいメソッドと関数

メソッドと関数は短いほうがよいが、読みやすさと率直さを犠牲にしてはならない。

必要ならコメントを書く

パブリックなクラス、メソッド、関数には、それが何をするか明らかに分かるのでなければ、解説をつける。込み入った内部ロジックにもコメントを書く。

不要なデバッグ出力は消す

不要になった NSLog は残さない。必要なデバッグ出力は、それが意味するところを分かるようにする。

よいエラーロギング

エラーログには、どこでエラーが起き、何が問題だったのかを含めること。エラーログは簡潔かつ正確で役に立つようにする。