既存プログラムから関数の導入。

Question

C言語で私が作ったプログラムを
① ファイル（R7_in.txt）からN人分の個人識別番号、名前
(family name)、身長[cm]、体重[kg]のデータを読み取り、
各配列に格納する。確認のため、読み込んだすべての
データを画面に出力する。
② 身長と体重からBMI値を計算した結果をBMI用の配列に格
納する。
③ BMI値の降順になるように、個人識別番号、名前、身長、
体重、BMI値の各配列要素の値を並び替える。
④ ファイル（R7_out.txt）に、BMI値の降順に、各行が個人識
別番号、名前、身長、体重、BMI値となるようにデータを出
力する。
の４つの関数にして新しく作りたいです。

発生している問題・エラーメッセージ

既存のプログラムを関数に落とし込むことができない。

該当のソースコード

#include <stdio.h>
#include <string.h>

int main( void ){
int ID[15] = {0};
double hight[15] = {0};
double weight[15] = {0};
double BMI[15] = {0}; /* データを配列に格納する /
char name[15][25]={0};
int i, j, n = 15; / nはデータ数 */
FILE *fpin, fpout; / ファイルポインタ変数の宣言 /
fpin = fopen("R7_in.txt", "r"); / 入力(read)用ファイルをオープン /
fpout = fopen("R7_out.txt", "w"); / 出力(write)用ファイルをオープン */

if (fpin == NULL) { /* 安全対策：入力用ファイルのオープンに失敗したときの処置 /
printf("入力ファイルのオープンに失敗しました");
return 0; / 途中でプログラムを終了させる */
}

if (fpout == NULL) { /* 安全対策：出力用ファイルのオープンに失敗したときの処置 /
printf("出力ファイルのオープンに失敗しました");
return 0; / 途中でプログラムを終了させる */
}
//数値の読み取り

i = 0;
while( fscanf(fpin, "%d  %s %lf %le", &ID[i],name[i], &hight[i],&weight[i] )!= EOF ) {
i++;
}

printf("ID    NAME     HIGHT[cm]     WEIGHT[kg]\n");
for( i=0; i<15; i++ ){
	printf("%d     %-8s     %.1lf    %.1lf\n", ID[i], name[i], hight[i],weight[i] );
}

for (i=0; i<15; i++){//i=0～14まで繰り返す
BMI[i] = weight[i]/hight[i]/hight[i]*10000;//BMIの計算
}

double a = 0, b = 0, c = 0, d = 0;
char e[25] = {0};

printf("\n");
for (i=0; i<n-1; i++) { /* i=0(最初のデータ)からi=n-2まで繰り返す /
for (j=i+1; j<n; j++) { / j=i+1からj=n-1(最後のデータ)まで繰り返す /
if(BMI[i] < BMI[j]) { / もしdata[i]>data[j]なら中身を交換し、data[i]を小さい状態にする /
a = BMI[i]; / BMI[i]～name[i]の中身を一時、a～eに保存する /
b = ID[i];
c = weight[i];
d = hight[i];
strcpy(e,name[i]);
BMI[i] = BMI[j]; / BMI[j]の中身をdata[i]に移す */
ID[i] = ID[j];
weight[i] = weight[j];
hight[i] = hight[j];
strcpy(name[i],name[j]);

    BMI[j] = a;         /* 保存していたaの中身をdata[j]に移す */
	ID[j] =  b;
	weight[j] = c;
	hight[j] = d;
	strcpy(name[j],e);
  }
}

}

for (i=0; i<n; i++) {
fprintf(fpout, "%d %-8s %.1lf %.1lf %.1lf \n", ID[i],name[i], hight[i],weight[i], BMI[i] ); /* 結果を出力ファイルへ出力 */
}

fclose(fpin); /* 入力用ファイルをクローズ /
fclose(fpout); / 出力用ファイルをクローズ */

return 0;

}

###R7_in.text
1001 Aoki 172.8 63.1
1002 Enomoto 167.5 68.9
1003 Fujita 182.6 69.3
1004 Hayashi 178.7 55.7
1005 Ishii 170.1 70.2
1006 Kawai 179.3 83.1
1007 Kubo 169.6 56.4
1008 Murakami 165.2 68.7
1009 Nakajima 187.6 61.0
1010 Nomura 172.0 71.5
1011 Okada 180.8 73.8
1012 Sakamoto 177.4 63.7
1013 Shibata 178.2 67.3
1014 Takeda 172.9 78.1
1015 Wada 170.7 65.4

