クラスと管理クラスの関係

[タンタル]

初めまして。早速ですが質問させていただきます。
適当な画像(今回は例として星にします)が10フレームおきに、右端から左へと流れていくというプログラムを作ろうとしています。
星クラスのインスタンスを配列で作ります。フレームのカウントはグローバル変数に任せます。時間が来たら、空いているインスタンスのフラグを立てて出現させ、画面端についたらフラグ消滅といった感じです。
STGの敵や弾の超簡易版といった感じでしょうか。

ここで分からないことです。
まず、星インスタンスの配列の空き状況を調べないといけませんが、繰り返しで配列全体を調べる関数を星クラスに含めることはできません。
ということで、星管理クラスなるものを作り、星配列全体を反復で調べる関数を作ります。しかし、こうすると今度は星管理クラスから星クラスにアクセスできなくなります。つまりフラグをいじれないのです。
クラスに含めると、配列全体の調査ができない。クラスに含めないと、中の変数にアクセスできない。ということになってます。

今回の質問は、上のプログラムをどう作りますか、ということです。
管理クラスの上手な作り方が分かれば、今回のカウント変数もゲーム管理クラスとかに入れることもできるのかなと思うのですが...
どうかよろしくお願いします。

[nullptr]

しかし、こうすると今度は星管理クラスから星クラスにアクセスできなくなります。つまりフラグをいじれないのです。

なぜ、そうなるのでしょうか。星管理クラスから星クラスへアクセスできるように設計すれば良いわけですが、タンタルさんはなぜアクセスできないとお考えなのでしょうか？
できれば試しにコードを書いてみせていただけますか。思い通りの挙動でなくても構いません。書くだけ書いて、一度見せていただけるとアドバイスがしやすくなるとおもいます。

[タンタル]

新月の獅子さん、返信ありがとうございます。

コード:

#include "DxLib.h"
#include "star.h"
#include"star_manage.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 ) ;	// 描画先画面を裏画面にする

	int System_c=0,i;
	CStar Star[20];
	CStarManage StarManage;

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

		// 本文の記述
		System_c++;
		StarManage.Check();
		for(i=0;i<20;i++){
			if(star[i].f)==1) continue;
			star[i].Move();
			star[i].Draw();
		}

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

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

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

	return 0	;
}

コード:

#include<DxLib.h>

class CStar{
private:
	int x;
	int f;
public:
	CStar();
	void Move();
	void Draw();
};

コード:

#include<DxLib.h>
#include"star.h"

CStar::CStar(){
	CStar::f=0;
	CStar::x=0;
}

void CStar::Move(){
	CStar::x+=2;
}

void CStar::Draw(){
	int Cr=GetColor(255,255,255);
	DrawString(CStar::x,240,"★",Cr);
}

コード:

#include<DxLib.h>

class CStarManage{
public:
	void Check();
}

コード:

#include<DxLib.h>
#include"star_manage.h"
#include"star.h"
extern int System_c=0;

void CStarManage::Check(){
	int i;
	if(System_c%10==0) for(i=0;i<20;i++){
		if(CStar::f[i]!=0) continue;

		CStar::f[i]=1;
		CStar::x[i]=0;
	}
}

明らかにアクセスできないとエラーが出る作りになってしまっていますが、無理やり作っているところはご容赦ください。
get,setを追加するべきかと考えたのですが、それなら変数をpublicにしてしまった方がいいのかなと思い、これは使えないなということでやめました。
お手数おかけしますがよろしくお願いします。

[beatle]

CStarにフラグ変数fをチェックするメソッドをつければ良いと思います。
変数をpublicにするのとアクセサメソッド（get/set）を付けるのは少し似ていますが違います。
僕なら
CStar::IsAllocated()
CStar::Allocate()
のようなメソッドを作ると思います。

細かいことで本題とは無関係なのですが、コンストラクタでメンバを初期化するときは「初期化子リスト」というものを覚えたほうがいいと思いますよ。

コード:

