５人の生徒の中から最高得点を出すプログラム

[トッピー]

コード:

#include<iostream>
using namespace std;

int max(int x[]);

int main(void) {
	
	int cube[5];

	cout << "5人のテストの点数を入力してください\n";
		for(int i = 0; i < 5; i++) {
			cin >> cube[i];
   }
		
		cout << "最高得点は、" << max(cube) << "です。\n";
}

int max(int x[]) {



	int max; /*変数を宣言*/
	max = x[0]; /*maxにt配列を代入*/
	for (int i = 0; i < x[i]; i++) {
		if (max < x[i]) {
			max = x[i];
			return max;
		}
	}
}

エラーは出ないんですが最高得点が出ません。
良かったら理由も教えてください、お願いします。

[トッピー]

あ、すみません、できたっぽいです。

[みけCAT]

• ループの継続をi < xという不自然な条件式で判定しているので、範囲外アクセスを起こす可能性がある
• 1回最高得点が更新された時点でreturnしているので、本当の最高得点が考慮されない可能性がある
• 最初の要素が最高得点だった場合、範囲外アクセスが起きなかったとしてもreturn文を通らずに関数から帰るので、返り値が不定になる

のがよくないですね。

(本当は「5人」という値を識別子に定義してパッと見で意味がわからない定数(マジックナンバー)の使用を避け、
1箇所の修正だけで人数を変えられるようにするとベターですが、とりあえず)
max関数を以下のようにするといいでしょう。

コード:

int max(int x[]) {



	int max; /*変数を宣言*/
	max = x[0]; /*maxにt配列を代入*/
	for (int i = 0; i < 5; i++) { // 配列の要素を過不足なくチェックする (x[0]は使ったので、iの初期値は1でもよい)
		if (max < x[i]) {
			max = x[i];
			// ここで焦ってreturnしない
		}
	}
	return max; // 全部の要素を見た後で、結果を返す
}

[トッピー]

回答ありがとうございます。
あ、やっぱりconst使うんですね、なるほどです。

[トッピー]

とりあえずもう一回プログラム書き直した後、どこがどうおかしいのか見直してみます。

[トッピー]

コード:

#include<iostream>
using namespace std;

int max(int x[]);

int main(void) {
	
	int cube[5];
	int answer;
	cout << "5人のテストの点数を入力してください\n";
		for(int i = 0; i < 5; i++) {
			cin >> cube[i];
   }
		answer = max(cube);

		cout << "最高得点は、" << answer << "です。\n";
}

int max(int x[]) {



	int max; /*変数を宣言*/
	max = x[0]; /*maxにt配列を代入*/
	for (int i = 1; i < 5; i++) {
		if (max < x[i]) {
			max = x[i];
		
		}
		
	}
	return max;
}

max関数のfor文の繰り返すところを配列をi < 5に直し
return max;の位置を変えたところプログラム通り点数の比較ができました。
後は、max関数の呼び出しに戻り値を返し、それをanswerへ代入して出力したところ、無事できました、
ありがとうございました。