Comments

[pepperleaf]

元のプログラムをちょっと脇に置いて、
①から④までのそれぞれについて、入力と出力がどうなるかを整理して、関数化しましょう。

[ooba]

アドバイスありがとうございます。
関数化難しそうです…でもやってみます。

Answer 1

関数名を決めて、配列を引数にするだけじゃないの？

#include <stdio.h>   // fopen, fclose, fscanf, fprintf, printf
#include <string.h>  // strcpy

int readData(int n, int *ID, double *height, double *weight, char name[][25])
{
	FILE *fp = fopen("R7_in.txt", "r");
	if (fp == NULL) {
		printf("入力ファイルのオープンに失敗しました");
		return 1;
	}
	printf("ID    NAME     HIGHT[cm]     WEIGHT[kg]\n");
	for (int i = 0; i < n && fscanf(fp, "%d%s%lf%lf",
			&ID[i], name[i], &height[i], &weight[i]) == 4; i++)
		printf("%d     %-8s     %.1f    %.1f\n", ID[i], name[i], height[i], weight[i]);
	fclose(fp);
	return 0;
}

void calcBMI(int n, double *height, double *weight, double *BMI)
{
	for (int i = 0; i < n; i++)
		BMI[i] = weight[i] / height[i] / height[i] * 10000;
}

void sortData(int n, int *ID, double *height, double *weight, double *BMI, char name[][25])
{
	for (int i = 0; i < n - 1; i++) {
		for (int j = i + 1; j < n; j++) {
			if (BMI[i] < BMI[j]) {
				double a = BMI[i]; BMI[i] = BMI[j]; BMI[j] = a;
				int b = ID[i]; ID[i] = ID[j]; ID[j] = b;
				a = weight[i]; weight[i] = weight[j]; weight[j] = a;
				a = height[i]; height[i] = height[j]; height[j] = a;
				char e[25];
				strcpy(e, name[i]); strcpy(name[i], name[j]); strcpy(name[j], e);
			}
		}
	}
}

int writeData(int n, int *ID, double *height, double *weight, double *BMI, char name[][25])
{
	FILE *fp = fopen("R7_out.txt", "w");
	if (fp == NULL) {
		printf("出力ファイルのオープンに失敗しました");
		return 1;
	}
	for (int i = 0; i < n; i++)
		fprintf(fp, "%d %-8s %.1f %.1f %.1f \n",
			ID[i], name[i], height[i], weight[i], BMI[i]);
	fclose(fp);
	return 0;
}

int main(void)
{
	int ID[15];
	double height[15], weight[15], BMI[15];
	char name[15][25];

	if (readData(15, ID, height, weight, name)) return 1;
	calcBMI(15, height, weight, BMI);
	sortData(15, ID, height, weight, BMI, name);
	if (writeData(15, ID, height, weight, BMI, name)) return 1;
	return 0;
}

どこが難しいのかをコメントください。

Comments

[ooba]

ご回答ありがとうございます。
とてもきれいなプログラミングで「これだ！」と思いました。
私は配列、ポインタをうまく扱うことができず、関数の作成も初心者です。
なので申し訳ないのですが、正直わからないところがわからない状態なのではっきりとはコメントできません。
配列を引数にした関数の作成行為が難しいのだと思います。
あまり良い返信ができずすみません。
ご回答ありがとうございました。

[yumetodo]

多分C言語ポインタ完全制覇(前橋和弥)という本が今のあなたに必要な本なのかなと思います。古い方のやつでも十分なので手に入れて読んでみてください

Answer 2

課題の良し悪しは別として下記でいかがでしょうか？
なるべく元の処理をくずさないように気をつけましたが。

#include <stdio.h>
#include <string.h>

#define RET_OK (0)
#define RET_NG (-1)

#define NAME_MAX (25)
#define DATA_NUM (15)

#define INPUT_FILE "R7_in.txt"
#define OUTPUT_FILE "R7_out.txt"

