STGクラス設計

[タンタル]

前回、クラス設計のことでお世話になりました。
今回も同じことでの質問になりますが、どうかよろしくお願いします。

main,cpp

コード:

#include"DxLib.h"
#include"Game.h"

// main関数の開始
int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance,LPSTR lpCmdLine, int nCmdShow )
{
	ChangeWindowMode(TRUE)	;		// debug用ウィンドウサイズ
	if( DxLib_Init() == -1 )		// ＤＸライブラリ初期化処理
	{
		return -1 ;			// エラーが起きたら直ちに終了
	}

	SetDrawScreen( DX_SCREEN_BACK ) ;	// 描画先画面を裏画面にする
	CGame Game;

	while (1) {
		ClearDrawScreen()	;	// 画面クリア

		// 本文の記述
		Game.process();

		ScreenFlip()	;		// redraw 1
		WaitTimer(17)	;		// 約60fps

		if (ProcessMessage() == -1 ) break	;	// windowsメッセージ処理
		if ( CheckHitKey(KEY_INPUT_ESCAPE ) == 1) break	;	// escキーで終了
	
	}

	DxLib_End()	;//DXライブラリ終了処理

	return 0	;
}

MyShip.h

コード:

class CMyBulletManage;

class CMyShip{
private:
	int x,y;
	enum{v=3};
public:
	CMyShip();
	void Move();
	const void Draw();
	int Getx();
	int Gety();
};

MyShip.cpp

コード:

#include"DxLib.h"
#include"MyShip.h"
#include"MyBullet.h"

CMyShip::CMyShip(){
	CMyShip::x=320;
	CMyShip::y=440;
}

void CMyShip::Move(){
	if(CheckHitKey(KEY_INPUT_LEFT)==1){
		CMyShip::x-=CMyShip::v;
		if(CMyShip::x<0) CMyShip::x=0;
	}
	if(CheckHitKey(KEY_INPUT_UP)==1){
		CMyShip::y-=CMyShip::v;
		if(CMyShip::y<0) CMyShip::y=0;
	}
	if(CheckHitKey(KEY_INPUT_RIGHT)==1){
		CMyShip::x+=CMyShip::v;
		if(CMyShip::x>640-24) CMyShip::x=616;
	}
	if(CheckHitKey(KEY_INPUT_DOWN)==1){
		CMyShip::y+=CMyShip::v;
		if(CMyShip::y>454) CMyShip::y=454;
	}
}

const void CMyShip::Draw(){
	DrawString(CMyShip::x,CMyShip::y,"▲",0xffffff);
}

int CMyShip::Getx(){
	return CMyShip::x;
}

int CMyShip::Gety(){
	return CMyShip::y;
}

MyBullet.h

コード:

class CMyShip;

class CMyBullet{
private:
	bool f;
	int x,y;
	enum{v=4};
public:
	CMyBullet();
	void Move();
	void Set(int x,int y);
	bool IsAllocated();
	const void Draw();
	int Getx();
	int Gety();
};

class CMyBulletManage{
private:
	enum{MY_BULLET_MAX=20};
	CMyBullet MyBullet[MY_BULLET_MAX];
public:
	void MoveBullet();
	const void DrawBullet();
	void SetBullet(int x,int y);
};

MyBullet.cpp

コード:

#include"DxLib.h"
#include"MyShip.h"
#include"MyBullet.h"

CMyBullet::CMyBullet(){
	CMyBullet::f=FALSE;
	CMyBullet::x=0;
	CMyBullet::y=0;
}

void CMyBullet::Set(int x,int y){
	CMyBullet::f=true;
	CMyBullet::x=x;
	CMyBullet::y=y;
}

void CMyBullet::Move(){
	CMyBullet::y-=CMyBullet::v;
	if(CMyBullet::y<0)CMyBullet::f=FALSE;
}

const void CMyBullet::Draw(){
	DrawString(CMyBullet::x,CMyBullet::y,"|",0xffffff);
}

bool CMyBullet::IsAllocated(){
	return CMyBullet::f;
}

int CMyBullet::Getx(){
	return CMyBullet::x;
}

int CMyBullet::Gety(){
	return CMyBullet::y;
}

void CMyBulletManage::MoveBullet(){
	int i;
	for(i=0;i<MY_BULLET_MAX;i++){
		if(MyBullet[i].IsAllocated()==TRUE){
			MyBullet[i].Move();
		}
	}
}

const void CMyBulletManage::DrawBullet(){
	int i;
	for(i=0;i<MY_BULLET_MAX;i++){
		if(MyBullet[i].IsAllocated()==TRUE){
			MyBullet[i].Draw();
		}
	}
}

