C言語とC++言語コミュのプログラムのアドバイスをお願い致します。(ぷよぷよ＠C++)

現在、CとC++を独学で勉強している者です。

以前、
http://mixi.jp/view_bbs.pl?id=58223845&comm_id=2880
にてアドバイスをもらった物をC++の勉強の為に書きなおしてみたので、また是非皆様のアドバイスを頂きたく再投稿させて頂きました。

　コードが長いのと mixi の投稿の仕様上インデントが反映されないので、アップローダーをお借りして上げさせて頂いてます。お手数おかけします。

////////////////////////////////////////////////////////////////////

http://u6.getuploader.com/zigenanime/download/47/DropGame.cpp
http://u6.getuploader.com/zigenanime/download/48/ClassPuyo.cpp
http://u6.getuploader.com/zigenanime/download/49/ClassPuyo.h

////////////////////////////////////////////////////////////////////

いくつか疑問に思ったこと

１．ぷよぷよに加えてテトリスなど他の落ちゲー動作を選択することが出来るようにする場合は、基本的な落ちゲーとしての動作を基底クラスにして、ぷよぷよやテトリスみたいな動作部分を派生クラスにすればいいのでしょうか？
　その場合ファイルの分け方は、
　・落ちゲーの動作.cpp　落ちゲーの動作.h
　・ぷよぷよ.cpp　ぷよぷよ.h
　・テトリス.cpp　テトリス.h
のように分ければいいのでしょうか？
　ファイルの良い分け方が、いまいちわかりません。

２．#include させるものは、どのようにしてヘッダファイルに書けばいいのでしょうか？
　例えば、#include<iostream> ですが、DropGame.cpp で書くべきかなと思ったのですが、どうせ "ClassPuyo.h" を #include させるし、そうすると using namespace も書く必要が出てくるので、書かなくてもいいのかなとも思いました。
　二重定義防止コードは書いてはありますし、やはりここは書くべきなのでしょうか？
　それともどんなプログラムでも基本的に使うような常套句集のような、お決まりのことが書かれたヘッダファイルを用意するという方法もありなのでしょうか？

３．メモリを new で動的に確保するのがまだ理解出来てなく今回は使ってませんが、積極的に使うべきでしょうか？（今回のプログラムに必要か必要じゃないかではなく今後の為に）
　使う場合、メンバ変数は全部動的に確保すればいいのでしょうか？

　
　上記の疑問に限らずコードの書き方、命名規則、ファイルの分け方など、どんな事でも構いませんので、良い点悪い点改善・改良点などのアドバイスが頂けたらと思います。
　よろしくお願い致します。

コメント(24)

最初
全て
最新の40件

[1] mixiユーザー 03月30日 10:25

ではまず ClassPuyo.h で気が付いたことなど。

> #define _CLASS_H_INCLUDE_

アンダースコアで始まるシンボル名は使ってはいけません。使える場所もありますが、面倒くさいのでちゃんと調べるまでは「使えない」と思っていていいと思います。

また、このシンボル名自体が他と衝突すると意味がありませんので、名前にも工夫が必要です。名前の一部にファイル名を組み込むのがお手軽でいいと思います。

二重インクルードの防止に関しては、#pragma once というのが使える処理系もあります。define による方法とこれと、両方書いておけば鉄板でしょう。インクルード・ガードを書かない、という選択肢は無いと思います。

> #define STAGE_HEIGHT 12 他

enum { STAGE_HEIGHT=12, ... } みたいにするか、
const int STAGE_HEIGHT = 12; みたいにするか。

define はスコープが制御できないし文法を無視して動作するので、C++ では記号定数の定義に使うのはやめた方がいいと思います。C でも enum を使うべきです。define で記号定数を宣言するのは過去の遺物的悪しき習慣です。

> using namespace std;

ヘッダに using 宣言を書くのは良くないです。ヘッダを include したコードの解釈が予測不明なものになってしまいます。ヘッダ内では面倒くさがらずにその都度 std:: なりを付けましょう。

> void Update ( int* , int* ) ;

仮引数名も書いておいた方が分かり易いと思います。
それと、このクラスの場合、殆どのメンバ関数は private でいいんじゃありませんか？

