ページ 1 / 1
5人の生徒の中から最高得点を出すプログラム
Posted: 2017年1月26日(木) 13:13
by トッピー
コード:
#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;
}
}
}
エラーは出ないんですが最高得点が出ません。
良かったら理由も教えてください、お願いします。
Re: 5人の生徒の中から最高得点を出すプログラム
Posted: 2017年1月26日(木) 14:00
by トッピー
あ、すみません、できたっぽいです。
Re: 5人の生徒の中から最高得点を出すプログラム
Posted: 2017年1月26日(木) 14:01
by みけ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; // 全部の要素を見た後で、結果を返す
}
Re: 5人の生徒の中から最高得点を出すプログラム
Posted: 2017年1月26日(木) 14:34
by トッピー
回答ありがとうございます。
あ、やっぱりconst使うんですね、なるほどです。
Re: 5人の生徒の中から最高得点を出すプログラム
Posted: 2017年1月26日(木) 14:39
by トッピー
とりあえずもう一回プログラム書き直した後、どこがどうおかしいのか見直してみます。
Re: 5人の生徒の中から最高得点を出すプログラム
Posted: 2017年1月26日(木) 15:00
by トッピー
コード:
#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へ代入して出力したところ、無事できました、
ありがとうございました。