/**
 * @brief １の処理関数
 */
int read_physical_data_from_file(const char *filename, int *ID, char name[][NAME_MAX], double *hight, double *weight, size_t num)
{
    int ret = RET_OK;
    FILE *fp;

    fp = fopen(filename, "r");
    if (fp == NULL) {
        printf("fopen() failed. name=%s\n", filename);
        ret = RET_NG;
        goto error_end;
    }

    int i = 0;
    while( fscanf(fp, "%d  %s %lf %le", &ID[i], name[i], &hight[i], &weight[i] )!= EOF ) {
        printf("%d     %-8s     %.1lf    %.1lf\n", ID[i], name[i], hight[i], weight[i] );
        i++;
    }

    if (i != num) {
        printf("warn: i=%d, num=%lu\n", i, num);
    }

  error_end:

    return ret;
}

/**
 * @brief ２の処理関数
 */
int set_bmi_from_phyisical_data(int *ID, char name[][NAME_MAX], double *hight, double *weight, double *BMI, size_t num)
{
    int ret = RET_OK;
    int i;

    for (i = 0; i < num; i++) {
        BMI[i] = weight[i] / hight[i] / hight[i] * 10000;
    }

    return ret;
}

/**
 * @brief ３の処理関数
 */
int sort_physical_data_desc_bmi(int *ID, char name[][NAME_MAX], double *hight, double *weight, double *BMI, size_t num)
{
    int i, j;
    double a = 0, b = 0, c = 0, d = 0;
    char e[25] = {0};
    int ret = RET_OK;

    for (i = 0; i < num - 1; i++)  {
        for (j = i + 1; j < num; j++)  {
            if(BMI[i] < BMI[j])  {
                a = BMI[i];
                b = ID[i];
                c = weight[i];
                d = hight[i];
                strcpy(e, name[i]);
                BMI[i] = BMI[j];
                ID[i] = ID[j];
                weight[i] = weight[j];
                hight[i] = hight[j];
                strcpy(name[i],name[j]);

                BMI[j] = a;
                ID[j] =  b;
                weight[j] = c;
                hight[j] = d;
                strcpy(name[j],e);
            }
        }
    }

    return ret;
}

/**
 * @brief ４の処理関数
 */
int write_result_file(const char *filename, int *ID, char name[][NAME_MAX], double *hight, double *weight, double *BMI, size_t num)
{
    int ret = RET_OK;
    FILE *fp;
    int i;

    fp = fopen(filename, "w");
    if (fp == NULL) {
        printf("fopen() failed. name=%s\n", filename);
        ret = RET_NG;
        goto error_end;
    }

    for (i = 0; i < num; i++)  {
        fprintf(fp, "%d %-8s %.1lf %.1lf %.1lf \n", ID[i], name[i], hight[i], weight[i], BMI[i] );
    }

  error_end:

    return ret;
}


int main( void )
{
    int ID[DATA_NUM] = {0};
    double hight[DATA_NUM] = {0};
    double weight[DATA_NUM] = {0};
    double BMI[DATA_NUM] = {0};
    char name[DATA_NUM][NAME_MAX]={0};

    int ret = RET_OK;

    ret = read_physical_data_from_file(INPUT_FILE, ID, name, hight, weight, DATA_NUM);
    if (ret != RET_OK) {
        goto error_end;
    }

    ret = set_bmi_from_phyisical_data(ID, name, hight, weight, BMI, DATA_NUM);
    if (ret != RET_OK) {
        goto error_end;
    }

    ret = sort_physical_data_desc_bmi(ID, name, hight, weight, BMI, DATA_NUM);
    if (ret != RET_OK) {
        goto error_end;
    }

    ret = write_result_file(OUTPUT_FILE, ID, name, hight, weight, BMI,  DATA_NUM);
    if (ret != RET_OK) {
        goto error_end;
    }

 error_end:

    return 0;

}

Comments

[yumetodo]

いやさすがにRET_OKってbool使いません？

[sukekeke0]

さすがにの理由はよくわかりませんが、そのdefine名はいくらなんでもダメだろ？ってことでしょうかね。サクッとエイヤーで作った案ですので大目に見てください。