void CMyBulletManage::SetBullet(int x,int y){
	int i;
	for(i=0;i<MY_BULLET_MAX;i++){
		if(MyBullet[i].IsAllocated()==FALSE){
			MyBullet[i].Set(x,y);
			break;
		}
	}
}

本当にもうわからないです。
どうしたってCMyShip::Shot()をprivateとして作れないです。
CGame::Process()にifなんて使いたくなかったのに、結局使わないで設計すると、すべてアクセスエラーおこします。
CMyShip::Getx(),CMyShip::Gety()だって本当なら使わずに済むはずなのに、どうしてもうまくいかないです。

弾の情報を作る、CMyBullet CMyShip::BulletMake()などを作って弾の情報を返すにしても、CMyBulletManageにその情報をまわせないです。
お互いの情報を渡せるところがCGame::Process()だけで、それでもクラスを直接引数にわたす形になってしまいます。

自機と弾のDraw(),Move()までならまだまとまるのに、Shot()が絡むとわけがわからなくなります。
どう考えたって、前回書いたコードの方がきれいだし、わかりやすいです。

個人的に考えていたまとめ方は、
void CMyBullet::Move()内で、スペースキーが押された判定を受けて、CMyBullet CMyShip::Shot()
CMyBullet CMyShip::Shot()内で、一時的にCMyBullet Temp を作り、Temp.x=CMyBullet::xとyを渡して return Temp
CMyBulletManage::SetBullet(CMyBullet &MyBullet)で、CMyBulletの空き情報を探して、MyBulletを渡す。
としたかったんですが...


前にサンプルだって見せてもらっていたのに、結局できずじまいです。
ずうずうしいのは分かっています。でも、本当に僕一人だとどう頑張ってもできないんです。
僕の作ったものを、みなさんならどう設計するか教えてください。どうかお願いします。

[へにっくす]

とりあえず変だと思うところ。

コード:

CMyBullet::CMyBullet(){
    CMyBullet::f=FALSE;
    CMyBullet::x=0;
    CMyBullet::y=0;
}

なんでいちいちクラス名書いてるんでしょうか。上の書き方は、staticな変数の場合ですよ。下のようにするか、

コード:

CMyBullet::CMyBullet(){
    f=FALSE;
    x=0;
    y=0;
}

次のようにthisキーワードを使います。

コード:

CMyBullet::CMyBullet(){
    this->f=FALSE;
    this->x=0;
    this->y=0;
}

分け方としてはこうですね。

コード:

class A {
static int member;
static int member2;
  int hensu;
  void sample();
};

void A::sample()
{
  int member; // auto変数。
  A::member = 0; // staticメンバ変数を明示(関数内のmemberと区別するため)
  member2 = 0; // staticメンバ変数だが、この関数内で同じ変数名の変数はないのでこれで十分。
  hensu = 0; // staticでないメンバ変数でも同じ変数名の変数がないならこの書き方でok
  this->hensu = 0; // staticでないメンバ変数を明示
}

[nullptr]

お疲れ様です。

ずうずうしいのは分かっています。でも、本当に僕一人だとどう頑張ってもできないんです。

いえ、とても試行錯誤されたのなら図々しくなどありませんよ。

まずパッと目についた間違いを指摘すると、関数のconst指定の仕方が間違っています。前回私が言ったので参考にして下さったんですね、ありがとうございます。しかし、constの位置が違います。例えば

コード:

const void CMyBullet::Draw(){
    DrawString(CMyBullet::x,CMyBullet::y,"|",0xffffff);
}

void CMyBullet::Draw() const {
    DrawString(CMyBullet::x,CMyBullet::y,"|",0xffffff);
}

この二つの関数は非常に似ていますが、全く意味が違います。
前者は「「const void」型を返す関数CMyBullet::Draw」で、
後者は「「void」型を返すconst指定の関数CMyBullet::Draw」です。

定数を作るときなどは前後が関係ないので(ポインタの定数は前後で意味が違うが)曖昧になりがちですが、関数では前後で意味が違うことを覚えてください。

関連して、関数のconst指定による変数の想定外の書き換えを防止する点ですが、まぁ構文を間違えたのはいいとして、CMyShip::Getx()などでもconstしていればよかったですね。
基本的に、constは付けられるなら必ずつけてしまうほうが良いです。
極端な話、一度すべてつけてしまい、エラーが出た時に本当にこの関数は書き換えが必要か考える、位でも良いかと思います。直接問題にはなりませんが、後々のバグにつながりますことですので意識してみると良いかと思います。

CGame::Process()にifなんて使いたくなかったのに、結局使わないで設計すると、すべてアクセスエラーおこします。

では、CGame::Process()を見せていただけないと、何も言えません･･･エラーが出ていて構わないので、現状最善を尽くしたコードをすべて載せていただけますか。コメントを付けていただけるとなお良いです。