CStar::CStar()
    : f(0), x(0)
{ }

[タンタル]

beatleさん、返信ありがとうございます。
CStar::IsAllocated()
CStar::Allocate()　について、
単語や話の流れからして、これがゲッタ、セッタだと思うのですが、
もしよろしければもう少し詳しく教えていただけないでしょうか。

int CStar::getf(){
return CStar::f;
}

void CStar::setf(int f){
CStar::f = f;
}

上記のものが、CStar::fのアクセッサだと思うのですが、
CStar::IsAllocated()
CStar::Allocate()　はどのように異なってくるのでしょうか。

[nullptr]

詳しく読んでないのでわかんないんですけど、とりあえず急ぎでも肝心な星配列をアクセスできないところに置かないでください(^p^)
まず星配列はmainに置くべきではないですよね。急ぎでもこれは設計に大きく関わります。
星配列（の要素）たちは、それらを管理するものが所持するべきです。私ならまず可変長配列を使いますが、今回は固定します。(あとこの例はC++11の機能とかstdとか極力使わないようにしてるんで普段の書き方と違ってミスってるかもしれないです)

※話に合わせて少し関数名を変えました。

コード:

namespace
{
	const int numberStar = 20;
}

class StarFactory
{
public:
	static InterfaceStar* createStar(){ return new Star; }
};

class StarManager
{
private:
	InterfaceStar* arrayStar[numberStar];

public:
	StarManager();
	~StarManager();

public:
	bool allocateStar()
	{
		for( int i = 0; i < numberStar; ++i )
		{
			if( this->arrayStar[i]->checkDelete() )
			{
				delete this->arrayStar[i];
				this->arrayStar[i] = StarFactory::createStar();
				return true;
			}
		}
		return false;
	}
	bool allocateStar( InterfaceStar* star )
	{
		if( !star ) return false;
		for( int i = 0; i < numberStar; ++i )
		{
			if( this->arrayStar[i]->checkDelete() )
			{
				delete this->arrayStar[i];
				this->arrayStar[i] = star;
				return true;
			}
		}
		return false;
	}
	void excuteProcess(){ /* 割愛。 */ }
	void excuteDraw(){ /* 割愛。 */ }
};

とします(例えばの話、それと最低限です)。

星クラス

コード:

class InterfaceStar
{
public:
        ~InterfaceStar(){}

	bool IsAllocated() const = 0;
	void excuteProcess() = 0; //< 右端から左へと流れていく　及び画面外ならフラグを折る操作などはこの内部で
	void excuteDraw() = 0;
};

class Star
	:public InterfaceStar
{
private:
	bool flagDelete;
	int positionX;

public:
	Star( int position )
		:flagDelete	( false )
		,positionX	( position )
	{}

	bool IsAllocated() const { return this->flagDelete; }
	void excuteProcess()
	{
		this->positionX += 2;
		// 画面からはみでてたらフラグ折る処理
	}
	void excuteProcess()
	{
		DxLib::DrawString( this->positionX, 240, "★", DxLib::GetColor(255,255,255););
	}
};

コード:

int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance,LPSTR lpCmdLine, int nCmdShow )
{
	ChangeWindowMode(TRUE)  ;       // debug用ウィンドウサイズ
	if( DxLib_Init() == -1 )        // ＤＸライブラリ初期化処理
	{
		return -1 ;         // エラーが起きたら直ちに終了
 	}
 
	SetDrawScreen( DX_SCREEN_BACK ) ;   // 描画先画面を裏画面にする
 
	int System_c=0;

	StarManager managerStar;
 
	while (1) {
		ClearDrawScreen()   ;   // 画面クリア
 
		if( DxLib::CheckHitKey( KEY_INPUT_Z )!=0 )
		{
			managerStar.allocateStar();
		}
        	managerStar.excuteProcess();
        	managerStar.excuteDraw();
 
		ScreenFlip()    ;       // redraw 1
 
 		if (ProcessMessage() == -1 ) break  ;   // windowsメッセージ処理
		if ( CheckHitKey(KEY_INPUT_ESCAPE ) == 1) break ;   // escキーで終了
    
	}
 
	DxLib_End() ;//DXライブラリ終了処理
 
	return 0    ;
}

