C++コーディングで心がけているポイント

これまでいくつかcocos2d-xでの記事を書いていますが、
cocos2d-xではC++,JavaScript,Luaを言語として使用できます。
弊社ではC++を使っています。
今回はC++でコーディングする際、心がけているポイントを挙げてみます。

プログラミング初心者によく教えている内容でもあります。
大規模で複雑な機能を実装していく中で、それらを正しく動作させるためには
基本的な一つ一つのことをきっちりやることが大切です。

メンバ変数は必ず初期化する

class Hoge
{
public:
    Hoge()
    : _fuga(nullptr); //※：２、初期化リストで初期化。
    {}

private:
    Fuga* _fuga; //※：１、変数を追加したら、、、
};

構造体も同様に、メンバ変数を初期化すること。

struct HogeParameter
{
    Fuga* fuga;
    int id;

    HogeParameter()
    : fuga(nullptr)
    , id(0)
    {}
};

・初期化しないと不定値が入っている場合があります。
・特にポインタは必ずnullptrで初期化する。
・コンストラクタの初期化リストを使う。
・メンバ変数を追加したら初期化リストにも記入、をペアで行うようにしましょう。

ポインタオブジェクトを参照するときはnullチェックをする

if (_hoge) {
    _hoge->func();
}

もしくは、

assert(_hoge);
_hoge->func();

nullptrにアクセスするとクラッシュします。
チェックの方法として２つ挙げました。ifとassertです。
ifによるチェックは"nullで入ってくる場合もある"という意図が、
assertによるチェックは、"この箇所ではnullでないことを前提としている"
という意図が含まれる書き方です。
ロジック的にあり得ない箇所であればチェックしなくてもよいです。
いずれにせよ、有効なポインタにアクセスしているかどうかを意識することが大切です。

コンテナを渡すときは出来るだけconst参照を使う

void hoge(std::vector<int> clothesList); //※：値渡しはオーバーヘッドがかかる

対応例：

void hoge(const std::vector<int>& clothesList); //※：const参照にしよう

参照渡しにすれば、値渡しよりもオーバーヘッドが減ります。
さらにconstをつけて、意図しない変更を防ぎます。
ユーザー定義型やオブジェクト型を渡すときも基本const参照で。std::stringやcocos2d::Point等も。
プリミティブ型は値渡しでよいです。

メンバ変数には先頭にアンダースコアをつける

class Hoge
{
private:
    int     _fuga;
};

流派によってはアンスコはつけなくてもいい、先頭でなく末尾につける、mをつける、等ありますが、いずれにせよローカル変数とメンバ変数を分かり易くするのが重要と考えます。
（構造体的な使い方のオブジェクトであればつけなくてよい、かな。。。）
チームで決めたコーディングスタイルにもよります。

ヘッダファイルのincludeは必要最小限にする

#include <Foo.h>    //※：ここでのincludeは仕方ない

class Fuga; //※：ポインタなので前置宣言でよい

class Hoge
{
private:
    Fuga*   _fuga;
    Foo     _foo;
};

・Fugaはポインタ型なので前置宣言でよい
・Fooは実体なのでincludeしなくてはいけない

#include <Fuga.h>   //※：実装ファイルでinclude

…
    _fuga = new Fuga(); //※：実体を生成
…

ファイル間の依存関係が増えるとコンパイルに時間がかかります。

using namespaceはヘッダで使わないようにする

using namespace std;    //※：string使うからといってヘッダで使ってはいけない

class Hoge
{
private:
    string  _bar;
};

こうしてしまうと、Hoge.hをincludeするところは全てnamespace stdが効いてしまいます。
実装ファイル側なら必要に応じて使ってもよいです。

ヘッダファイルのdefine定数について

#define HOGE_VALUE (100)

こうしてしまうと、Hoge.hをincludeするところは全てHOGE_VALUEが効いてしまいます。（それを意図するなら別ですが）
クラス定数かクラスのenumにしましょう。スコープを意識することが大切です。

対応例：

class Hoge
{
public
    static constexpr int HOGE_VALUE = 100;
};

もしくは、

class Hoge
{
public
    enum {
        HOGE_VALUE = 100,
    }; 
};

オブジェクトをnewする場合、生存期間を意識して必ずdeleteする

Hoge::onEnter()
{
    Node::onEnter();
    _fuga = new Fuga();
}

Hoge::onExit()
{
    Node::onExit();
    if (_fuga) {
        delete _fuga;
        _fuga = nullptr;
    }
}

この場合、_fugaの生存期間はHogeがシーンに入ってから出るまで。オブジェクトがどこで生成されてどこで破棄されるか意識して、生成と破棄が必ずペアで呼ばれるようにしましょう。
生存期間の管理がしづらい場合、スマートポインタ(shared_ptr,weak_ptr等)を使いましょう。

単語の誤字脱字に気を付ける

