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


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

特にチーム開発では「 自分の書いたコードが他のメンバーから使用されること 」を意識してコーディングしなければ、そこから意図せず不具合につながることもあります。 そういった意味でも、よく言われるように「コードを書く時間」よりも「コードを読む時間」が長いことは事実であり、「 自分であとから見ても、容易に把握できる 」であることがとっても大切です。

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

コードの格付け

  • ランクA ~ コメントを読まなくても使い方がわかる
  • ランクB ~ コメントを読めば使い方がわかる
  • ランクC ~ 中身を見ないと使い方がわからない

再利用可能なコードとは

格付けでいうところの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.net
命名の時にお世話になっているサービス
atuweb.net


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

CODE COMPLETE 第2版 下 完全なプログラミングを目指して

スティーブ マコネル
出版社:日経BP社  発売日:2005-03-26

Amazonで詳細を見る

この記事の著者 Webrow (うぇぶろう)
Web アプリ開発、 Web 顧問 エンジニア、WordPress サポートいたします。