こんな感じですか？実際にコンパイルはしてないしインクルード書いてないしですが、私ならこんなかんじになると思います。タンタルさんが何を疑問に思ってるかよく理解できていないので、的を外していたら申し訳ありません。


ちなみにですが。
変数はパブリックにしないでください。アクセッサも必要最低限です。私のコードでもフラグのゲッターしか定義してません。

更についでですが
＞10フレームおきに、右端から左へと流れていく（おそらく2px？）
っていうのはつまり
1フレーム毎に0.2px移動する
と同じ意味ですよね。わざわざカウンタ変数を用意するのは助長かと。

[sq]

言われていることをそのまま受け取ると
単純に使用を示す配列を別に用意すれば問題は解決できます。

Star stars[20];
bool isuse[20];

として、isuseに使用しているかどうかを書き込んでいけばよいと思います。

[タンタル]

新月の獅子さん、sqさん、返信ありがとうございます。

初心者なのに無理にクラスにこだわりすぎた、自分の能力をわきまえてない要求だったと反省しております。いろいろとご迷惑をおかけしてしまい、本当に申し訳ありませんでした。質問文も、毎回時間をかけてチェックしているのですが、うまく伝えきれない部分ができてしまい、本当にすみませんでした。

しばらく考えてみたのですが、結局クラス内のprivate変数にget,set関数をつけて問題解決できそうです。
皆様、本当に申し訳ありませんでした。

[ISLe]

上記のものが、CStar::fのアクセッサだと思うのですが、
CStar::IsAllocated()
CStar::Allocate()　はどのように異なってくるのでしょうか。

振る舞いと実装は分けて考えるべきです。

getter/setterは変数そのものにアクセスするためのものですから、オブジェクトが割り当て済みかどうかという意味で使うと保守性を著しく落とします。

例えば『足を交互に動かす』実装を『歩く』という振る舞いとして使ってしまうと、逆立ちして歩く場合に困ります。
外部に対して、『歩く』という振る舞いを見せて、『足を交互に動かす』という実装は見えないようにするのが良い設計ということになります。

[タンタル]

一応できたので、張っておきます。
50フレームおきに変更しています。

main.cpp

コード:

#include "DxLib.h"
#include "star.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 ) ;	// 描画先画面を裏画面にする

	int System_c=0,i;
	CStar Star[20];

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

		// 本文の記述
		System_c++;
		if(System_c%50==0){
			for(i=0;i<20;i++){
				if(Star[i].Getf()==1){
					continue;
				}
				Star[i].Setf(1);
				Star[i].Setx(0);
				break;
			}
		}
		for(i=0;i<20;i++){
			if(Star[i].Getf()==0){
				continue;
			}
			Star[i].Move();
			Star[i].Draw();
		}

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

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

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

	return 0	;
}

star.h

コード:

#include<DxLib.h>

class CStar{
private:
	int x;
	int f;
public:
	CStar();
	void Move();
	void Draw();
	int Getf();
	void Setf(int f);
	int Getx();
	void Setx(int x);
};

star.cpp

コード:

#include<DxLib.h>
#include"star.h"

CStar::CStar(){
	CStar::f=0;
	CStar::x=0;
}

void CStar::Move(){
	CStar::x+=2;
	if(CStar::x>640) CStar::f=0;
}

void CStar::Draw(){
	int Cr=GetColor(255,255,255);
	DrawString(CStar::x,240,"★",Cr);
}

int CStar::Getf(){
	return CStar::f;
}

void CStar::Setf(int f){
	CStar::f = f;
}

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

void CStar::Setx(int x){
	CStar::x = x;
}

