C言語で関数が呼ばれない

Question

C言語で税込み価格か割引価格を表示するプログラムが、思ったように動きません
苦Cの学習用Ｃ言語開発環境で動かしています。

バグのあるプログラム

#include <stdio.h>
#include<stdlib.h>
int input (int alleysize ,int* write)
	{
		int num;
		char inputalley[alleysize];
		fgets(inputalley,alleysize,stdin);
		num =  atoi(inputalley);
		*write = num;
		return 0;
	}
int main(void)
{
	int anser;
	do
	{
	int price = 0;
	int mode = 0;
	
	printf("定価を入力して下さい。\n");
	input(10,&price);
	
	printf("何を示しますか。\n0 : 税込み価格\nそれ以外 : 割引価格\n");
	
	input(1,&mode);
	
	if(mode = 0)
	{
		int tax = 0;
		printf("税率を入力してください。\n");
		input(2,&tax);
		price = (int)(price *(1+ tax/100));
		printf("税込み価格は%d円です。\n",price);
	}
	else
	{
		int tax = 0;
		printf("何割引きか入力してください\n");
		input(2,&tax);
		price = (int)(price * (1 - (tax/10)));
		printf("割引後の価格は%d円です。\n",price);
	}
	printf("まだつずけますか？。\n1 : はい\nそれ以外 : いいえ\n");
	input(1,&anser);
	}while(anser == 1);
	return 0;
}

実行結果

一部のinput関数が呼ばれていません。

変数名などわかりにくいと思いますが回答お願いします。

Comments

[thkana]

プログラムに全然関係ないですけれど、どうにも気持ち悪いので。
「続ける」はひらがなでは「つづける」と書きます。

[jimbe]

fgets へのパラメータを確認してください。

[episteme]

プログラムに全然関係ないですけれど、どうにも気持ち悪いので。

alley じゃなく array です。

[BeatStar]

基本も全くできていないし、ロジックもめちゃくちゃ。
問題を解く前に基礎からやった方が早いかも。

[Zuishin]

プログラムに全然関係ないですけれど、どうにも気持ち悪いので。
anser じゃなく answer です。

[BeatStar]

後、実行結果に関してもできるだけ画像はやめましょう。GUIとかの外観に関してとかのように文字では表現しづらい部分だけ画像にすべきです。(HTMLでのデザインがずれる問題とか)

コピペで実行結果を張り付ければ他のユーザ(回答者とか)がテキストエディタとかに張り付けて参考にするとかも出来ますが、画像だとそれが出来ません。手打ちでやればできますが面倒です。
質問するなら読み手に配慮する意識をしましょう。(結果的に出来ていなくても改善はできるので)

Answer 1

(※ 現在修正中です…)

[指摘]

関係ない部分ですが気になった部分を書きます。

まず一つ目。変数名 等はちゃんと名前を付けるべきです。
趣味で自分ひとりで開発する場合でも保守は当たり前のようにやります。一か月前のコードに手を入れるとか当たり前のことです。一か月前の自分はもはや他人です。自分が組んだコードですらマジで忘れています。読めばわかるとは思いますが、変数名がでたらめだとわかりにくくなります。ましてや実際に会社とかで開発する際は複数人で開発するのが当たり前のようなので、わかるわけないです。ちゃんと変数名を定義しましょう。

inputalley[alleysize];

→ inputarray[arraysize]では?

二つ目。記号の意味を明確に理解すべきです。

if(mode = 0)となっていますが、C言語やC++においてはif文内の条件式で 「=演算子」は「単なる代入」です。つまり、「modeの値が0なら」ではなく「modeに0を代入できるなら」的な意味になってしまいます。
そうなるとtrueになるのでelseとかに行きません。

三つ目。関数をちゃんと理解しましょう。引数や戻り値の理解がまったく出来ていません。

input関数を見ると、int input(int alleysize ,int* write)となっていますが、それぞれの引数・戻り値はどういう意味でしょうか? 引数 alleysizeの役割は? 引数 writeの役割は? input関数の戻り値であるintの役割は?

input関数の定義を見ると、

int input (int alleysize ,int* write)
{
		int num;
		char inputalley[alleysize];
		fgets(inputalley,alleysize,stdin);
		num =  atoi(inputalley);
		*write = num;
		return 0;
}

となっていますが、ちょっと違和感があります。

まず、なぜ第一引数alleysizeが必要なのでしょうか? 処理内容を考えると別に必要ないのでは?
コードを読んでみましょう。
コードを読むコツは「一行レベルで、その行が何をしているかを考えながら読む」です。

int input (int alleysize ,int* write)
{
                
		int num;
		char inputalley[alleysize];
		fgets(inputalley,alleysize,stdin);
		num =  atoi(inputalley);
		*write = num;
		return 0;
}

を読むと、

int input (int alleysize ,int* write)
{
                // 変数numを宣言する
		int num;
		// 要素数alleysizeとした配列inputalleyを宣言する
		char inputalley[alleysize];
		// 標準入力からalleysize分inputalleyに入れる
		fgets(inputalley,alleysize,stdin);
		// inputalleyの文字列データを整数に変換してnumに入れる
		num =  atoi(inputalley);
		// 戻り値扱いの第二引数writeにnumを設定
		*write = num;
		// 戻り値として0を返す
		return 0;
}

