病みつきエンジニアブログ

機械学習、Python、Scala、JavaScript、などなど

もっともっと良いコーディングをするための勘所8つ

先日とあるコードレビューを拝見することがあったのですが、それにインスパイアされて記事を書いてみます。レビュワーの方が言ったことも含んでいますが、それと必ずしも一致するものでもありません。

Objective-Cのコードで書いていることが多いですが、わりと一般論だと思います。

photo by Hugo-photography

命名規則は言語の「普通」に任せる

例えば、Objective-Cだと変数にはcamelCaseを使うことが多いです。逆にRubyではsnake_caseを使ったりします。もしくは、略語を使うとか使わないとか、そういう違いもあります。

変数名に対してどういう書き方をするかというのは、個人の好みではなく、言語の慣習に任せるのがいいのではないかと思います。

言語の慣習の調べ方は、Githubで「stars:>100」と検索して、言語を絞るといいでしょう。(参考:Rubyの例)

ちなみに、著名なコーディングスタイルガイド(Google Style Guideなど)を参考にするのも一つの手ですが、「その企業にとっての主観」であることもあるように感じるので、僕はあまり好きではないです(言い換えると、著名なコーディングスタイルでも「え?」となることがあります)。

メインのスレッドをブロックしない

具体的に言えば、HTTP通信を同期通信にしない、などです。これはAndroidではそれ自体が仕様という感じなのですが(UIを止めると落ちるって聞いた気がする、ちょっとあやふや)、Objective-CJavascriptでは普通にできるので、やってしまった過去があります。

他には、計算量が高い画像処理なども、これに該当します。

ちなみに、Obj-CでのHTTP通信はAFNetworkingMKNetworkKitという素晴らしいライブラリがあるので、積極的に使いましょう。

早期リターン

これは単純なテクニックですが、

void something() {
  if (condition) {
    // 何かがOKなときは、いくつかの処理をする
  }
}

とするのではなく、

void something() {
  if (!condition) {
    return;// 何かがNGだったら、早期リターンする
  }
  // いくつかの処理をする
}

ということです。そうすることで、処理の部分のインデントが浅くなります。returnをすると関数はそこで処理が終わります。

パッケージングシステムを使う

RubyでいうGem、JavaでいうMaven(使ったことない)、Objective-CならばCocoaPodsなどを使いましょう。

CocoaPodsのような非公式なパッケージングシステムが好きではない場合でも、git submoduleを使うなどすると良いと思います。自分の書いたソースコードと、ライブラリや別リポジトリを切り分けるのには明確なメリットがあります。例えば、ライブラリの更新がコマンド一つでできるようになったりします。

関数やクラスを積極的に切り分ける

「意味的に正しくなるように切り分けましょう」と言いたいところなのですが、慣れないと難しいので、アグレッシブに切り分けましょう。

これに関しては、言語や宗派によって諸説あるところですが、「1クラス50行以下」「1関数のインデントは1つまで」といったルールを先に決めておくことで、よりオブジェクト指向で書けることがあります。

関数呼び出しのオーバーヘッドなどが気になるところですが、それが障害になることは極めて少ないです。

参考

ソースにコメントアウトしたコードを残さない

// good_old_fashioned_process();

あるコードが鬱陶しくなったとき、それをコメントアウトしてしまいがちです。一時的であれば良いのですが、それ自体をソースコードに残し続けるべきではありません。使わなくなったコードは、適切にバージョン管理をすることで、過去のバージョンに残しておくことができるはずです。

特に、こういうコメントアウトは、意図が不明瞭になりがちです。ちゃんとソースコードから削除して、その意図をコミットメッセージに書いて、バージョン管理しましょう

警告は削除する

特にハッカソンなどでは無視しがちですが、警告は思った以上に重要な意味を持っていることがあります。警告を消すことで何かの問題を解消できるかもしれません。

特にコンパイル型の言語では文字列の埋め込みを避ける

次のソースのように、文字列で比較したり、文字列でファイルを呼び出すことがあります。

if ([input_text isEqualToString:@"human"]) {
    button.image = [UIImage imageNamed:@"japanese.png"];
}

これは、コンパイル型言語のいいところを半減させてしまいます。文字列は、コンパイル時に評価してくれません。つまり、スペルミスをしてもコンパイル時には気づかない、ということです。補完もしてくれない、という意味でもあります。文字列はコンパイラには解釈できないことが多いですから、文字列の変更もミスが発生しがち、と言うこともできます。

もしくは、「マジックナンバーを避ける」というのを、もっと進めた考え方、ということもできるかもしれません。埋め込んだ文字列やファイル名を、より説明的にする、ということです。

上記のような文字列の比較や呼び出しが、複数にまたがる場合は特に、DRYの観点からも定数化したほうがよいです。

if ([input_text isEqualToString:BioCategoryHuman]) {
    button.image = [UIImage imageNamed:ASSET_HUMAN];
}

※もちろん、必ずどこかには文字列を書く必要はあります。それをどこかに潜り込ませたり、繰り返したりしない方が良い、ぐらいの意味合いです。

※上の例は、もっとオブジェクト指向に出来る余地があります。

で、実践出来てるのか?

くっ・・・・! って感じです。なので、積極的にまさかりを投げ合っていきましょう。それでは。