> int gameover ;

この変数のみ public ですが、これは private にすべきではありませんかね。
変数への public なアクセスが必要なときはそのためのメンバ関数（アクセサ）を作って、インターフェースを明確化した方がベターだと思います。

あと、

> ３．メモリを new で動的に確保するのがまだ理解出来てなく今回は使ってませんが、積極的に使うべきでしょうか？

必要なら使う、それだけです。いつでも使えるように使い方は身に付けておく必要があります。new を使う場合は delete をどうするか（いつ、どこでやるか）も同時に考えておかないといけませんね。

[2] mixiユーザー 03月30日 18:36

僕もプログラミング能力はまだまだですが自分の経験から書きます。

１．継承はあまり考えないほうがいいと思います。
継承はオブジェクト指向の話になりますが、説明するのはすごく難しいです。
僕もよくわかりませんし、継承は殆ど使ったことがありません。
はじめのうちは、クラスの中に別のクラスを定義するというやり方でいいと思います。

２．#include は確かに疑問に思うところです。
Google C++スタイルガイドは参考になるかもしれません。
http://www.textdrop.net/google-styleguide-ja/cppguide.xml#%E3%82%A4%E3%83%B3%E3%82%AF%E3%83%AB%E3%83%BC%E3%83%89%E3%81%AE%E5%90%8D%E5%89%8D%E3%81%A8%E9%A0%86%E5%BA%8F

ただ、これは超超巨大なソースコードを書く場合のGoogle流のやり方です。
全てこれに従う必要はありませんが、コーディングスタイルについてもかなり疑問が解消すると思います。

３．new について、使う場合は僕は以下の３つが考えられると思います。
?ファイルなど実行時までサイズがわからないをデータを読み込む場合
?巨大な画像データなどスタックメモリに入りきらないメモリが必要な場合
?リスト構造や木構造など特殊なデータ構造を扱う場合

僕はいずれも（他の人が作ったものを含む）ライブラリでやるべき、あるいはファイルを分割すべきだと思います。
プログラムにはバグがつきものですが、メモリは管理は特に注意が必要だからです。
また、再利用することも多いでしょうから。

[3] mixiユーザー 03月30日 21:55

OpeGameメソッド内でキー操作とドロップ操作で
その都度clock取ってるのが気になるかな＾＾

key_startとdrop_startをkey_nextとdrop_nextに変更して
次回clockの小さい方を優先する方が好きかな。＾＾

あと、clock関数がCPUの精度にどれだけか信頼性あるのかわからないけど
キー受付のクロックの記録はキー操作があった場合のみにするとか
それでCPUに負荷がかかりすぎるのであれば、キー処理後は100ms後でも
キー処理なしの場合、100ms後以降は110ms後,120ms後というように
ポーリング10msとか早めたいな。

[4] mixiユーザー 03月31日 01:46

コードを読んだので思ったことを少し書きます。間違いがあるかも・・・

クラス内のメンバ変数はm_などのプレフィックスまたはサフィックスをつけたほう良いです。
Google流は後ろに _

仮引数などの更新元、更新先はそれぞれsrc(source)、dest(destination)にしましょう。

> void CPuyo::Update ( int *target , int *up )
これは僕なら関数名を CopyStageData などとして、呼び出し側のコメントで呼び出し理由を書きます。

ClassPuyo.cppでメンバ変数の説明はいらないのでは？
コメントのコピペはコードのコピペ同様なるべく避けたほういいです。

引数に i や j を使わないで x , y にしましょう。
i, jはループのみにしたほういいです。

クラスのメンバ変数の height , width は英単語の意味は高さ、幅ですが、座標の位置として使われています。
> bool CPuyo::MoveBlock ( int height_2 , int width_2 )
この引数も同様です。block_left_position_on_field くらいですか、、、難しいですね

ClassPuyo.cpp 内の
> //表示するブロックの選択 [引数] 表示元データ
> void CPuyo::SelShowBlock ( int type )

このコメントは　ブロックを１つ表示　でいいと思います。

落下してくるブロックデータは構造体にしてまとめて扱ったほういいと思います。