となる。

で、char型配列inputalleyを宣言していますが、要素数を考えてみてください。
たとえばinput( 2, &n );と呼び出したなら、inputalley[2];となるはずです。
char型配列の場合で文字列と見なすなら「終端文字」('\0'とか)が末尾につきます。
"Hello World"であれば "Hello World\0" という風に終端文字が付きますから、実際には本来の文字数 + 1 が必要になります。引数alleysizeが 2 であれば 実際の文字数は 2 - 1 = 1 つまり1文字分だけです。
(厳密には文字数ではなくバイト数ですが。)

よって

// main関数内

	printf("何を示しますか。\n0 : 税込み価格\nそれ以外 : 割引価格\n");
	
	input(1,&mode);

を読むと、要素数1 (実際に入れることができる文字数は1 - 1 = 0なので 0文字)なのでスキップされたような状態に。

さらに戻り値として return 0;と返していますがこれは意味あるのでしょうか?
仮にprintf関数とかみたいに他人が使うとしたら勘違いされますよ。
戻り値を返しているはずなのに意味のない値を返している。これならいっそvoidとして「戻り値無し」とした方がいいです。混乱のもとです。

「関数」「引数」「戻り値」これらが分かっていればありえないバグです。(厳密にはバグの温床になりやすいですが)

そして四つ目。おそらくここが原因だと考えています。「標準入出力を使う場合、バッファを意識せよ」です。

軽いサンプルとして、

#include <stdio.h>
#include<stdlib.h>
int input (int alleysize ,int* write)
{
		int num;
		char inputalley[alleysize];
		fgets(inputalley,alleysize,stdin);
		num =  atoi(inputalley);
		*write = num;
		return 0;
}

int main( void )
{
    int n;
    input( 3, &n );
    printf( "n = %d\n", n );
    int m;
    input( 3, &m );
    printf( "m = %d\n", m );
    int p;
    input( 3, &p );
    printf( "p = %d\n", p );
return 0;
}

とやってみたところ、nとpは入力を促されるが、mだけはなぜかスキップされる。(nを入力直後に"n = ..."と"m = ..."が同時に表示される)

C++でも一応この問題があって、cin >> 変数; の直後にstd::getline( ... );を使うとスキップされるという現象があります。

ユーザがコンソールに入力する。するとコンソールに文字列なり数値なりのデータが記録され、そこからNバイト分読み取っているようです。たとえば10バイト分入力したとしてfgets関数とかで4バイト分取り出した場合、コンソールには 10 - 4 = 6バイト分データがまだ残っています。

この残ったデータがあるので二回目のfgets関数が取り出すときに残りのデータを『新しく入力されたもの』と解釈して読み取る。そうするとユーザは入力していないのになぜか「入力された」と解釈していますからスキップされたような状態になります。

恐らくこれが原因かと。

「C言語 標準入力 スキップされる」で検索すると質問: ch = getchar();が時折スキップされます。がヒットします。

使っている関数がgetchar関数ですが発想は同じです。上記のある回答者の方の仰るようにfflush関数を使ってコンソール内のデータをフラッシュすると上手くいくのではないかと。

Comments

[abadcafe]

違います。代入は代入した値に評価されます。mode = 0 なら 0,　mode = -7 なら -7と評価されます。

>if(mode = 0)となっていますが、C言語やC++においてはif文内の条件式で 「=演算子」は「単なる代入」です。つまり、「modeの値が0なら」ではなく「modeに0を代入できるなら」的な意味になってしまいます。

Answer 2

(追記依頼にしようかとも思ったけれど...)

int input (int alleysize ,int* write)
	{
		int num;
		char inputalley[alleysize];
       printf("***func input() called.\n");//追加
		fgets(inputalley,alleysize,stdin);
		num =  atoi(inputalley);
		*write = num;
		return 0;
	}

として実行してみて、それでも

関数が呼ばれない

という現象なのかどうか考えてみてください。

そして、input()関数はどういう関数でしょう。説明してみてください。その点で勘違いしていないか...という気がします。

Comments

[ryoutan134]

実行してみると、関数自体は呼ばれていました、入力が何か間違っているということしょうか？

また、input関数は、入力された数値を引数のポインタに代入する関数だと思っています。
何か間違いがありますか？　

[thkana]

> 実行してみると、関数自体は呼ばれていました
少なくとも、検証もせずに「関数が呼ばれていない」と思い込んでしまったことを反省しましょう。
「これが原因」と思いこんでしまうことはベテランでもよくありますが、間違った思い込みをするとどんどん解決から遠ざかってしまうことになりかねません。

> 入力が何か間違っているということしょうか？
そこでいう「入力」とは具体的になんのことでしょう?

> 入力された数値を引数のポインタに代入する関数
ならばなぜ引数が２つあるのでしょう?

面倒なので言ってしまえば「input関数に与える引数が間違っている」のですが、
それは、input関数の設計がよろしくないことから誘発された間違いに思います。

