C言語で勉強中（max,min）の出し方

C言語で勉強中（max,min）の出し方

maxまで表示が正しく出るようになったんですが、minでは終了時に入力の-999が表示されます。
今後、平均まで出るようにと考えているのですが、未だにminで思考停止中です。何がなんだかわかりません。どなたか、お助けください。

#include<stdio.h>
main(void)
{
float cm,kg,m,bmi,max,min;
int nin;
cm=kg=nin=0;
max=0;
min=-0;

printf("身長(cm)と体重を入力してください。\n");
while (cm=250,kg=200){

scanf("%f %f",&cm , &kg);

max=min=cm;
if(cm>max) max=cm;
if(cm<min) min=cm;





if(cm == -999 ||kg == -999) break;/*-999で終了 */

nin ++;

m=cm/100.0 ; /* Mに変換 */
bmi=kg / m / m; /* BMIの計算 */


printf("身長%5.1f ",cm);
printf("体重%5.1f ",kg);
printf("BMI値%7.1f",bmi );


if(bmi>=25)
printf("肥満です。\n");

if(bmi<18.5)
printf("低体重です。\n");

if(bmi <25 && bmi>=18.5)
printf("**.* \n");
}

printf("総人数%d\n",nin);
printf("最大身長%3.1fcm",max);
printf( "最低身長%3.1fcm",min);

return 0;
}

No.6 ベストアンサー 回答者： kmee 回答日時： 2010/06/20 11:58

>main(void)

最後にreturn 0しているのですから、最近の規格に合せるなら
int main(void)
とする方がよいでしょう。

> cm=kg=nin=0;
たしかに全部0で初期化されますが、int型のninとfloat型のcm,kgを同時に初期化するのはどうか、と。
文章としても
「身長(cm)=体重(kg)=人数(人)=0」
となり。単位の違うものを=で結んでいます。
コンピュータにとってはどちらでもいいことですが、プログラムを作るのはあくまで人間です。
人間がわかりやすい書き方にするのが、間違えを減らすコツです。

> while (cm=250,kg=200){

文法的には正しいけど、意図が不明。かっこの中は
cmに250代入して、式としての値は250
→,演算子で前の結果は破棄
→kgに200を代入して、式としての値は200
となり、実質
while(200)
となります。単に、無限ループを作りたいなら
while(1){
cm=250; kg=200;
の方が意図がわかりやすいですし、scanfでの失敗を回避する目的なら、scanfの戻り値をチェックして
if ( scanf("%f %f",&cm , &kg) != 2 ) {break ;}
等とする方がよいです。

>if(cm == -999 ||kg == -999) break;/*-999で終了 */
この時の値(-999)は、集計には使わない値ですから、max,minの集計の前にこの判定をする必要があります。
身長、体重ということを考えたら
cm <= 0 || kg <=0
で、0以下を入力したら、としてもいいかも。
身長=0が入力されてしまったときの、 「0で割る」のを防ぐこともできますし。

>max=min=cm;
すでに指摘があるように、ここで、maxもminも今入力したcmになっているので
>if(cm>max) max=cm;
>if(cm<min) min=cm;
はどちらも条件が成立しないはずです。
本当にmaxが正しく表示されたのでしょうか?
このプログラムは実際に確認したプログラムを貼り付けただけなのでしょうか?

また、これが max=min=cm が無い場合、maxは正しく求められます。
しかし、minは初期値が0のため、身長という0より大きな値と比較すれば、常に0の方が小さくなります。
対策は
・初期値をありえない大きな数にする。
この方法は、身長のような最大値が決まっている(人間の身長なら9999cmは有り得ない)ものでは有効だが、最大値が不明なデータに対しては使えない。
min=-0; → min = 9999 ;
・minが初期値(0)だったら、無条件で現在のcmを「最低値」とする(maxも同様)
この方法は、今回のような「初期値(0)は実際のデータに含まれない」場合では有効だが、「初期値が実際のデータに含まれる可能性がある」場合には使えない。
if( (max == 0) || (cm>max)) max=cm;
if( (min == 0) || (cm<min)) min=cm;
・最初の入力(つまり人数nin=0)だったら、無条件で現在のcmを「最低値」とする(maxも同様)
この方法は、「最初のデータ」であるかを判定する必要がある(今回の「人数=0」とか)が、どんなデータに対しても確実に最大、最小も求められる。
if( nin == 0) {
max=cm;
min=cm;
} else {
if (cm>max) max=cm;
if (cm<min) min=cm;
}



あと余談
> m=cm/100.0 ; /* Mに変換 */
単位の大文字小文字はちゃんと書け、と、先生や諸先輩によく言われたものです。
メートルはmであってMではありません。
大文字しか使えないディスプレイとかプリンタとかもあるんで、そういう場合は仕方ないんですが。

No.7 回答者： CanvasShoes 回答日時： 2010/06/22 11:17

scanfで取得した値は、

　　・計算で使いたい値
　　・終了するための特別な値
のいずれかです。
終了するための特別な値は、その目的だけに使い、最小値や最大値を計算しないほうが望ましい。

ということで、

【現状】
(1) scanf()
(2) 最大値、最小値更新
(3) 終了判定(if (...) break; )
の順にされていますが、


【改善】
(1) scanf()
(2) 終了判定(if (...) break; )
(3) 最大値、最小値更新

の順に処理してはどうでしょうか。

No.5 回答者： t_nojiri 回答日時： 2010/06/20 10:58

>>while()の継続条件をどうにかしましょう。

>>scanf()で終了する値も計算の中に入っちゃってるし
>2つとも何とかしないといけないと思っていたのですが、もう少しお願いします。

まず、while()の中に代入文が入ってるんです。
>while (cm=250,kg=200){
これは、普通ループの継続条件を書くのですが、cm=250って変数に値を入れちゃってます。
比較条件なら==なんです。もし値を入力するループでここでは無条件にループしたいんなら、while(1){でもいいし。

後、No1さんにも指摘されてる通り、ループ中にmax=min=cm;なんて代入処理を、判定無しで行ったら全てパーになりますよ。

No.4 回答者： asuncion 回答日時： 2010/06/20 10:57

＞min=-0;

最小値の初期値は、うんと大きい値にしておきましょう。

No.3 回答者： t_nojiri 回答日時： 2010/06/20 09:51

もうちょっとだけ書きますね。

>minでは終了時に入力の-999が表示されます。
それは、scanf()の直後に
>max=min=cm;
>if(cm>max) max=cm;
>if(cm<min) min=cm;
代入しちゃってます。
本当なら
>if(cm == -999 ||kg == -999) break;/*-999で終了 */
この終了判定は、scanf()直後です。
本来。

No.2 回答者： t_nojiri 回答日時： 2010/06/20 09:11

while()の継続条件をどうにかしましょう。

２つ条件をカンマでつなげても、継続条件読みにくくなってるだけです。
両方の条件を同時に満たすならAND(&&)、片方だけでも満たしているのならOR(||)ですよね？

後、入力用のループと集計用のループに分けた方が良いと思いますよ。
scanf()で終了する値も計算の中に入っちゃってるし。

この回答へのお礼

早い回答ありがとうございます。
いま少し教えていただきたいのですが、

>while()の継続条件をどうにかしましょう。
>scanf()で終了する値も計算の中に入っちゃってるし

2つとも何とかしないといけないと思っていたのですが、もう少しお願いします。

お礼日時：2010/06/20 10:08

No.1 回答者： edomin7777 回答日時： 2010/06/20 09:10

max=min=cm;

if(cm>max) max=cm;
if(cm<min) min=cm;

ここは、何をやっているつもり？
本当にMAXは表示される？