結局管理クラスを使ってないのですが、mainのごちゃごちゃしたところを別の関数にまとめ、それをメンバ関数に含むクラスを作れば、それが管理クラスになるのかなと考えています。
こうした方がいいという意見がございましたら是非教えてください。

[nullptr]

みなさん言ってますがgetter/setterは全部書いとけばいいってものじゃないんですよ。

結局管理クラスを使ってないのですが、mainのごちゃごちゃしたところを別の関数にまとめ、それをメンバ関数に含むクラスを作れば、それが管理クラスになるのかなと考えています。

それクラスにする必要無いですよね？
クラスを使ったオブジェクト指向プログラミングをしようと思ったら、ISLeさんの返信などをよく読んで理解しようとしてください。

そうでないなら、C++で書く必要は微塵もありません。

[タンタル]

ISLeさん、返信していただいたことに気付かず素通りしていました。申し訳ありません。

振る舞いは関数の名前であらわされるもので、実装が関数の中身、でしょうか。
関数の名前でどういうことができるかわかるけど、どうやっているかはわからない。そして、中身を知らない関数でも動かせるのがよい設計だ。このように解釈しました。
allocateについても調べてみましたが、これは新たにインスタンスを作るものであって、アクセス云々の話ではなかったようですね。
ここで分からないのが、どうしてこの例が出されたのかということです。getsetをそのまま呼び出すのではなく、allocate/isallocatedの中で呼び出すようにしなさい、ということでしょうか。
それと、もしよろしければ、具体的にallocate/isallocated関数の中身を教えていただけないでしょうか。その方が僕も理解しやすくなると思いますし、せっかく時間を割いていただいたのに結局理解できませんでしたとなっては、皆様の時間が無駄になっただけになってしまいます。僕も理解できるよう努力しますので、面倒だとは思いますが、どうかよろしくお願いします。


新月の獅子さん、返信ありがとうございます。
おっしゃる通りです。オブジェクト指向のためにC++をやっているのに、無理やりクラスを作るくらいならHSPやC言語で作ったほうがましですね。ご指摘ありがとうございます。
結局、今回の疑問の答えは新月の獅子さんの書いてくださったコードにほぼ書かれていたような気がします。星管理クラスの中に星のインスタンスを含めてしまえばよかったということですね。

ぐだぐだち引き伸ばしてしまい、本当に申し訳ありませんでした。

[ISLe]

難しく考えることはないと思います。

コード:

        for(i=0;i<20;i++){
            if(Star[i].Getf()==0){
                continue;
            }
            Star[i].Move();
            Star[i].Draw();
        }

Getf関数の戻り値が0と等しいというのはどういうときなのか、仕様を確認する必要があるでしょう。

コード:

        for(i=0;i<20;i++){
            if (star[i].IsAllocated()) {
                star[i].Move();
                star[i].Draw();
            }
        }

このように書けば、割り当てられて(Is Allocated)いたら、Move関数とDraw関数を実行すると読めます。
たとえ将来、仕様変更でフラグを使わなくなったとしても、「割り当て済みであるかどうか」という振る舞いは変わらないので呼び出し側のコードを変更する必要もありません。

[nullptr]

ぐだぐだち引き伸ばしてしまい、本当に申し訳ありませんでした。

いえそれは気になさらないでいいんですよ。理解しようとしてるのは伝わってきますから時間をかけて理解してください。オブジェクト指向をこんなんで理解できる方があり得ないです。


set,getを全て書いている点について。
これは、保守性が高いと思いますか？考えてみます。

このsetは、星クラスにアクセスさえできれば誰でも変数を変更できるということです。
これでは誰かが変更してしまうかもしれません。と言っても一人で書いてるならそんな事はない、と思うかもしれませんが、コードが肥大化すればありえない話ではないでしょう。
それに、アクセッサを変数分用意して、意味があるのでしょうか。その変数を必要としているもののみがアクセスできる方が綺麗だと思いません？

例えばタンタルさんというクラスがあるとします。私からタンタルさんのメソッドを見ると、

• 年齢を知る
• 年齢を設定する
• 返信する