[タンタル]

ヘにっくすさん 新月の獅子さん、返信ありがとうございます。
...すみません、コード一つ張り忘れていました。
今回のものですが、挙動が不安定というか、弾を出すとものすごくかくつくんです。パソコンがフリーズしたり、プログラムが落ちたりするわけではないのですが、なんだか怖いので先にそのことだけ書いておきます。

Game.h

コード:

#include"MyShip.h"
#include"MyBullet.h"

class CGame{
private:
	CMyShip MyShip;
	CMyBulletManage MyBulletManage;
public:
	void process();
};

Game.cpp

コード:

#include"Game.h"
#include"DxLib.h"

void CGame::process(){
	MyShip.Move();
	MyShip.Draw();
	if(CheckHitKey(KEY_INPUT_SPACE)==1){
		MyBulletManage.SetBullet(MyShip.Getx(),MyShip.Gety());//CMyBulletManageに自機の座標を渡しています。
	}
	MyBulletManage.MoveBullet();
	MyBulletManage.DrawBullet();
}

本当でしたら、CMyBulletとCMyBulletManageを別ファイルにするべきだったのですが、余計に混乱するだけになってしまったので、結局そのままにしてしまいました。この点はもう一度挑戦してみようと思います。

constの使い方は勉強しなおした方がいいですね...詳しく教えていただきありがとうございます。
それにGetも...関数の書き換えが頻繁にあったので、アクセス制御が頭から吹っ飛んでいました...すみません...


いちいちクラス名をかくことについてなのですが、参考サイトや参考書籍でこんな書き方してるの見たことないけど、できるならわかりやすいしつけとこって感じでやってました...、
しかし、これはstaticメンバ変数を明示するのが目的だったとは知りませんでした。前回this->を使った方がいいといわれたのですが、その意味がよく分かりました。ここももう一度直してみたいと思います。

[nullptr]

詳細をまだ見ていないのですが先に答えられる点を答えます。

今回のものですが、挙動が不安定というか、弾を出すとものすごくかくつくんです。パソコンがフリーズしたり、プログラムが落ちたりするわけではないのですが、なんだか怖いので先にそのことだけ書いておきます。

WaitTimer(17) ; // 約60fps
を消してください。DxLibは標準で垂直同期を取るので不要ですし、17msは絶対に60fps出ません。

それにGetも...関数の書き換えが頻繁にあったので、アクセス制御が頭から吹っ飛んでいました...すみません...

ええ、でしょうね。
だからこそ前述のとおり、まずすべてconstを付けてしまう、という慣習をつけると良いのです。

では詳細を見てまた返信をします。

メモ：前回のトピック。

[nullptr]

まず、前回と比べてコードが綺麗になったのではないでしょうか？

前回指摘した、描画と演算の分離、基本的にはできていますね。
せっかくならCGame::Processも描画部分を別のCGame::Drawに分けてしまっても良かったかも知れませんが、これだけでも、だいぶコードが修正しやすくなると思います。

そして、まさかのenumハック。突然だったのでびっくりしました。いいと思います。

すごく細かい点ですが、

コード:

    int i;
    for(i=0;i<MY_BULLET_MAX;i++){
        if(MyBullet[i].IsAllocated()==TRUE){
            MyBullet[i].Draw();
        }
    }

は

コード:

    for( int i=0;i<MY_BULLET_MAX;i++){
        if(MyBullet[i].IsAllocated()==TRUE){
            MyBullet[i].Draw();
        }
    }

で良いと思います。
実はこれも意味が違います、というかiにアクセスできる範囲が異なります。
今回の場合は全く関係がありませんが、前後で意味が違うこと、場合によっては後者のほうが良い場合があることは覚えておいても良いと思います。
ブロックスコープはもっと細かいレベルのアクセス制御に使ったりするので。


　＊　＊　＊

では、そうですね･･･
まず

void CMyBullet::Move()内で、スペースキーが押された判定を受けて、

移動関数がなぜ弾発生の判定をするのでしょう･･･


じゃあタンタルさんの要望を実現してみます。
まず、メイン関数から。
メイン関数ではゲームすべてを管理するクラス（今回はCGame）を作成する。
そして、メインループでプロセスを呼び出していますが、せっかくなので演算と描画は分けましょう。

コード:

class Game
{
public:
	void excuteProcess();	// ゲームの演算を処理する
	void excuteDraw();		// ゲームを描画する
};

さて、ではメイン側からは一旦これだけで充分でしょう。
次にゲームクラス内部です。

コード:

class Game
{
private:
	MyShip			myShip;
	BulletManager	managerBulet;

public:
	//
};