[ryoutan134]

引数をなくして、要素数を固定し、戻り値で値を返しましたが。結果は変わりませんでした。

[thkana]

> 結果は変わりませんでした
適切な変更をしなかったから、だと思います

input関数が中でバッファを確保しているから取得したい桁数に1を足したものを引数に...
いや、そんなの考えなきゃ使えないような関数作っちゃだめでしょう。

int型って、普通のPCならたかだか符号含めて11桁。なら最初から文字バッファのサイズは12(終端文字含む)にしてしまえばよい。それをいちいち指定するメリットがどこかにありますか?
不要な引数を指定しなきゃ動かない->悪い設計です

なんらかの理由で桁数を制限したいなら、「数値を扱う関数」なのですから「取得したい桁数」を指定するようにすべきです。バッファのサイズ云々ってのは関数の側の内部処理に基づく身勝手な都合なのだから、それを関数の利用者に押し付けるべきではありません。自分自身ですらこのように間違ってしまうのですから...
意味のわかりにくい引数を指定させる ->悪い設計です

> 引数をなくして
合理的な理由で、明確な指針のもとに引数を要求するのは何ら悪いことではありません。なくせばいいというものではないです。

Answer 3

基本的にfgetsの使い方に問題があるようです。
1回の入力ではEnterキーが押下されるまでを読み取るようにするのが普通です。
とりあえず、動くようにしました。
//修正　のコメントがある箇所が、修正場所です。

#include <stdio.h>
#include<stdlib.h>
int input (int alleysize ,int* write)
    {
        int num;
        //char inputalley[alleysize];
        char inputalley[256];               //修正
        fgets(inputalley,256,stdin);		//修正
        num =  atoi(inputalley);
        *write = num;
        return 0;
    }
int main(void)
{
    int anser;
    do
    {
    int price = 0;
    int mode = 0;

    printf("定価を入力して下さい。\n");
    input(10,&price);

    printf("何を示しますか。\n0 : 税込み価格\nそれ以外 : 割引価格\n");

    input(1,&mode);

    if(mode == 0)	//修正
    {
        int tax = 0;
        printf("税率を入力してください。\n");
        input(2,&tax);
        price = (int)(price *(1+ tax/100));
        printf("税込み価格は%d円です。\n",price);
    }
    else
    {
        int tax = 0;
        printf("何割引きか入力してください\n");
        input(2,&tax);
        price = (int)(price * (1 - (tax/10)));
        printf("割引後の価格は%d円です。\n",price);
    }
    printf("まだつずけますか？。\n1 : はい\nそれ以外 : いいえ\n");
    input(1,&anser);
    }while(anser == 1);
    return 0;
}

Answer 4

if(mode = 0)

ここ、代入文なので、modeは、常に 0
従って、 false となり、 elseの方に行きませんか?

ぱっと、目についたところ。

コンパイル環境が分かりませんが、最近のだと、警告が出ないか?

Answer 5

✕　つずける
○　つづける

誰もツッコんでないみたいなので一応ｗ

「続ける」に変換できるかどうかで判断するのがひとつの手です。
teratailっぽいことを言うなら、コンパイラに文法ミスをチェックしてもらうのと同じことですね。

参考：なぜか変換できないとは (ナゼカヘンカンデキナイとは) [単語記事] - ニコニコ大百科 https://dic.nicovideo.jp/a/%E3%81%AA%E3%81%9C%E3%81%8B%E5%A4%89%E6%8F%9B%E3%81%A7%E3%81%8D%E3%81%AA%E3%81%84

array→alleyという間違いもそうですが、誤字が多いとそれだけ他のプログラマに伝わりにくくなったり、検索でヒットしなくなったりして保守性が下がりますし、何より「バカだと思われる」という、貴方のキャリアにとって重大なリスクが生じます。なのでなるべく直していきましょう。

Comments

[ryoutan134]

ありがとうございます。
気を付けます。

Answer 6

fgetsの仕様は「第2引数の数-1文字を読み取る」なので、

input(1,&mode);

で呼ばれた際、第2引数に1がわたるため1文字も読み取られず、modeが空っぽになります。

char inputalley[alleysize+1];
fgets(inputalley,alleysize+1,stdin);

として+1しておくか、そもそもinput関数の引数で配列サイズを決めるのではなく余裕を持った数にしておく

char inputalley[20];
fgets(inputalley,20,stdin);

などかと思います。ただしfgetsの仕様では改行文字も配列に格納されるようなのでその辺は注意が必要かもしれません(atoi関数がよろしくやってくれていそうですが)。

あと mode=0 は mode==0 で、 tax/100 は tax/100.0f などとしないと期待する結果は出なさそうです。

Comments

[ryoutan134]

前者の方法でやってみたものの結果は変わりませんでした。
また、バグと条件式は関係がないようです。

[abadcafe]

input(1,&mode);
の後ろに
printf("%d\n", mode);
を追加してmodeの値を表示（またはデバッガで変数の値を見る）し、回答者さんの変更の前後で、関数input()の動作がどう変わったかを確認してみることをお勧めします。