リーダブルなコードを目指して：コードへのコメント（1）

リーダブルコードの解説を読んだ大木さんから次のような連絡をもらいました。

私はプログラマーなのですが、まだまだ未熟で、人が読めるコードを書くことができません。本の内容を意識しコーディングしてみましたが、自信がありません。そこで、添削していただきたいと考え、メールを送りさせていただきました。

リーダブルコードの解説に「私にコードを見せてみるといい」と書いていたので連絡してくれたとのことです。解説を書いてから6年経ちますがこのような連絡をもらったのは初めてです！うれしいですね。

大木さんから提供してもらったコードは https://github.com/yu-chan/Mario です。そこそこ量があるので何回かに分けて少しずつコメントします。

なお、私と大木さんだけでやりとりしてコメントをするのではなく、誰でも参照できるところでコメントするのは大木さんと合意しています。できるだけ多くの人と情報を共有したいという私の希望もある（リーダブルコードの解説がCreative Commonsなのも同じ理由）のですが、大木さんにとっても私以外の人からもコメントをもらえる機会がある方がよいと考えたからです。ということで、この記事を読んだ人で興味がある人はコメントしてみてください。この記事のように文章でまとめて https://github.com/yu-chan/Mario/issues で大木さんに連絡するのもよいですし、リポジトリー上のコードに直接コメントするのでもよいでしょう。

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

注意点は次の通りです。

悪いところ探しではない
- 目的はどんなコードがリーダブルだろう？どうしてリーダブルなんだろう？を共有することです。既存のリーダブルなところを見つけてそれを明文化することはそのためにすごく大事なことです。
- 悪いところ探し「だけ」をしたい人はコメントしない方がよいでしょう。
自分の考えを押し付けることは大事ではない
- リーダブルかどうかは読む人によって変わります。「自分」がリーダブルなら他の人もリーダブルであるべきだ、ということではありません。
- コメントする中でいろんな視点に気づき、いろんなリーダブルがあることを知ることが大事です。それは自分がリーダブルなコードを書くときにも使えますし、チームのリーダブルを見つけるときにも使えるはずです。
  - チームでリーダブルなコードを書きたい人はチームでリーダブルなコードを書くためのワークショップも参考にしてください。
- 自分の考えを押し付けたい人はコメントしない方がよいでしょう。

README

私はこのコードの前提知識がないので、まずはREADMEから確認します。

トップレベルのREADME.mdは次のようになっています。

"# Mario"

どこから手を付ければよいかのヒントを期待したので期待と違いました。

トップレベルには次のファイル・ディレクトリーがありました。

Mario/
Mario.sln
README.md

拡張子が.slnのファイルがあるのでVisual Studioでビルドするんだな、ということがわかります。README.mdと.slnのファイルを除くとあとはMario/ディレクトリーしかないので、このディレクトリーを見てみましょう。

このディレクトリーの下のファイルの拡張子の多くは.cppと.hです。C++で実装していることがわかります。このディレクトリーにもREADMEがあるので見てみましょう。

1. 開発環境

OS       : Windows7/10
(ゲーム作成したときの環境に7、10を使用していた。
もしかしたら8でもいけるかもしれない。)
OSビット : 64ビット
IDE      : Visual Studio Express 2012 for Windows Desktop

まず開発者が使っている環境が書いてあります。これはすごくよいですね！この環境なら動くということがわかるので、読む人にとって非常に助かる情報です。

また、「もしかしたら8でもいけるかもしれない」と「やっていないこと」についても書いているのがよいです。コードのコメントに書くときもそうですが、「やっていないこと」についての情報は有用なことがあります。「やっていない」条件で使わないといけなくなったときにどう対応すればよいかの指標になります。「いけるかもしれない」であれば、「動くなら動いた方がよい」ということがわかります。この場合なら、Windows 8で動くことを確認できたらここの記述を更新すればよいし、動かなくても簡単に動くようにできそうなら頑張るということがわかります。

「やっていないこと」と同じように「やらないこと」もあれば書いてあると読む人の役に立つことがあります。たとえば、「Windows 8はサポートしない」と書いていれば、Windows 8用の作業はしないと判断できます。

READMEのこれ以降の説明を読むとDXライブラリを使っていることがわかりました。DXライブラリのURLも書いているのでDXライブラリのサイトにすぐにアクセスできました。サイトには次のような説明があるのでゲーム開発用の便利ライブラリーだということがわかりました。

DirectXを使ったWindowsデスクトップアプリの開発に必ず付いて回るDirectXやWindows関連のプログラムを使い易くまとめた形で利用できるようにしたＣ＋＋言語用のゲームライブラリ

コードの構成の説明は特にないのでメイン関数（プログラムの処理を始める関数）から見ていくことにします。手元で動かせるならまずは動かすのですが、私の環境はWindows 7/10ではないので今回は動かさずにコードを見るだけにします。

メイン関数

どこにメイン関数があるか探し方はいろいろあります（たとえばmainで検索する）が、ファイルを眺めていたらmain.cppという名前のファイルがあったのですぐにわかりました。よい名前ですね。

メイン関数をmain.cppという名前をつけるのはよくあるやり方です。よくあるやり方に合わせるのは理解しやすくなる人が増えるのでリーダブルという観点でよいやり方です。

