db tech showcase Tokyo 2018やApache Arrow 0.11.0のリリースとかしていたら2ヶ月ちょい経ってしまっていた須藤です。

リーダブルなコードを目指して：コードへのコメント（4）の続きです。前回はフレームレート関連の処理のところを読んでコメントしました。

リポジトリー： https://github.com/yu-chan/Mario

今回のコメントに関するやりとりをするissue： https://github.com/yu-chan/Mario/issues/5

ゲーム本体

今回はついにゲーム本体の処理を見ていきます。ゲーム本体の処理はメインループからはじまるのでまずはメインループの中をおさらいします。

	while(!InputInterface::isOn(KEY_INPUT_Q)) { //Qを押したら終了
		if(ProcessMessage() != 0) {
			break;
		}
		InputInterface::updateKey();
		Framerate::instance()->update();
		ClearDrawScreen();

		//ゲーム開始
		Sequence::Parent::instance()->update();

		ScreenFlip();
		Framerate::instance()->wait();
	}

「ゲーム開始」というコメントがあるのでSequence::Parentを見ていきましょう。

定義はSequence/Parent.hにありました。名前空間とパスが一致していて探しやすいですね。

#ifndef INCLUDED_SEQUENCE_PARENT_H
#define INCLUDED_SEQUENCE_PARENT_H

namespace Sequence {

class Child;

class Parent {
public:
	static void create();
	static void destroy();
	static Parent* instance();

	void update();

	enum NextSequence {
		NEXT_TITLE,
		NEXT_GAMEOVER,
		NEXT_ENDING,
		NEXT_GAME,

		NEXT_NONE
	};
	void moveToNextSequence(NextSequence);
	
	void deleteChild();

private:
	Parent();
	~Parent();
	static Parent* mInstance;

	NextSequence mNextSequence;
	Child* mChild;
};

}

#endif

以下の部分はシングルトンパターンを実現するためのコードなので今回は無視しましょう。

	static void create();
	static void destroy();
	static Parent* instance();

	Parent();
	~Parent();
	static Parent* mInstance;

ということで注目するのは以下の部分です。

namespace Sequence {

class Child;

class Parent {
public:
	void update();

	enum NextSequence {
		NEXT_TITLE,
		NEXT_GAMEOVER,
		NEXT_ENDING,
		NEXT_GAME,

		NEXT_NONE
	};
	void moveToNextSequence(NextSequence);
	
	void deleteChild();

private:
	NextSequence mNextSequence;
	Child* mChild;
};

}

メインループはupdate()を呼んでいるだけだったので、update()の中でmoveToNextSequence()やdeleteChild()を呼んでいるのでしょう。

名前から想像するとそれぞれのメンバー関数が実現する機能は次の通りです。

• update()

  • ループ毎に適切な次のなにかを実行する。キャラクターを動かすとか。

• moveToNextSequence()

  • NextSequenceがNEXT_TITLEとかNEXT_GAMEOVERなので、ゲームの状態（開始とか終了とか）の遷移をするんだろう。

  • Sequenceという名前空間はこのNextSequenceのSequenceと同じ意味なの？

  • 同じ意味ならSequence::Parent::NextSequenceというように複数回出現するのは冗長かなぁ。

  • 違う意味なら紛らわしいので違う単語を使うとわかりやすくなりそう。

• deleteChild()

  • 子どもを削除？

  • このクラスはParentでclass Child;という前方宣言もあるのでなにかしら親子関係がありそうだけど、なにとなにが親子関係になっているんだろう。

私はゲームを作らないのでゲーム固有の用語をよく知らないのですが、シーケンスという名前や（なにかわからないけど）親子関係にするのは普通なのかもしれません。実装も読んでみましょう。

update()は次のようになっています。

void Parent::update() {
	mChild->update(this);

	switch(mNextSequence) {
		case NEXT_TITLE:
			deleteChild();
			mChild = new Title();
			break;
		case NEXT_GAMEOVER:
			deleteChild();
			mChild = new GameOver();
			break;
		case NEXT_ENDING:
			deleteChild();
			mChild = new Ending();
			break;
		case NEXT_GAME:
			deleteChild();
			mChild = new Game::Parent();
			break;
	}

	//処理をしておかないと、次へ進めない
	mNextSequence = NEXT_NONE;
}