タンタルさんは、ゲーム内部に弾管理クラス、及び自機を含むことにしたそうなので、こうします。

ゲームの演算部で、自機と弾管理を操作します。

コード:

void Game::excuteProcess()
{
	this->myShip		->excuteProcess();	// 自機の演算処理を行う
	this->managerBulet	->excuteProcess();	// たま管理の演算処理を行う
}

次に自機の演算部です。

コード:

void MyShip::excuteProcess()
{
	this->moveShip();	// 移動処理
	
	
	if( CheckHitKey(KEY_INPUT_SPACE)==1 )
	{
		this->shotBullet();	// 弾を発射
	}
}

コード:

void MyShip::shotBullet()
{
	// ??
}

しかし、これでは自機が弾管理クラスを知らないので、shotBullet関数で手詰まりします。
これを知らせる方法は、
1)コンストラクタ
2)excuteProcessの引数
がありますが、コンストラクタで渡すということは自機のメンバ変数に弾管理へのアクセス手段を含めることになるわけです、それだと弾を発射する以外の関数からも弾登録を行う手段ができてしまう可能性があります。
ですので、引数で渡すことにしましょう。
本当はインターフェイスを引数で渡したいのですが、この話は後回しにして今は単純に弾管理クラスの参照をわたします。

コード:

void Game::excuteProcess()
{
	this->myShip		->excuteProcess( this->managerBulet );	// わたす
	this->managerBulet	->excuteProcess();
}

void CMyShip::excuteProcess( ManagerBullet& bullet_manager )
{
	this->moveShip();	// 移動処理
	
	
	if( CheckHitKey(KEY_INPUT_SPACE)==1 )
	{
		this->shotBullet( bullet_manager );	// 弾を発射
	}
}

void MyShip::shotBullet( ManagerBullet& bullet_manager )
{
	// 
}

さて、ではshotBullet関数で弾を打ちたいわけですが、今回のタンタルさんの設計だと弾は全てCMyBulletManage(ManagerBullet)が管理するようです。
弾を設定する際、自機が弾を生成するのですね。

まずは今回自機が生成する弾クラス

コード:

class MyBullet
{
private:
	bool	flagDelete;	// 削除されるべきか
	int		x;			// x座標
	int		y;			// y座標
	// ...等々

public:
	MyBullet( int x, int y );

public:
	void	excuteProcess();
	void	excuteDraw()		const;
	bool	isDelete()	const;
};

弾の生成時、座標などをコンストラクタで初期化します。
弾を生成したら、弾管理クラスにsetBulletします

コード:

class MyShip
{
private:
	//
public:
	//

private:
	void moveShip();
	void shotBullet();

	MyBullet makeBullet();
};

void MyShip::shotBullet( ManagerBullet& bullet_manager )
{
	bullet_manager.setBullet( this->makeBullet() );
}

とします。


さて、前回は引数で弾管理を渡すことができていましたね。
ですが

それでもクラスを直接引数にわたす形になってしまいます。

という事は、クラスを今の例のように渡したくないのですね。

別に、渡すことは間違いとは限りません。問題となるなら、関係ないモノも見えてしまう点ですね。
例えば。

コード:

void MyShip::shotBullet( ManagerBullet& bullet_manager )
{
	bullet_manager.excuteProcess();
}

とされてしまう可能性があるわけです。これは明らかに結果に支障が出るでしょうね。でも可能です。
でも、弾管理クラスは教えたい。なんとかアクセスできる範囲をsetBulletだけにして渡したい、というわけです。

その実現には、オブジェクト指向的プログラミングに欠かせない多態化、インターフェイスなどのキーワードを知る必要があります。
ムズカシイところなので、二つのワードを検索してまず調べて見てください。
（以前別トピックで私が回答した多態化に関することも見てみるといいかも知れません。


雑な例ですが、実際にコードになると、こんな感じです。

コード:

// インターフェイス
class InterfaceBulletManager
{
public:
	virtual void setBullet( MyBullet bullet ) = 0;	// 見せたい関数
};

class MyBulletManage
	:InterfaceBulletManager
{
public:
	void setBullet( MyBullet bullet );
};

void Game::excuteProcess()
{
	this->myShip		->excuteProcess( this->managerBulet );	// 渡せちゃう
	this->managerBulet	->excuteProcess();
}

void MyShip::excuteProcess( InterfaceBulletManager* bullet_manager )	// 要求するのはInterfaceBulletManager*型
{
	bullet_manager->setBullet(MyBullet());	// OK
	bullet_manager->excuteProcess();		// エラー！
}

このテクニックはオブジェクト指向に欠かせないテクニックです。
あえて後回しでいいと言って来ましたが、そろそろこのテクニックのありがたさの一部がなんとなくわかるのではないでしょうか。