もしよろしければ、質問者のために別の案を提示してあげてください。

[fana]

割とどうでもいい話ですが

> なるべく元の処理をくずさないように

といったことを意識するならば，
質問者が用いていない goto を使わずに普通にreturnした方が優しい気がします．

Answer 3

まずその関数分けの仕方がまったく良くないんですが、そう分けると決めたのはあなたですかね？それともなんかの課題？

例えば

ファイル（R7_in.txt）からN人分の個人識別番号、名前

(family name)、身長[cm]、体重[kg]のデータを読み取り、
各配列に格納する。確認のため、読み込んだすべての
データを画面に出力する。

ってすでにいろいろなことをやりすぎていますし、その格納する配列ってどこにあるのよ、って話になります。こういう分け方だとグローバル変数を増やしがちでよくありません。

まず、データの読み取りと画面出力は分けるべきです。

例えばデータの読み取りなら、ファイル名とか書き出す配列へのポインタは引数で与えられるべきです。つまり

#include <stddef.h>
#include <stdbool.h>
#include <stdint.h>
typedef struct {
    uint64_t individual_number;
    const char* name;
    uint32_t height;
    uint32_t weight;
} data_t;
bool data_read_from_file(data_t** result, size_t result_length, const char* file_name);

のような関数であるべきです。

関数の分け方から見直せませんかね？

---

せっかくなので、元のプログラムを適切に関数分けして、さらにもっとまじめにエラーハンドリングして、可変長の入力に対応させてみました。文字数の関係でソースコードと実行結果はリンク先を参照してください。

まずはC言語で書いたもの。エラー処理の関係であまり関数を分けられていないです。

ソースコードと実行結果

次にC++で書いたもの。エラー処理をたくさん書かなくてもいい感じになるのでスッキリしますね。C++20で追加されたstd::formatを使っているのですが、まだ殆どの処理系で実装されてないと思うので、その場合はfmtlib/fmtを呼び出すようにしてます。

ソースコード

実行結果

Comments

[ooba]

ご回答ありがとうございます。
こちらは学校の課題です。条件が以下のようになっております。

1. 個人識別番号、名前(family name)、身長[cm]、体
重[kg]用の配列は全てmain関数のローカル変数と
して宣言する。
2. ①～④の処理はそれぞれ関数にまとめ、関数呼び
出しにより各処理を実行する構成とする（関数は４
つ必要）。関数名は各自で決めること。関数内で必
要な変数の宣言を忘れないように注意すること。

となっており、残念ですが関数の分け方は決まっているようで見直しは難しいです。

[yumetodo]

どうしてこう、よくない関数分けを教えるかなぁ・・・。
まあしかしその条件だと①の関数のプロトタイプ宣言は上のdata_read_from_fileと同じでいいですね。まあ関数名はもうちょっと考えるとして。

https://qiita.com/yumetodo/items/5886b2c0c421c24c909b#%E9%96%A2%E6%95%B0%E3%81%A8%E3%81%AF%E7%BE%A9%E5%8B%99%E6%95%99%E8%82%B2%E3%81%AB%E7%AB%8B%E3%81%A1%E8%BF%94%E3%81%A3%E3%81%A6
こんなのも読んでみて下さいな。

[yumetodo]

え、ところで③のソートってqsortみたいなの使うんじゃなくて自力でやらなきゃいけないんですか・・・。

[ooba]

使ってはいけないのような制限はないですが、qsort?などは初めて聞く単語でよくわからないので、私たちのほうではそうなると思います

[yumetodo]

もしかするとソートの練習も兼ねていたのでしょうかねぇ。ちなみにqsortは
https://linuxjm.osdn.jp/html/LDP_man-pages/man3/qsort.3.html
こういうやつで、第4引数に比較の基準を記した関数へのポインタを渡すことでソートできる関数です。

[archiver]

色々と突っ込みたくなる「課題」ですね。課題の作成者って、Cのプログラムを書いたことが無いんじゃないかというくらいに。
この課題、個人識別番号、名前、身長、体重とBMI値のデータを構造体にまとめちゃダメなの？構造体にまとめた方が楽だと思うんだけど（ってか、普通はまとめると思う。構造体はまだ習ってないのかな？）。