コードレビューの振り返り その1

研究室内でコードレビューが始まった．研究室にとっても私にとっても初めての取り組みだ．
去年2012年，私が学部4年であった年は実際の開発に重きを置いていたが，中々品質というところまで手が届くことはなかった．そのおかげで，成果物のコードはずいぶんとからまっていた．ということで，今年2013年は，内部品質の向上もしくは，研究室内プログラマーの腕を磨こう，ということでコードレビューを開始した．
ちなみに，研究室内のコードレビューにおいて，私個人の目標としては「自分の書いたコードについて，全て説明できるようになる」としている．
すでに1.5h×3回ほどコードレビューを行って，今更気がついたのだが，せっかくコードレビューを行っているので，ブログに備忘録として公開しておく．

今回の対象は，現学部4年生が1月に書いたiPhone向けアプリ内のコードの一部，あるビューを読み込んだときに呼ばれるviewDidLoadに注目した．

- (void)viewDidLoad
{
    [super viewDidLoad];
 // Do any additional setup after loading the view, typically from a nib.
    self.battingButton.hidden = YES;
    self.changeButton.hidden = YES;
    self.pitcher.hidden=YES;
    enemyscore = arc4random() % 11;
    totalEScore=enemyscore;
    [[self nanaomoteLabel] setText: [ NSString stringWithFormat : @"%d", enemyscore]];
    [[self totalELabel] setText: [ NSString stringWithFormat : @"%.f", totalEScore]];
    self.nanauraLabel.text = @"";
    self.hachiomoteLabel.text = @"";
    self.hachiuraLabel.text = @"";
    self.kyuuomoteLabel.text = @"";
    self.kyuuuraLabel.text = @"";
    self.nameLabel.font = [UIFont boldSystemFontOfSize:20];
    
    //球場
    [self studium];
}

初期化は初期化用のメソッドを用意する

viewDidLoadはあるビューが読み込まれたときに実行されるので，ひと通りの初期設定がまとめられていた．今の状態ではどんな初期設定を行っているのかわかりにくいので，きちんと初期化用のメソッドにまとめた方がいいだろう．
また，初期化用のメソッドにまとめることでビューを読み込む時以外に初期化を行うとき(リセットするとか)にメソッドを呼ぶだけでいいので，保守も簡単だ．

=の前後には半角スペースをいれる

=の前後には半角スペースをいれるのか

self.battingButton.hidden = YES;

=の前後には半角スペースをいれないのか

totalEScore=enemyscore;

どちらか一方に統一するべきだ．この2つのうち，どちらかを選ばなければならないが，そこは個人の趣味になる．私たちの中では前者の半角スペースをいれるということで一致した．その理由はスキマが空いて読みやすくなるためだ．

メソッドや変数名は小文字始まり，以降頭文字は大文字

enemyscoreという変数が存在しているが，これはenemyScoreに変更するべきだ．
大きな理由としてAppleのコーディングルールで決まっているからだ．ちなみにクラス名は大文字始り，定数は全て大文字で命名する．
個人的な落としどころとしては，その方がぱっとみた時にそのものが変数なのかメソッドなのかクラスなのかすぐわかるから，と思っている．

基本的に単語の省略をしない

totalEScoreという変数が存在しているが，totalとScoreの間にEが入っている．これはEnemyのEなのだが，その場合はtotalEnemyScoreとした方が，よく分かる．コードを描いているときはよくても，数ヶ月後には忘れてしまうのだから，誤解を招く原因や理解を阻む表現は避けるのが吉だ．

基本的にローマ字は避ける

nanauraLabel，hachiomoteLabel，hachiuraLabel，kyuuomoteLabel，kyuuuraLabelといった変数があるが，これは野球の7回裏だとか8回表のことをいっている変数だ．ローマ字だと分かりづらいので，基本的には変数名は英語でつけるようにしよう．そもそも，似たような動きをする変数は，適度コードで生成する方が管理がしやすいのではないだろうか．

同じ動作に対する記述は統一する

オブジェクトの属性に値をセットする方法として，

[[self nanaomoteLabel] setText: [ NSString stringWithFormat : @"%d", enemyscore]];

と

self.nanauraLabel.text = @"";

の2通りが混在している．これは統一した方がすっきりする．nanaurLabelに注目すると，

[[self nanauraLabel] setText:@""];

と

self.nanauraLabel.text = @"";

の2通りになるのだが，研究室内では後者で統一することにした．その理由として，前者は後者より仰々しい感じがするため，コードが見づらくなるためだ．そもそもドット演算子はコードを見やすくするためにある(らしい．自分では未確認なので今度調べることにする)ので，後者で落ち着いた．

IBOutlet系の部品はhoge+部品名で統一する

IBOutlet系の部品はコード上でみると，どうしても何が何なのか分かりづらい．ということで，すぐにわかるようにUIButtonならhogeButtonなど，後ろに部品名を付けておくといい．

パラメータな定数(マジックナンバー)はdefineしておく

enemyscore = arc4random() % 11;

の11や，

self.nameLabel.font = [UIFont boldSystemFontOfSize:20];

の20などの，パラメータな定数(マジックナンバー)を直書きするのはよくない．コードの文頭にdefineして定義しておこう．
理由として，直書きすることで修正や変更に対応するのが大変になるためだ．個人で把握できるような規模のコードならまだいいだろうが，それが数ヶ月前に書いたコード，または他人が書いたコードだったらどうだろうか．数字が何を意味しているのか，なぜその数字になったのかが全く分からない，という状態で修正や変更を加える，なんてことは考えただけで嫌になる．

コードレビューをしていて，面白い話が聞けた．アジャイルに関して，今までで1番すっきり飲み込めたのでついでに残しておく．
そもそもプログラムに修正，変更などが加わるのはなぜだろうか．プログラムは正しく作らなければならない．プログラムが正しいとは，正しい仕様にそっていることだ．正しい仕様というのは顧客が決めるもので，顧客は何らかのビジネスのためにソフトウェアを発注するために生まれる．正しいプログラムの大本，正しいビジネスとは何だろうか．様々な推測はできるだろうが，正解なんてものは常に変化するだろう．やってみなければ，世に出してみなければわからないのだ．以上から，プログラムというものはどうやったって変化してしまうものだ．だから，ソースをコピー&ペーストしてはならなくて，パラメータな定数を直書きしてはならない．
変化に対して文句を言う前に，変化に対応して柔軟なプログラムを書けるようにならなければならない．アジャイルの考えとはきっとビジネスを健全に実現するためにあるのではないだろうか．

ちなみに公開メモとってみた．