的外れなことを言っている、あるいはわからないことがあれば言ってください。

[タンタル]

新月の獅子さん、親切な回答本当にありがとうございます。
アドバイスをもとにもう一度コードを書き直してみましたが、なかなか簡単にはいかなさそうです。
やはり弾を生成するプロセスが理解しづらいところです。自分の設計にも関わらず...

お聞きしたいことがいくつかあるのですが、とりあえずコードだけ見ていただけますでしょうか。

main.cpp

コード:

#include"DxLib.h"
#include"Game.h"

// main関数の開始
int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance,LPSTR lpCmdLine, int nCmdShow )
{
	ChangeWindowMode(TRUE)	;		// debug用ウィンドウサイズ
	if( DxLib_Init() == -1 )		// ＤＸライブラリ初期化処理
	{
		return -1 ;			// エラーが起きたら直ちに終了
	}

	SetDrawScreen( DX_SCREEN_BACK ) ;	// 描画先画面を裏画面にする

	CGame Game;

	while (1) {
		ClearDrawScreen()	;	// 画面クリア

		// 本文の記述
		Game.excuteProcess();
		Game.excuteDraw();

		ScreenFlip()	;		// redraw 1

		if (ProcessMessage() == -1 ) break	;	// windowsメッセージ処理
		if ( CheckHitKey(KEY_INPUT_ESCAPE ) == 1) break	;	// escキーで終了
	
	}

	DxLib_End()	;//DXライブラリ終了処理

	return 0	;
}

Game.h

コード:

#include"MyShip.h"//ヘッダーにヘッダーをインクルードしてもいいのか
#include"MyBullet.h"

class CGame{
private:
	CMyShip myShip;
	CMyBulletManage myBulletManage;
public:
	void excuteProcess();
	void excuteDraw()const;
};

Game.cpp

コード:

#include"Game.h"

void CGame::excuteProcess(){
	this->myShip.excuteProcess(myBulletManage);
	this->myBulletManage.excuteProcess();
}

void CGame::excuteDraw()const{
	this->myShip.draw();
	this->myBulletManage.drawBullet();
}

MyShip.h

コード:

class CMyBullet;
class CMyBulletManage;

class CMyShip{
private:
	int x,y;
	enum{v=3};	//定数はどうするべき？静的メンバ変数atatic?define?
	CMyBullet makeBullet();
public:
	CMyShip();
	void move();
	void draw()const;
	void excuteProcess(CMyBulletManage &myBulletManage);
	void shot(CMyBulletManage &myBulletManae);
};

MyShip.cpp

コード:

#include"MyShip.h"
#include"MyBullet.h"
#include"DxLib.h"

CMyShip::CMyShip(){
	this->x=320;
	this->y=440;
}

void CMyShip::move(){
	if(CheckHitKey(KEY_INPUT_LEFT)==1){
		this->x-=this->v;
		if(this->x<0) this->x=0;
	}
	if(CheckHitKey(KEY_INPUT_UP)==1){
		this->y-=this->v;
		if(this->y<0) this->y=0;
	}
	if(CheckHitKey(KEY_INPUT_RIGHT)==1){
		this->x+=this->v;
		if(this->x>640-24) this->x=616;
	}
	if(CheckHitKey(KEY_INPUT_DOWN)==1){
		this->y+=this->v;
		if(this->y>454) this->y=454;
	}
}

void CMyShip::draw() const{
	DrawString(this->x,this->y,"▲",0xffffff);
}

void CMyShip::excuteProcess(CMyBulletManage &myBulletManage){
	this->move();
	if(CheckHitKey(KEY_INPUT_SPACE)==1){
		this->shot(myBulletManage);
	}
}

void CMyShip::shot(CMyBulletManage &myBulletManage){
	myBulletManage.setBullet(this->makeBullet());
}

CMyBullet CMyShip::makeBullet(){
	CMyBullet Temp;
	Temp.setX(this->x);
	Temp.setY(this->y);
	return Temp;
}

MyBullet.h

コード:

#define MY_BULLET_MAX 20

class CMyBullet{
private:
	bool f;
	int x,y;
	enum{v=3};
public:
	CMyBullet();
	void set(int x,int y);
	void move();
	void draw() const;
	bool isAllocated()const;
	void allocate(bool f);
	int getX()const;
	int getY()const;
	void setX(int x);
	void setY(int y);
};

//クラスを別ファイルにするには、どうする？前方宣言では対応できない
class CMyBulletManage{
private:
	CMyBullet myBullet[MY_BULLET_MAX];//上の原因部分
public:
	void setBullet(CMyBullet &myBullet);
	void moveBullet();
	void drawBullet()const;
	void excuteProcess();
};

MyBullet.cpp
[code]
#include"DxLib.h"
#include"MyBullet.h"

