過去お世話になったガツガツバリバリなエンジニアの方がおっしゃっていたことを記事にさせていただきました。


職業プログラマはタイトなスケジュールで開発することが多く、時間を意識し、生産性の向上を意識することはとても大切なことです。

特にチーム開発では「 自分の書いたコードが他のメンバーから使用されること 」を意識してコーディングしなければ、そこから意図せず不具合につながることもあります。

そういった意味でも、よく言われるように「コードを書く時間」よりも「コードを読む時間」が長いことは事実であり、「 自分であとから見ても、容易に把握できる 」であることがとっても大切です。

1 か月前に書いた自分のコードですら、覚えてられるエンジニアはいません!

コードの格付け

ランク A コメントを読まなくても使い方がわかるコード
ランク B コメントを読めば使い方がわかるコード
ランク C 中身を見ないと使い方がわからないコード
ランク D 中身を見ないと使い方がわからないコード
ランク E 中身を見るきにも慣れないとんでもないコード

再利用可能なコードとは

格付けでいうところの A、B ランクのコードが「 再利用可能なコード 」です。

ランク C は、都度コードの中身を調べる確認作業が発生し、生産性向上には貢献しません。 そのため、ランク C は「 生産性を落とすゴミ 」と言われます。

全てのコードがランク A で構成されれば良いのですが、ランク C のコードがほとんど、というのが実際のところです。

タイトなスケジュールでは、どうしてもその場しのぎの「ゴミコード」を大量生産してしまいがちです。

しかし、その一瞬が苦しくても、後で楽をするために、ちょっと頭をひねらせてコードを改善する工夫が必要になってきます。

その積み上げは、数日後、数か月後に生きてきて、あなたに「定時帰り」という恩恵をもたらしてくれるでしょう。

それに、苦労して実装したコードがすべてゴミだなんて、、、悲しすぎるじゃないですか。

C ランクのコードと改善例

C ランクのコード

1// C++
2//
3// AppMstCard->id を検索して最初に一致したクラスを取得
4// @param id ID
5//
6// @return AppMstCard*
7jsonp::_card getMstID(int id);

レビュー内容

  • getMstID() をそのまま解釈すると「マスター ID を取得する」関数かと思うが、戻り値の型が構造体 jsonp::_card である
  • DB の検索を行う関数で処理コストがかかるため、get() という関数名が不適切である
    get() は単純なゲッターで、軽い処理であることを連想する
  • 引数に一致するデータが存在しない場合、構造体 jsonp::_card の id に 0 をセットする仕様となっており、コードを見ないとわからない実装である
    こういった場合は無効値を返すことが妥当

改善後のコード

1std::experimental::optional<jsonp::_card> findMasterCardById (int masterCardId);

使用例

1auto card = findMasterCardById(100);
2if (card) { // 無効値のチェック
3    std::cout << "カードの名称" << (*card).name << std::endl;
4} else {
5    // 存在しないIDだった時の処理
6}

クラス名や関数名には十分気を使い、なるべく齟齬が少ないものを選択するべきです。
当たり前ですけど。当然ですけど。

命名の時にお世話になっているサービス
良いコードは良い命名から。私がプログラミングで命名に困ったときによく利用するサービスを紹介いたします。
atuweb 開発ブログ

良いコードを書くならば、リーダブルコード、コードコンプリートは必読ですね。