ヒットアンドブローのプログラム

[Lawliet]

ヒットアンドブローのプログラムを自分の力だけで制作してみました。

このコードについて何かアドバイスを頂けないでしょうか？
それと
自分なりにコードを作成したあとは、WEB上にあるコードなどを見たほうがいいですか？
それとも見ずに自分のコードを突き詰めたほうがいいですか？

コード:

/*
	ヒットアンドブロー

*/
#define LONG 4		//桁数
#define RIREKI 5	//履歴の個数

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

typedef struct {
	int input;
	int hit;
	int blow;
} hitandblow;

void print_rule(void)
{
	printf( "*ヒットアンドブローのルール(製作中)************\n"
			"コンピューターが適当に４桁の数を設定します\n"
			"あなたはその数字を当ててください\n"
			"\n"
			"４桁の数は先頭が０になることもあります\n"
			"そして全桁とも異なる数字です\n"
			"\n"
			"さあ、頑張って４桁の数字を当ててみてください!\n"
			"***********************************************\n"
			"\n");
}

void print_command(void)
{
	printf( "-コマンドの一覧--------\n"
			"終了　　　　　　[0]\n"
			"答える　　　　　[1]\n"
			"覆歴の表示　　　[2]\n"
			"ルールの表示　　[3]\n"
			"コマンドの表示　[4]\n"
			"と入力してください\n"
			"------------------------\n");
}

int check_number(int n)
{
	int temp[LONG];
	int waru, del;
	int loopFLAG = 1;
	int i, j;

	//3桁以上かどうかの判断
	if( n >= 123){
	
		//nを配列に格納
		for(i = 0, waru = 10, del = 1; i < LONG; i++, waru *= 10, del *= 10){
			temp[i] = (n % waru) / del;
		}

		//比較
		for(i = 0; i < LONG; i++){
			for(j = 1; j < LONG; j++){
				if((i != j) && (temp[i] == temp[j]))
					loopFLAG = 0;//loopFLAGを'偽'に
			}
		
			if(loopFLAG == 0)
				break;
		}
	
		return ( loopFLAG );
	}
	else{
		return 0;
	}
}

hitandblow check_hit_blow(int n1, int n2)
{
	hitandblow re = {0, 0, 0};
	int waru, del;
	int i, j;
	int n1_ch[LONG], n2_ch[LONG];

	//===============================
	//文字列に格納
	//===============================

	//n1
	for(i = 0, waru = 10, del = 1; i < LONG; i++, waru *= 10, del *= 10){
		n1_ch[i] = (n1 % waru) / del;
	}
	//n2
	for(i = 0, waru = 10, del = 1; i < LONG; i++, waru *= 10, del *= 10){
		n2_ch[i] = (n2 % waru) / del;
	}
	//===============================
	//===============================

	//===============================
	//比較
	//===============================

	for(i = 0; i < LONG; i++){
		for(j = 0; j < LONG; j++){

			if(n1_ch[i] == n2_ch[j]){
				if(i == j){
					re.hit++;
				}
				else{
					re.blow++;
				}
			}

		}
	}

	return re;
}

void ctrl_history(hitandblow temp, hitandblow history[])
{
	int i;

	for(i = RIREKI - 1; i > 0; i--){
		history[i] = history[i - 1];
	}

	history[i] = temp;
}

void print_history(hitandblow history[], int count)
{
	if(count != 0){
		int i;
		int n = 0;

		if(count < RIREKI){
			n = (RIREKI - count);
		}

		printf( "\n"
				"4桁  HIT数 BLOW数\n"
				"----------------\n");

		for(i = 0; i < (RIREKI - n); i++){
			if(history[i].input >= 1000){
				printf("%4d %dHIT %dBLOW\n",
				history[i].input, history[i].hit, history[i].blow);
			}
			else{
				printf("0%3d %dHIT %dBLOW\n",
				history[i].input, history[i].hit, history[i].blow);
			}
		}
		putchar('\n');
	}
	else{
		printf( "まだ1回も回答していないので履歴はありませんよ\n"
				"\n");
	}
}

hitandblow play_it_play(int right)
{
	int answer;	//解答
	hitandblow temp = {0, 0, 0};

	//数字の入力
		do{
			printf("4桁の数を入力してください:");
			scanf("%d", &answer);
			
			if( !( (answer >= 123) && (answer <= 9999) )){
				//123->"0123"で4桁の最小数
				printf("\a警告:4桁で入力してください\n");
			}
			
			if( check_number(answer) == 0 ){
				printf("\a警告:全桁とも異なる数で入力してください\n");
			}


		}while( (check_number(answer) == 0) || !((answer >= 123) && (answer <= 9999)));

		//===============================
		//比較と代入
		//===============================

		//"hit"と"blow"
		temp = check_hit_blow(right, answer);

		//"input"
		temp.input = answer;
		//===============================
		//===============================

		return temp;
}