CMyBullet::CMyBullet(){//コンストラクタで座標初期化とはどういうこと？
	this->f=false;
	this->x=0;
	this->y=0;
}

void CMyBullet::move(){
	this->y-=this->v;
	if(this->y<0)this->f=FALSE;
}

void CMyBullet::draw()const{
	DrawString(this->x,this->y,"|",0xffffff);
}

bool CMyBullet::isAllocated()const{
	return this->f;
}

void CMyBullet::allocate(bool f){
	this->f=f;
}

void CMyBullet::set(int x,int y){
	this->x=x;
	this->y=y;
	this->f=true;
}

int CMyBullet::getX()const{
	return this->x;
}

int CMyBullet::getY()const{
	return this->y;
}

void CMyBullet::setX(int x){
	this->x=x;
}

void CMyBullet::setY(int y){
	this->y=y;
}



void CMyBulletManage::moveBullet(){
	for(int i=0;i<MY_BULLET_MAX;i++){
		if(myBullet[i].isAllocated()==TRUE){
			myBullet[i].move();
		}
	}
}

void CMyBulletManage::drawBullet()const{
	for(int i=0;i<MY_BULLET_MAX;i++){
		if(myBullet[i].isAllocated()==TRUE){
			myBullet[i].draw();
		}
	}
}

void CMyBulletManage::excuteProcess(){
	this->moveBullet();
}

void CMyBulletManage::setBullet(CMyBullet &mybullet){
	for(int i=0;i<MY_BULLET_MAX;i++){
		if(myBullet[i].isAllocated()==TRUE){
			myBullet[i].set(mybullet.getX(),mybullet.getY());
		}
	}
}

全体の流れは僕の考えたとおりになってきたんですが、動かない理由がさっぱりです。
新しく作った、CMyBullet CMyShip::makeBullet()が怪しいかなと考えたのですが、ここはただデータを作るだけですし、コピーであろうと数値さえ渡せていれば大丈夫なはずなので、やっぱりCMyBulletManageから内部変数myBulletに参照型変数がうまくわたっていないせいなのかなと思っています。
ではCMyBulletManage::setBullet()のせいかと言われても、そんなはずないと思うけど...としか言えないです。

質問もいくつかあるのですが、一番の疑問がこれなのでこちらだけお願いします。お手数おかけしますが、よろしくお願いします。

[nullptr]

正しく動かない理由はCMyBulletManage::setBulletのせいです。以下のように修正してください。

コード:

void CMyBulletManage::setBullet(CMyBullet& mybullet){
	for(int i=0;i<MY_BULLET_MAX;i++)
	{
		if( ! myBullet[i].isAllocated() )	// ==TRUE では絶対に真を返しません。
		{ 
			printfDx("set\n");
			myBullet[i].set(mybullet.getX(),mybullet.getY());

			break;	// breakしないと･･･
		}
	}
}

次にコード内の質問に1つづつ答えます。

＞//定数はどうするべき？静的メンバ変数atatic?define?
enumハックを使うならそれで構いません。マクロ定数ははアクセス制御ができないので避けるべきです。


＞//ヘッダーにヘッダーをインクルードしてもいいのか
全然構いません。ただし、なんでもかんでもヘッダインクルードすればいいものではないです。ヘッダインクルードと翻訳単位でのインクルードは意味が異なってきます(順番次第で)。また、現在タンタルさんのコードでは多重インクルード対策をしていないので、前方宣言でのコードをおすすめしています。でないとおそらくその内、多重定義のエラーに悩まされるでしょう。

＞//クラスを別ファイルにするには、どうする？前方宣言では対応できない
前述のとおりヘッダでインクルードすることで可能にもなるでしょう。


＞//コンストラクタで座標初期化とはどういうこと？
これなんですが、前回のコードよりおかしくなってますね。

void set(int x,int y);
int getX()const;
int getY()const;
void setX(int x);
void setY(int y);


CMyBulletはもうアクセス制御ガン無視ですね。挙句セッタがなぜか多い。
私のコードではセッタは1つも用意しませんでした。弾は外部から位置をセットされるのですか？弾は、最初に与えられた座標から自身でmoveして移動するはずです。セッタは要りません。
最初に座標を与えるのに、セッタを使わないでください。コンストラクタで初期化しろというのはそういうことです。


インターフェイスを結局使っていない点など幾つか直せる点はありますが質問があるなら言ってください。

[タンタル]

新月の獅子さん、返信ありがとうございます。
単なるロジックエラーだったみたいですね...お手数おかけしてしまい、すみませんでした。

今回setterが異常に多くなってしまった原因の、コンストラクタを使うという部分の質問になります。
コンストラクタは、実体が作られたときに呼び出される関数だったと思います。

