(cache) [開発]ランクCのゴミコードを生産し続けないために、、、

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

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

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

コードの格付け

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

再利用可能なコードとは

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

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

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

タイトなスケジュールでは、どうしてもその場しのぎの「ゴミコード」を大量生産してしまいがちです。
しかし、その一瞬が苦しくても、後で楽をするために、ちょっと頭をひねらせてコードを改善する工夫が必要になってきます。
その積み上げは、数日後、数か月後に生きてきて、あなたに「定時帰り」という恩恵をもたらしてくれるでしょう。

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

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

Cランクのコード

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

レビュー内容

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

改善後のコード

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

使用例

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

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