・クラスや変数の命名、ファイル名など、正しい単語を使うよう常に意識することが大切です。
・少しでも不安に思った場合はすぐググりましょう。
・defense? defence? など微妙な単語はチーム内で統一しましょう。

配列アクセスのインデックスチェックをする

int hoge[HOGE_NUM];
if (i >= 0 && i < HOGE_NUM) {
    hoge[i] = 123;
}
//※もしくはassert(i >= 0 && i < HOGE_NUM);

std::vector<int> fuga;
fuga.push_back(1);
fuga.push_back(2);
fuga.push_back(3);
if (i >= 0 && i < fuga.size()) {
    fuga[i] = 123;
}
//※もしくはassert(i >= 0 && i < fuga.size());

配列やコンテナのアクセスでは、インデックスが範囲内かチェックしましょう。

コンテナの要素チェックは必ず行う

std::vector<int> hogeList;
hoge = hogeList.front();

空要素へのfront()アクセスはクラッシュの原因を引き起こします。
必ず要素チェックを行いましょう。

対応例：

if (!hogeList.empty()) {
	hoge = hogeList.front();
}
//※もしくはassert(!hogeList.empty())で。

除算するときは分母の非zeroチェックを必ずする

if (maxValue != 0) {
    result = value / maxValue;
} else {
    // もしmaxValueが0だったときも考慮する
}
//※もしくはassert(maxValue != 0)で。

非zeroチェックは習慣にしましょう。

グローバル変数を作らない

const string HOGE = "fuga";

関数の外で変数を宣言するとグローバル変数になってしまいます。
staticをつけるか、無名namespaceでくくりましょう。

対応例：

static const string HOGE = "fuga";

もしくは、

namespace {
    const string HOGE = "fuga";
}
//※こちらのほうがよりC++っぽい書き方

いずれの書き方にせよ、スコープを意識することが大切です。

デストラクタのvirtualについて

ほぼvirtualをつけておけば問題ないです。

class Hoge
{
public:
    Hoge();
    virtual ~Hoge() {}

    virtual void fuga();  //※：仮想関数がメンバにあるので
};

ググってみると、つける条件・理由がたくさん出てきます。

この項目に関しては少々大雑把な言い方だと自覚はしつつも、
"デストラクタにvirtual"の記述があることで、少しでもその意味を意識してもらえるように、
という意味も込めてこれをポイントとして挙げました。
もちろん吟味した結果必要なければvirtualは無くても構いません。

cppcheck

もう一つ、cppcheckというC/C++用の静的解析ツールを紹介したいと思います。
先ほど挙げたコーディングのポイントは、意識すると良い習慣のようなものですが、
ツールを使ったシステマチックなコーディングのチェックも行なっています。

メモリリーク、バッファオーバーラン、メンバ変数の未初期化など、他にもさまざまなチェックを行なってくれます。
以下、検知の具体例を挙げます。

・メモリリーク

badcode_leak.cpp

  1 int someFunction(void)
  2 {
  3         int* ptr = new int;
  4         for (int i = 0; i < 10; i++) {
  5                 *ptr = i;
  6                 if (someCondition(i)) {
  7                         return 1;
  8                 }
  9         }
 10         delete ptr;
 11         return 0;
 12 }
 13 int main(void)
 14 {
 15         return someFunction();
 16 }

>  [badcode_leak.cpp:7]: (error) Memory leak: ptr

7行目でメモリリークが発生します。
関数から抜ける時もdeleteしましょう。

・バッファオーバーラン

badcode_over.cpp

  1 int main(void)
  2 {
  3         char buff[8] = {0};
  4         sprintf(buff, "%04d%04d", 1, 2);
  5         return 0;
  6 }

> [badcode_over.cpp:4]: (error) Buffer is accessed out of bounds: buff

4行目でbuffに対してバッファオーバーランが発生します。
終端文字用に要素数がもう一つ必要です。

・メンバ変数の未初期化

badcode_init.cpp

  1 class Hoge
  2 {
  3 public:
  4         Hoge() {}
  5 
  6 private:
  7         int _hoge;
  8 };

> [badcode_init.cpp:4]: (warning) Member variable 'Hoge::_hoge' is not initialized in the constructor.

メンバ変数がコンストラクタで初期化されていません。
先に記したように初期化リストで初期化するべきですが、それをうっかり忘れてもこのように検知できます。

おわりに

改めてポイントを書いてみると当たり前のことばかりですね。
でも当たり前のことをきっちりやることが大切なんです。
（基本的なことをし忘れたばかりに大惨事につながったことが何度かあります^^;）
コードレビューするときもこれらのポイントは最低限としてチェックしています。
レビューでの指摘を通して学び得る機会にもなります。
また、弊社ではプログラミング経験の浅い人に対しては個別でプログラミングを教えていたりもします。

みなさんも初心を忘れず良いコーディングライフを。

GCREST_engineerのブログ

ブログの説明を入力します。

C++コーディングで心がけているポイント

cppcheck

おわりに