int main(void)
{
	int right;	//正解
	hitandblow  temp = {0, 0, 0},
				history[RIREKI];
	unsigned count;
	int command;
	int loopFLAG = 1;

	printf( "===================================\n"
			"####################\n"
			"#ヒットアンドブロー#\n"
			"####################\n"
			"\n");
	print_rule();

	print_command();
	printf(	"===================================\n"
			"\n");


	do{//rand()をtime()の戻り値で初期化
		srand((unsigned) time(NULL));

		//ここの9999をあとでLONGを使って再現する
		right = ((rand() % 9999) + 1);
	}while(check_number(right) == 0);

	for(count = 0; (temp.hit != LONG) && (loopFLAG);){
		do{
			printf("何をしますか?:");
			scanf("%d", &command);

			if(command < 0 || command > 4)
				printf("もう一度入力してください\n");
		}while(command < 0 || command > 4);

		switch( command ){
			case 0:
				loopFLAG = 0;
				break;

			case 1:
				count++;	//カウントアップ

				printf( "---------------\n"
						"第%d回戦\n"
						"---------------\n"
						"\n", count);
		
				temp = play_it_play(right);
	
				ctrl_history(temp, history);

				//結果
				if(temp.input >= 1000){
				printf("%dは %dHIT %dBLOW\n", temp.input, temp.hit, temp.blow);
				}
				else{
					printf("0%dは %dHIT %dBLOW\n", temp.input, temp.hit, temp.blow);
				}

				break;

			case 2:
				print_history(history, count);
				break;
			case 3:
				print_rule();
				break;
			case 4:
				print_command();
				break;
		}
	}

	if(temp.hit == LONG){
		printf( "===========\n"
				"= -正 解- =\n"
				"===========\n"
				"\n"
				"%d回目で正解しました\n", count);
	}
	else{
		printf( "===========\n"
				"= -失 敗- =\n"
				"===========\n"
				"\n");
		if(right >= 1000){
			printf("正解は%dでした\n",right);
		}
		else{
			printf("正解は0%dでした\n",right);
		}
	}

	return 0;
}

[box]

4桁とも異なる数字であるならば、最大値は9876であるような気がします。
そもそも、4桁の数値として扱うよりも、4つの数字からなるchar型の配列とする方が
数値が123以上かどうかを気にしなくてすむ分、コードがシンプルになるような気がします。

[beatle]

インデントもちゃんと出来ているし、動きそうなプログラムだし、いいんじゃないでしょうか。

でもこれ、きちんと動きますか？いやまあ、大抵の場合は上手く動くのでしょうが、気になる点があります。

コード:

    do{//rand()をtime()の戻り値で初期化
        srand((unsigned) time(NULL));
 
        //ここの9999をあとでLONGを使って再現する
        right = ((rand() % 9999) + 1);
    }while(check_number(right) == 0);

ここなのですが、ループの中でsrandしてますよね。
このループは非常に早く回りますので、time()の戻り値は変化しません。（time()は1秒ずつしか変化しないことを思い出して下さい）
すなわち、運悪くcheck_number(right) == 0になってしまった場合、1秒くらいループを回り続けることになります。
（さらに運が悪く、time()が変化した後もやはりcheck_number(right) == 0になってしまった場合、さらに1秒ループが継続します）

それから、プログラムを手っ取り早く改良するためには、フラグ、チェックという言葉を使わないように改造してみましょう。
C言語では0が偽、0以外が真ということになっていますが、 loopFLAG と言われると、真でループするのか偽でループするのか分かりづらいのです。
例えば continueLoop という変数名にしておけば、 continueLoop が真ならループをするのだなと分かります。

同様に、 check_number では、 number が何だったら真を返すのか分かりません。
is_valid_number にしておけば、 number が有効な値なら真を返すのだな、と分かります。

コード:

if (check_number(right)) {
    /* right が何だったら実行されるのか？ */
}
if (is_valid_number(right)) {
    /* right が有効な値なら実行される！ */
}

[usao]

・値を各桁に分解している処理(例えばline55-56)は複数個所にある　→　関数化する
・他人にソースを見てもらう際には各関数が何をするものなのか？といったコメントを付けた方がよいと思う．
・変数名は役割で付けるとよいかと．（tempとかでなく．）
・細かい点ですが，line59以降の判定は↓でどうかな？

コード:

{//比較
    for( i=0; i<LONG-1; i++ )  //iの範囲は1だけ狭くていい
    {
        for( j=i+1; j<LONG; j++ )  //一度見た(i,j)の組み合わせは見ない
        {
            if( temp[i] == temp[j] )
            {
                loopFlag = 0;
                break;  //ループを抜けていい
            }
        }

        if( loopFLAG==0 )break;
    }
}

>自分なりにコードを作成したあとは、WEB上にあるコードなどを見たほうがいいですか？
>それとも見ずに自分のコードを突き詰めたほうがいいですか？
自由に　両方とも　やればよいのではないでしょうか．

[Lawliet]

皆様のご意見とても参考になりましたありがとうございます。

今はc++の勉強をしているので今度また
Ｃ++で再挑戦します。その時はまたよろしくお願いします。