今回の質問なのですが、どうして弾を発射するときにCMyBulletのコンストラクタが呼び出せるのですか？
僕の設計ですと、newを使っていませんし、プログラムを起動したときに一気にコンストラクタが呼び出されてしまって、コンストラクタで自機の座標を教えるのは無理だと思うのですが...
書いてて思ったことなのですが、もしかして、CMyBullet CMyShip::makeBullet()の返り値が作られるときに、コンストラクタが呼ばれてるから平気だということでしょうか。

[nullptr]

エラーについてですが、自身で見つけられるようにしてたほうがいい設計ができるとおもいます。
というのも、オブジェクト指向的な設計は「バグを作らせない」、そして「バグを見つけやすい」という特徴があったりします。（アクセス制御もバグを作らないようにする一環ですね）。ですので、せっかくのバグなのでどうやったら今のバグを見つけやすいか、考えてみるのもよいでしょう。ちなみに今回のミスは、描画と演算を分けていたおかげで数分で見つかりました。モジュール化も、バグを見つけやすくすることにつながりますから、積極的にしていくべきですね。

今回setterが異常に多くなってしまった原因の、コンストラクタを使うという部分の質問になります。
コンストラクタは、実体が作られたときに呼び出される関数だったと思います。

今回の質問なのですが、どうして弾を発射するときにCMyBulletのコンストラクタが呼び出せるのですか？
僕の設計ですと、newを使っていませんし、プログラムを起動したときに一気にコンストラクタが呼び出されてしまって、コンストラクタで自機の座標を教えるのは無理だと思うのですが...
書いてて思ったことなのですが、もしかして、CMyBullet CMyShip::makeBullet()の返り値が作られるときに、コンストラクタが呼ばれてるから平気だということでしょうか。

この設計で実は一番残念なところって、まさにnewを使わず、固定長配列で、しかも実体の配列を保持している点なんです。
他の点から直すべきなので今はそのままでいいと思いますがね。

ただ上記の点を見逃すにしても、コンストラクタで初期化すべきですし、コンストラクタで初期化できます。
タンタルさんもわかっての通り、保持している配列は「任意のタイミングで初期化できません」。これは途轍もなく柔軟性がない設計です。
ただし、コンストラクタではなく「コピーコンストラクタ」を利用することができます。タンタルさんはなぜか、わざわざセッタを用意してテンプレートの弾の座標を渡していますが、これはセッタもゲッタも増えるただの蛇足です。テンプレートの弾をコンストラクタで初期化し、まるまるコピーしてしまえばよいだけの話です。ですから、セッタは一切必要ありません。

＞もしかして、CMyBullet CMyShip::makeBullet()の返り値が作られるときに、コンストラクタが呼ばれてるから平気だということでしょうか。
まあそんなところですかね。厳密には「平気」じゃなくて、それを利用してください、っていう感じですが。


　＊

オフトピック

ちょっと私見なのでオフトピック

＞コンストラクタで自機の座標を教えるのは無理だと思うのですが...
こういうとき、「コンストラクタで初期化できるようにするにはどう設計しようか」と考えてみたらどうでしょう。
セッタだらけのコードを見て、何も思わない内は、コードを練る意識が足りないのかなと思います。一方で、セッタだらけのコードを見て、セッタの存在意義を吟味して、もしも不要ならばどう初期化すべきで、コンストラクタで初期化するにはどうすればよいのか考える事ができる人は、きっと綺麗で作りやすいコードを書ける（あるいは、書けるようになる）と思います。「使い手に何を見せれば、使いやすいか」を考えられる人と考えられない人では、根本的な技量の差も伺えるというものです。
設定画面を開いたらものすごい量の設定情報があって、しかし実際使っているとほとんど要らないものばかりなアプリケーションと、
必要な設定だけが、わかりやすく、簡潔に、使いやすく用意されているアプリケーションでは、
やっぱり後者が洗練されたアプリケーションなんじゃないかなって私は思います。
オブジェクト指向という考え方はもっと奥の深いものですが、アクセス制御っていう基本的な点くらいは、意識していきたいですね。

[ISLe]

コンストラクタはnewを使ったときだけ呼ばれるのではありません。
クラス型のオブジェクトが構築されるときに呼ばれます。

現在のコードの具体的な流れを書くと…
CMyShip::makeBullet関数で、CMyBullet型の一時変数Tempが構築されるとき、コンストラクタが呼ばれます。
CMyShip::makeBullet関数からの戻り値は、Tempそのものではなく、Tempを引数としたコピーコンストラクタで新たに構築されます。
TempはCMyShip::makeBullet関数を抜ける時点で解体(デストラクト)されます。
戻り値も戻った先において、適当なタイミングで解体されます。