メインの処理はmChildが扱うようです。update()ではなにもしていませんでした。

caseの中を見るとそれぞれのシーケンス（状態？）毎にmChildのオブジェクトが変わっています。NEXT_TITLEならTitleオブジェクトになっていますし、NEXT_GAMEOVERならGameOverオブジェクトになっています。

update()内でmNextSequenceが変わっていないのが気になりますが、たぶん、deleteChild()内で変えているのでしょう。

caseでNextSequenceのすべてをカバーしていないことが気になりました。NEXT_NONEだけがないのですが、おそらく、NEXT_NONEのときはなにもしないから省略したのでしょう。

caseですべての値をカバーしていないとコンパイラーが警告を出すはずなので私は書いておいた方がよいと思います。次のような感じです。

	switch(mNextSequence) {
		case NEXT_TITLE:
			// ...
			break;
		case NEXT_GAMEOVER:
			// ...
			break;
		case NEXT_ENDING:
			// ...
			break;
		case NEXT_GAME:
			// ...
			break;
		case NEXT_NONE:
			// Do nothing
			break;
	}

case NEXT_NONE:を使わずにdefault:を使う方法もあります。

	switch(mNextSequence) {
		case NEXT_TITLE:
			// ...
			break;
		case NEXT_GAMEOVER:
			// ...
			break;
		case NEXT_ENDING:
			// ...
			break;
		case NEXT_GAME:
			// ...
			break;
		default:
			// Do nothing
			break;
	}

私は使わなくて済むときはdefault:を使わないようにしています。そうすると、新しいenum値を追加したときにコンパイラーが（警告という形で）教えてくれるからです。これにより新しいenum値の対応漏れを減らせます。

たとえば、NextSequenceにNEXT_CONTINUEを追加して次のようになったとします。

	enum NextSequence {
		NEXT_TITLE,
		NEXT_GAMEOVER,
		NEXT_ENDING,
		NEXT_GAME,
		NEXT_CONTINUE,

		NEXT_NONE
	};

default:を使っていると既存のswitchで特に警告はでませんが、使っていない場合は「NEXT_CONTINUEの処理がないよ」とコンパイラーが教えてくれるはずです。

それではdeleteChild()を見てみましょう。

//画面遷移する際に、今の画面を破棄する
void Parent::deleteChild() {
	delete mChild;
	mChild = 0;
}

mChildを解放しているだけでした。mNextSequenceはParent内では変えないようです。

mChild = 0はC言語ではNULLを代入しているのと同じことですが、C++では0よりもnullptrを使った方がよいです。整数と区別できますし、ヌルポインターを使いたいという意図を表現できます。

mChild = nullptr;

ただ、C++11からしか使えないので古いコンパイラーもサポートしないといけないときは使えません。

一応、moveToNextSequence()も見てみましょう。

//画面遷移を更新
void Parent::moveToNextSequence(NextSequence next) {
	mNextSequence = next;
}

mNextSequenceを更新しているだけですね。外のオブジェクトからこのメソッドを呼んで状態を更新するのでしょう。でも誰がどこから？Sequence::Parentはシングルトンパターンになっているのでどこからでも更新できます。mChildのオブジェクトの実装を見ていくと見つかるでしょう。

Sequence::Parentは以上です。今回はわからないことが多かったです。どうしてSequenceという名前空間なんだろうとか、どうしてParentというクラス名なんだろうとかです。ParentとmChildではなく、もう少し具体的な名前をつけられないでしょうか。たとえば、ScenarioとmStageとかです。今後、mChildのオブジェクトになるクラスを見ていくとなにかよい名前が浮かぶかもしれません。

まとめ

リーダブルコードの解説を読んで「自分が書いたコードにコメントして欲しい」という連絡があったのでコメントしています。今回はついにゲームの処理に入りました。しかし、まだわからないことが多く、名前がピンときていません。次回はより具体的な処理を1つ選んで読んでいきます。

「リーダブルなコードはどんなコードか」を一緒に考えていきたい人はぜひ一緒にコメントして考えていきましょう。なお、コメントするときは「悪いところ探しではない」、「自分お考えを押し付けることは大事ではない」点に注意しましょう。詳細はリーダブルなコードを目指して：コードへのコメント（1）を参照してください。

ツイートする

フォローする