c言語つくってみました

#include<stdio.h>
#define N 5
struct MEMBER{
int no;
char name[8+1];
int run;
int jump;
int power;
int sum;
double avg;
};
void Calc_Sum(struct MEMBER *data)
{
data->sum=data->run+data->jump+data->power;
return(0);
}

void Calc_Avg(struct MEMBER *data)
{
data->avg=data->sum/3.0;
return(0);
}

void Display_Data(struct MEMBER data)
{
printf("---判定---\n");
printf("背番号:%5d\n",data.no);
printf("名前:%5s\n",data.name);
printf("走力　:%5d 点数\n",data.run);
printf("跳躍力:%5d 点数\n", data.jump);
printf("筋力　:%5d 点数\n", data.power);
printf("総計　:%5d 点数\n", data.sum);
printf("平均　:%5.1f 点数\n", data.avg);
return(0);
}

int main(void)
{
int i;
struct MEMBER member[N]
={{51,"イチロー",95,95,80,0,0},
{55,"松井秀喜",70,80,90,0,0},
{18,"松坂大輔",75,75,80,0,0},
{18,"黒田博樹",80,85,85,0,0},
{19,"上原浩治",85,85,85,0,0},
};
struct MEMBER *p=member;






/*総計の算出*/
for(i=0;i<N;i++)
Calc_Sum(p+i);

/*平均の算出*/
for(i=0;i<N;i++)
Calc_Avg(p+i);

/*結果の出力:*/
for(i=0;i<N;i++)
Display_Data(member[i]);
return(0);
}

以上のようなのc言語をつくりました。
他に関数を２つ使用しなければならないのですが
アドバイスよろしくお願い致します。

No.3 回答者： hitomura 回答日時： 2011/08/07 01:05

普段ならば「なにを作るんですか？」と聞くところですが、それ以前に突っ込みどころがいくつかあるのでそれを書きます。

(1)
Calc_Sum, Calc_Avg関数のどちらも引数として渡された値がNULLかどうかのチェックを行っていない危険なコードです。関数の冒頭に
　if (data==NULL) return;
を追加しましょう。

(2)
Calc_Avg関数はCalc_Sum関数の後に呼び出さないと正しい答が戻ってきません。
void Calc_Avg(struct MEMBER *data)
{
　if (data==NULL) return; /*(1)参照*/

　Calc_Sum(data);
　data->avg=data->sum/3.0;
　return; /*(3)参照*/
}
とすれば呼び出す順番を気にする必要がありません。しかし、これは「常に合計を計算するため、構造体メンバーsumにあまり意味がなくなる」という問題があります。

(3)
Calc_Sum, Calc_Avg, Display_Data関数は戻り値の型がvoidなのに0を返している。関数の戻り値の型がvoidならばreturnの後に戻り値を書かない。
逆に、戻り値の型をintにして、(1)で指摘した事態が起きた場合にエラーを表す値を、そうでない場合は正常に処理を終わったことを表す値を返すようにするという手段もある。

(4)Calc_Sum, Calc_Avg関数は引数が構造体のポインタ渡しになっているが, Display_Data関数は構造体を直接引数に渡しているためその分時間がかかる。他の2関数と同じように構造体のポインタ渡しにすべき。

(5)そもそも質問のタイトルは「c言語プログラムをつくってみました」とか「c言語でつくってみました」とかにすべき。質問者のタイトルのままでは、C言語のコンパイラを作ってみたともとられかねない。

No.2 回答者： KEIS050162 回答日時： 2011/08/07 00:53

コンパイラーがエラーやワーニングを出さなければ、大丈夫だと思います。

思いつきレベルですが気になる点をあげてみました（致命的ではないと思います）

・平均としている変数は float では精度が足りない？　特にこだわりがなければ float　でもいい様な気がします。
・平均値の算出にcastが必要？　（ワーニングが出なくて、結果が概ね良好なら大丈夫でしょう。）
・void　の関数に　return(0)　は要らない？　（コンパイラがワーニングを出しませんか？）
・Display_Data()の引数は、struct　のポインターとする方が良い。（他の関数と同じに。）


”作ってみました”、って初めてではないですよね？　（もし、初めてだとしたら、すごいです）
とてもオーソドックスな記述で、分かり易く書けていると思います。

No.1 回答者： Tacosan 回答日時： 2011/08/07 00:49

プログラム以前に, そもそも「何をしたいかを書こう」と, ちょっとでも思わなかったんだろうか.