というものがあったとします。

これを見て、「ん！？！？」ってなりませんか？
なんであんたが年齢わかるんだよｗｗｗってなりますよね。なんで年齢が設定できるんだよｗｗｗｗｗってなりますよね。

でもsetter/getterを全て書くってのはこういう状態です。
アナタのそれ全部見えちゃってます状態です。さて、これはわざわざタンタルさんクラスを使って管理しているといえるでしょうか。

年齢は仲の良い一部の人にだけ見せればよいし、年齢を設定できるのは自分か神様か法律くらいじゃないですか（？）。
望ましいのは、私から見て

• 返信する

だけでいいんです。

このほうが使い手はすっきりするし、間違えて中身を変えるなんてことは絶対に起こらないですよね。

[beatle]

ここで分からないのが、どうしてこの例が出されたのかということです。getsetをそのまま呼び出すのではなく、allocate/isallocatedの中で呼び出すようにしなさい、ということでしょうか。
それと、もしよろしければ、具体的にallocate/isallocated関数の中身を教えていただけないでしょうか。その方が僕も理解しやすくなると思いますし、せっかく時間を割いていただいたのに結局理解できませんでしたとなっては、皆様の時間が無駄になっただけになってしまいます。僕も理解できるよう努力しますので、面倒だとは思いますが、どうかよろしくお願いします。

一応僕が考えていたAllocate()とIsAllocate()の中身を書いておきますね。超単純ですが。
僕のコードでは、f（フラグ）はbool型としています。そのほうがフラグっぽいでしょう？

コード:

void CStar::Allocate()
{
    Setf(true);
}
bool CStar::IsAllocated()
{
    return Getf();
}

現在はCStarクラスがフラグを用いた実装なのでAllocate()とIsAllocate()はフラグを利用していますが、ISLeさんが仰るように、将来フラグを使わなくなったとしても外部からは常にAllocate()とIsAllocate()というメソッドは利用可能です。インターフェースが変わらないのは良い設計の証拠です。

[タンタル]

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

ISLeさん、ありがとうございます。
なんだか予想以上にあっさりしていました。そして前回何が言いたかったのかがよくわかりました。
今回のコードではフラグ変数を使って、割り当ての状態を調べていますが、これを単にgetf()で調べていては、fを使わなくなったときに実装と振る舞いどちらも変更する必要が出てきます。しかし、allocate()で調べておけば、実装を変更するだけでいい、ということですね。実装と振る舞いはわけるべき、ですね。

新月の獅子さん、本当にいつもありがとうございます。
もしかしたら、一度自分でそういう困った状況を作らないとイメージができないのかもしれないですね。
ですが、今までの皆さんとのやり取りで、単純にget/setとするよりかは、関数名を目的に合ったものにして、クラスに含まれるのにふさわしいものにするべきなのかなと思いました。
よく見てみればget/setと変わらないような関数でも、単なるget/setがクラスに含まれているのが不自然なことは実際によくおきそうですからね。

beatleさん
せっかくのC++ですから、確かにboolを使った方がいいですね。ON,OFFしかないのは目に見えてますからね。
そして、ISLeさんとbeatleさんのコードにはget/setが含まれているのですが、システマティックに考えれば僕も必要だと思います。なんにしても、存在しているかを調べる必要がありますから。
じゃあ、一般的に考えて、自分が存在しているかを調べたり、自分の存在状況を操るっていうのは...
まぁ、ありですね。死んでる時に生き返られるかとか、調べるもなにも行動できないだろ、とか考えてしまいますが、単純に考えてしまえば、間違ってはいない気がします。星クラスですしね。


脱線し続けてしまいましたが、最初の疑問も、途中で出てきた疑問も、自分の言葉で理解できたような気がします。
これは最後まで助けてくださった皆様のおかげだと思います。本当にありがとうございました。
また何かしらつまずくことがあって、再びこちらで質問することになるかもしれませんが、そのときはよろしくお願いします。
本当にありがとうございました。