Tempの構築・解体、戻り値の構築・解体が発生しているので非常に無駄が多いと言えます。

コピーコンストラクタを使って初期化しても一時変数や一時オブジェクトは無くならないので実行効率は良くならないと思います。

ちなみに、既存の配列オブジェクトをnew演算子を使いコンストラクタで初期化することは可能です。
その方法だと、余分なオブジェクトを作らないで済ますことができます。

[nullptr]

あ、セッタが不要だという文に集中してて効率の話をし忘れてました(^p^)

[タンタル]

返事が遅くなってしまい、本当に申し訳ありませんでした。
ISLeさん、新月の獅子さん、返信ありがとうございます。

言い訳がましいですが、アドバイスを意識的に無視しているつもりはないんです。
今回は名前だけコピーコンストラクタを知っていても、使いどころや意味を理解していなかったため、コンストラクタってなんでいきなり...となってしまいました。だったらそこを聞けよ、と言われるかもしれないですが...

まぁ、なんにしても僕の責任です。結果に何もあらわれていなければ、そういわれても仕方ないですし...
何度も言われているにもかかわらず、本当に申し訳ありません。

効率の話ですが、頭の片隅にとどめておくのが限界で、コードに即座に反映するのは厳しいかもしれないです...
でも、そういったアドバイスをいただけると、調べるときのキーワードになるのでとても助かると思います。
ありがとうございます。

次にお聞きしたいことです。外部からアクセスという話がよくでるのですが、これってどういうことでしょうか。
少し思ったことなのですが、自機について、これも外部からmoveやshotを呼び出せますよね。これも不自然な気がします・
何となく自分で考えた問題点ですが、自機の移動やショットは自機が行うのではなく、キーボードからの指示で行われるはずだから、それがクラス設計に反映されていないせいなのでは？と思います。
やはり、本来ならmoveやshotもprivateにするべきなのでしょうか。

インターフェースに関することも質問があるのですが、自分の中で何が分からないかうまくまとめられないので、もう少し自分で調べてから質問させてください。
何度もお手数おかけしてしまい、本当にすみません。よろしくお願いします。

[ISLe]

次にお聞きしたいことです。外部からアクセスという話がよくでるのですが、これってどういうことでしょうか。
少し思ったことなのですが、自機について、これも外部からmoveやshotを呼び出せますよね。これも不自然な気がします・
何となく自分で考えた問題点ですが、自機の移動やショットは自機が行うのではなく、キーボードからの指示で行われるはずだから、それがクラス設計に反映されていないせいなのでは？と思います。
やはり、本来ならmoveやshotもprivateにするべきなのでしょうか。

どういう実装を考えているのか細かいところが分からないので、単語だけから受け取った意見です。

外部から指定するのは目的地です。
移動するのは自分です。
分けて考えましょう。
#ひとつのメソッドで即座に移動する実装もありますが。

『キーボードの指示』で行われると決め付けるのもいけません。
もっと分解して考えましょう。
いわゆる抽象化です。

[タンタル]

ISLeさん、返信ありがとうございます。

キーボード、という部分がまずかったということでしょうか。
外部からの入力とすればいいでしょうか？

move()内で外部からの入力を受け取るのではなく、外部からの入力を引数としてmove()に与えるということでいいですか？

すみません、個人的な事情で忙しくなり、長い間放置してしまうということになりかねないので、一度解決にして再びトピックを立てるという形にしたいと思います。
僕のために時間を割いてくださった皆様、本当にありがとうございました。
そして、ご迷惑おかけしてしまい、大変申し訳ありませんでした。

クラス間のやり取りで困っていたので、ここで質問して本当によかったと思います。
すみませんが、ここで一度失礼させていただきます。本当にありがとうございました。

[ISLe]

例えばZキーでショットを撃つと決めたとします。

だからと言ってショットを発生させるところでZキーが押されたかどうかを判断するのは良くありません。
必要なのは、ショットを発生させるべきかどうか、であり、キーが押されたかどうかではありません。

そして、Zキーが押されたら、ショットを発生させるべき、という状態に設定する機能を分離します。

例えば、仮想ゲームコントローラークラスを用意します。
4方向+斜め移動と2ボタン(A,B)といった感じに必要な入力を定義できるようにします。
仮想ゲームコントローラーに対してはショットをAボタン、ボムをBボタンと決めてプログラムしてしまってかまいません。
そして仮想ゲームコントローラーのステータスを実際のゲームパッドやキーボードからの入力に応じて変更する派生クラスなどを実装します。

そうすることでZキー以外でショットを撃つように変更するのが容易になります。
キーコンフィグの実装や、様々なゲームパッドへの対応が容易になります。
さらには、リプレイの実装も容易になります。