[5] mixiユーザー 03月31日 20:11

<< 1 ジャンリュック藤田さん

コメントありがとうございます。

#define について理解が足りないようです。勉強しなおしてみます。
確かにクラスを必要ないのに無駄に public ですね。そう言えばと、言われて初めて気づきました(汗

>この変数のみ public ですが、これは private にすべきではありませんかね。
>変数への public なアクセスが必要なときはそのためのメンバ関数（アクセサ）を作って、インターフェースを明確化した方がベターだと思います。

無駄なメンバ関数になるかな、と思ったのですがそれよりもスコープを気にするべきですね。。。

メモリの動的確保なのですが、動的確保した際のデメリットって考えられますか？

<< 2 cpageさんのを引用させて頂きますと、、、

１．ファイルなど実行時までサイズがわからないをデータを読み込む場合
２．巨大な画像データなどスタックメモリに入りきらないメモリが必要な場合
３．リスト構造や木構造など特殊なデータ構造を扱う場合

の必要なとき以外など、必要でもないものを動的確保した場合に生じるデメリットってありますか？

[6] mixiユーザー 03月31日 20:18

>>2 cpage さん

Google C++スタイルガイドはとても参考になりました。
まだ理解出来てないところが多いですが、こういう企業の指針みたいなものが見られるとすごいわかりやすいですね。
いくつか疑問に思っていたことが解決出来ました。
ありがとうございます。

[7] mixiユーザー 03月31日 20:45

5> 必要でもないものを動的確保した場合に生じるデメリット

取り扱いが面倒くさいです。解放する時のことを考えないといけないのが特に。

それと、本来のものを格納するために必要な領域が例えば4byteだったとして、これを new して場所を取ったとすると、その場所を指すためのポインタが4byte、newが使う管理領域が8byteとか12byteとか（ここまでの話は32bitマシンを想定）余計に必要です。4byteの領域のためにその何倍もの領域を消費するのは、たぶん素直にデメリットと言っていいんではありませんかね。

[8] mixiユーザー 03月31日 20:48

>>3 あき♪ さん

コメントありがとうございます。

あき♪ さんのコメントで確認してみて気づきました。
if ( _kbhit () )　で、入力があればということなのでその前の
if ( clock () > ( KEY_TIME * CLOCKS_PER_SEC ) / 1000 + key_start )
は必要ないですね（汗

前回は、時間を取得手段がわからず if(time<20000){time++; }で時間調節していたのですが時間単位で指定したく clock関数を使ってみました。
time関数もclock関数も精度は1/100秒と載っていたので、使いやすそうなclock関数を選びました。

今の時点ではclockを毎回取る方法しか、ブロックを自動落下させる時間調整が出来ないのですが
（そのうち作ってみようと思っていたのですが、だんだん drop_time を小さくするようにすれば、
ゲームが進むにつれてブロックの自動落下をだんだん早くするというのが実現出来るかなと考えていました。）
他にこういったある一定時間事に処理をさせる良い方法はありますか？

[9] mixiユーザー 03月31日 21:11

>>4 cpage さん
2 と、まとめて返事すればよかったですね…;;;

コメントや変数の付け方は、自分が読みやすいというだけで付けてしまってました。気をつけます。
あともっと他の方のコードも読んで、読みやすいコーディングの定石みたい点を身につけた方がよさそうですね。。。

構造体もなのですが、一通りどういう機能(？)があるかはわかっているのですが、いまいち良い使い方というのがわからなくて理解が乏しいんですよね。。。
今回もとりあえず実現したいことに合いそうな物を再度調べて作ると感じでした。
例えばなのですが、構造体はどのように使うといいのでしょうか？
簡単でかまいませんので、ヒントを教えて頂けるとありがたいです。

[10] mixiユーザー 03月31日 21:16

>>7 ジャンリュック藤田さん

なるほど、そう考えれば良いのですね。
ありがとうございます。

[11] mixiユーザー 04月01日 01:05

なぜか誰もクラス設計に触れないけど．．．
せっかく class を使うのであれば CPuyo class だけでなく CStage class と CBlock class を用意し、CPuyo class は１つの CStage object と 0 個以上の CBlock object を所有する構成でクラス設計するのはいかがでしょうか？

[12] mixiユーザー 04月03日 07:32

構造体はデータ設計を考えればいいですが・・・。
RPGを考えてみると、キャラクタにはHP、MP、攻撃力や防御力などがありますね。装備にもあります。
このゲームはどういうデータ設計をしているかを意識しつつ、RPGをやったり、攻略本を読むのはどうでしょうか。
また、データーベースの正規化あたりを軽く読んでおくのもよいと思います。適当に検索してみましょう。

それから、、、悪ノリしてコードUIの部分を拡張してみました。
VC++なら実行できるでしょう。ビルドできなかったらごめんなさい。
やってみる場合は以前のコードのバックアップを忘れずに！
（一応、トピックの最初に環境を書いたほういいと思います）
わからないところばかりでしょうが、全体像がなんとなく見えれば十分です。
既存の３つのファイルに、以下のURLのCoinEx.cppとCoinEx.hとClassPuyoEx.cppとClassPuyoEx.hを加えます。

https://sites.google.com/site/cpagelinks/monooki/puyopuyokakuchou

その後、DropGame.cpp内の #include"ClassPuyo.h" を #include"ClassPuyoEx.h" に変更、
CPuyo game ;
は CPuyoEx game ;に変更。

ClassPuyo.hは
　#include <conio.h> // _getch , _kbhit
を#include "ConioEx.h"に変更、
　void ShowField () ; //画面の描写
この下に
virtual void ShowFieldEx () {};
を追加、その下のprivate: を protected: に変更。

ClassPuyo.cpp 内の void CPuyo::ShowField () メンバ関数の開始直後に ShowFieldEx () ;を追加。

以下、無事にビルド、実行できたものとしてコメントを続けます。
これでGUIウィンドウが表示されさらに、方向キーとCtrlでも操作が可能になります。
もしウィンドウがなくプロセスが生き続けていたら、タスクマネージャで強制終了してください。
既存コードをできるだけ変更しないという方針で作りましたので、全体的な設計はかなり悪いです。
実際に拡張しようとすると、問題点が色々見えてきます。

CoinEx.cppとCoinEx.hはキー入力を拡張していますが、例えば左キーを's'キーだと誤魔化しているわけです。このようなことをしないためには、例えば s キーを押したということを、一旦ぷよぷよを左に移動させる入力が発生したということに置き換える必要があります。こうすれば入力キーを変更することが容易になります。可読性も向上するでしょうが、記述量が増えているので慣れが必要です。

それから、ゲームロジックの部分と描画の部分は分離したほうようよさそうですね。
拡張したところのウィンドウへの描画は ShowField () のみで他の部分は描画をしていません。
ロジックと描画それぞれ分けてコード記述すれば、その部分のみに集中してコードを記述できます。また他の環境への移植や拡張も容易になります。可読性もかなり向上するでしょう。

今回の拡張ではWindowsAPIを使用しました。今後、OSよりの開発をしたいのならWindowsAPIに、ゲームをやりたいなら他のゲームライブラリに挑戦してみましょう。クラスにこだわるならC#やJavaを学習してみましょう。

[13] mixiユーザー 04月03日 10:14

linux環境でも使えるようにconioを作ってみたよ♪
当環境で動作確認したい人は使ってくださいw
http://pub.ne.jp/aki_onnpu/?entry_id=3574231

ただし、置換が必要♪(Ubuntu)
s/system("cls")/system("clear")/
s/"□"/"□ "/

ウィンドウとクロスプラットフォームとC++なら
wxWidgets という手もあるね＾＾

[14] mixiユーザー 04月03日 14:19

>11
そうですね＾＾そういうのいいかも♪
問題はCBlockクラスの構成かな？

CPuyo … コンソールへの描画
CStage … ぷよぷよのゲーム動作とCBlockの管理
CBlock … ブロックの削除・配置
CBlockの要素としては
・ベースベクトル
・X軸単位ベクトル
・Y軸単位ベクトル
として
Drop：ベースベクトル＋(0, 1)
回転：(X軸ベクトル, Y軸ベクトル) = (Y軸ベクトル, -X軸ベクトル)
としたらいいかな。
CBlockの書き込みは
ベースベクトル+0*X軸ベクトル + t*Y軸ベクトル
としてt = 0とt = 1の２ブロックのみをCStageに削除・配置する。
こうしておくとテトリス拡張も容易になるしw＾＾

[15] mixiユーザー 04月03日 22:21

私だったら以下のような感じかな。
CPuyo：ぷよぷよのゲームの制御
　　　　ステージの生成とCBlockの管理（生成／移動／消滅）と描画指示
CStage：ステージ
　CStage の要素としては draw() isInnerArea()
CBlock：ブロック
　CBlock の要素としては move() getPos() getColor() draw()
CMon：コンソール描画
　CMon の要素としては textOut() flush()

[16] mixiユーザー 04月04日 22:02

>>11.15. Sahmaro さん
なるほど。関数単位でしか分けようと考えていませんでした。
クラスごとにわければ拡張も多用途に再利用も容易になりますよね。。
今後気をつけてみます。ありがとうございます。

[17] mixiユーザー 04月04日 22:53

12. cpageさん

開発環境は、前回書き忘れて今回もまた書き忘れてしまいました、、失礼しました。
Windows7(boot camp)
Visual C++ 2010
です。ところで環境を言う場合ってこの２つで十分なのでしょうか…？

GUIウィンドウで動きました！
理解出来ないところばかりですが、コードを追いながら勉強させて頂きます。

そうですね、最終的にやりたいことは Objective-C でiOSのアプリを作りたいのですが、せっかく独学で一からやるので
プログラムの初歩や概念的なものからじっくりやっていきたいと思い、今に至ります。

では、次はWindowsAPIも合わせて勉強して行こうと思います。
ありがとうございます。

[18] mixiユーザー 04月04日 23:04

13. あき♪ さん

wxWidgets というのもあるのですね。知らないことばかりで、勉強になります。
ありがとうございます。

[19] mixiユーザー 04月05日 01:00

> 16
独学は大変だよね。特に何て読むかがわからなくて困りますよね。
参考になるかわかりませんが、15 ベースで書いた code を示しますね。
（完成していませんが、Block 生成～消滅部分は書いています）

http://www2s.biglobe.ne.jp/~sahmaro/DropGame.zip

あと、確か前回も指摘したかもしれませんが
// char key ;
// key = _getch () ;
int key;
key = _getch();
ですね。

[20] mixiユーザー 04月07日 22:42

>>19 Sahmaroさん

じっくりコードを見て勉強させて頂きます。
ありがとうございます！

> // char key ;
> // key = _getch () ;
> int key;
> key = _getch();
> ですね。

見落としてました、、、。
ところでこれがいけない理由は何なのでしょうか？
私が考えられる問題点は…

1. 2バイト文字が受けられないから。
・けど仮に実行中キーボードで日本語入力しても、コンソール上に打たれるだけで何も反応がない。(逆になぜ反応がないかも疑問ですが。)キーボードからの1バイト文字だけを受けるのを想定してるから大丈夫？
ただ予期せぬバグを避けるためにも、わざわざcharを使う理由はないと思う。

2. int(32bit)の方が処理効率がいいから？

どうでしょうか？

[21] mixiユーザー 04月08日 11:13

>20
妄想する前にマニュアル読めよ

[22] mixiユーザー 04月08日 22:46

>)21 sock/vuelaさん

はい、わかりました。
コメントありがとうございます。

[23] mixiユーザー 04月08日 22:53

まぁ、ライブラリは作った人に聞く（マニュアル）のが一番だよねw＾＾
私の初めてのプログラミングはBASICのマニュアルだったなw
ABC順に試していった♪w

私も14で作りかけ～
http://www16.plala.or.jp/aki_onnpu/Puyo.zip

できるの分かったら面倒になったのでw
興味のある人は自由に引き継いで作ってくれてもいいよ♪
その時は公開してね♪

[24] mixiユーザー 04月26日 17:55

>>20 あき♪
返信遅くなりました。
参考にさせて頂きます。ありがとうございます。

ログインすると、みんなのコメントがもっと見れるよ