他によくあるやり方は実行ファイルと同じ名前を使うやり方です。今回はMario.exeができると思うのでMario.cppという名前にするという感じです。

main.cppの中は次のようになっていました。上から順に見ていきましょう。

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow) {
	ChangeWindowMode(TRUE);
	SetWindowSize(WINDOW_WIDTH, WINDOW_HEIGHT);

	if(DxLib_Init() == -1) {
		return -1;
	}
	SetDrawScreen(DX_SCREEN_BACK);

	srand((unsigned)time(NULL));

	//フレームレート制御
	if(!Framerate::instance()) {
		Framerate::create();
	}
	if(!SoundManager::instance()) {
		SoundManager::create();
	}
	if(!Sequence::Parent::instance()) {
		Sequence::Parent::create();
	}

	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::destroy();
	SoundManager::destroy();
	Framerate::destroy();
	DxLib_End();

	return 0;
}

最初の行で「ん？」と思いました。

	ChangeWindowMode(TRUE);

なぜ「ん？」と思ったかというと、「TRUEだとどんなモードということなのか」ということがわからなかったからです。ChangeWindowMode()について調べるとDXライブラリが提供している関数でした。DXライブラリには「ウインドウモード」と「フルスクリーンモード」というモードがあるらしく、ChangeWindowMode(TRUE)は「ウインドウモードにする」ということでした。

説明を読んだ結果、「この行はDXライブラリのことを知っていればわかるんだろうなぁ」と思いました。おそらく、このコードを読む人はDXライブラリのことを知っているべきだと思う（前提知識としてもよさそう）のでこのままでもよいと思いました。（前提知識によってリーダブルなコードがどんなコードかは変わってくるので前提知識をどこに置くかは大事です。）

ただ、もしDXライブラリのAPIが次のようになっていればDXライブラリのことを知っていなくてもわかりそうだなぁと思いました。

	UseWindowMode();

もし、DXライブラリをすでに使っている人でも同じような気持ちになるなら、DXライブラリにフィードバックしてよりよいAPIを模索するのがよさそうだなぁと思います。

DXライブラリを知らない人でも「ん？」と思いにくくするならアプリケーション内でUseWindowMode()という名前の関数を定義してそれを使うという方法があります。こうすればUseWindowsModeという名前から「あぁ、ウインドウモードというものを使うんだな」ということがわかります。「ウインドウモード」というのがわかりにくそうなので、DisableFullScreenとして「あぁ、フルスクリーンじゃない（普通のウインドウを使う）んだな」というコードにするのもよいでしょう。

static int UseWindowMode() {
	return ChangeWindowMode(TRUE);
}

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow) {
	UseWindowMode();
	// ..
}

それでは次の行を見てみましょう。

	SetWindowSize(WINDOW_WIDTH, WINDOW_HEIGHT);

これもDXライブラリが提供する関数で、ウインドウモードの設定をするものでした。DXライブラリを知らない人でもわかりやすくするならこれも合わせて次のようにするとよいと思います。

static void InitializeWindowMode() {
	ChangeWindowMode(TRUE);
	SetWindowSize(WINDOW_WIDTH, WINDOW_HEIGHT);
}

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow) {
	InitializeWindowMode();
	// ..
}

次のようにコメントで補足する方法もありますが、今回のケースでは関数に切り出す方がよいでしょう。関数に切り出すと読む人は詳細を知らなくてもよくなるからです。「あぁ、まずウインドウモードというやつを初期化するんだなー」くらいの理解でよいなら関数に切り出した方がよいです。「まずウィンドウモードというやつを初期化するんだな。そして、それはこういう内容なんだな。」というところまで知っておいた方がよいなら切り出さずにWinMain()の中に書いておいた方がよいです。

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow) {
	// ウインドウモードの初期化
	ChangeWindowMode(TRUE);
	SetWindowSize(WINDOW_WIDTH, WINDOW_HEIGHT);
	// ..
}

SetWindowSize()の引数を640, 480と書かずに、WINDOW_WIDTH, WINDOW_HEIGHTと書いているのはよいですね！関数名がSetWindowSizeなので引数が幅と高さだろうということはすぐにわかるので640, 480でもそんなに悪くないのですが、「他にもウインドウサイズに関係している処理があったらどうしよう。ここの値だけを変更するのでは変更漏れがあるかも。」という気持ちになるので、値に名前をつけておくとリーダブルになります。

リーダブルかどうかの判断基準の大きな部分は「そのコードがなにをやっているかがすぐにわかるかどうか」ですが、「そのコードを読んでもやっとしないかどうか」も大事な要素です。「もやっとする」と他のところが気になってしまって集中できません。そうすると次のコードに進みにくくなってリーダブル度が下がってしまいます。

まとめ

リーダブルコードの解説を読んで「自分が書いたコードにコメントして欲しい」という連絡があったのでコメントをはじめました。

まだ、READMEとメイン関数の2行目までしかコメントしていませんが、引き続きコメントしていきます。「リーダブルなコードはどんなコードか」を一緒に考えていきたい人はぜひ一緒にコメントして考えていきましょう。

ククログ

README

メイン関数

まとめ

関連情報