自作の関数を見てください。

質問

私が作った関数がこれでいけるか気になったので、
評価してください。
内容は、決められた文字が入力されるまでループを繰り返すというものです。
評価して欲しい内容・・・
1.関数名や変数名
2.strchrの使い方
3.その他だめなとこがあったらすべて
お願いします。
ソース(わかりやすいようにmain関数もつけます)：
#include<stdio.h>
#include<string.h>
#include<stdarg.h>
char Input_True_char(char *str,char *format,...);
int main(void)
{
char mozi;
mozi = Input_True_char("ny","あなたは%d才以上ですか？(y/n)\n",18);
if(mozi == 'y')
{
printf("18才以上です\n");
}
else if(mozi == 'n')
{
printf("18才未満です\n");
}
else
{
printf("バグ\n");
}
return 0;
}
char Input_True_char(char *str,char *format,...)
{
char mozi[3];
va_list args;
va_start(args,format);
do{
vfprintf(stdout,format,args);
rewind(stdin);
fgets(mozi,sizeof(mozi),stdin);
}while(mozi[1] != '\n' || strchr(str,mozi[0]) == NULL);
va_end(args);
return mozi[0];
}

回答No.2

hello_world

・3の補足
3の入力をファイルに拡張する事を前提とした場合問題は発生します。ファイルは
yEOF
と書けますよね?
また入力文字列が3文字以上の場合も改行コードがmoziに入りません。
まぁ、1文字入力だけを見るならばここは聞き流してください。

・なぜ文字ではなく文字列対応にすべきか
自作された関数は「質問に対して決められた回答文字以外は受け付けない」機能を有したものですよね?
その視点から見れば'y'や'n'だけでなく'キャンセル'や別の回答(例えば拡張して日本語回答にするかもしれない)を使用するケースもあるわけです。
>>ほとんど同じことを5ヶ所でしていたのでソースが見づらくなったのでそれならとわけて見ました。
と書かれている以上関数の意味合いを「まとめる」ものだけで見られているのかもしれません。関数は機能として作成するものであり「まとめられる」のはその特典の一つでしかありません。拡張性を失った時「まとめられる」特典は失われるからです。

>>GUIプログラムにして～
GUIにするのは一つの手段ですが、互換性を考えると得策ではないのはお判りかと。I/Fとしては誤入力を防ぐ効果は確かに高くユーザフレンドリーではあるのですが

回答No.1

hello_world

色々ツッコミどころはあります。
1.関数名・変数名
日本語読みか英語かハッキリさせましょう。通常は英語ですが、英語に自信が無いなら日本語読みでも可です。自分だけが使うなら下手な英語よりは害悪がありません。

2.strchr()の使い方
使い方としては別に良いのですが、後で述べますが使う場面を間違っているように見られます。

3.その他の指摘
・関数が複数の機能を有している…もしこの関数を入力がファイルから読み込んだものにする場合どうしますか?表示・入力チェックをキチンとばらしたモジュールにするか、せめて入力先は選択できる様にすべきでしょう。

・文字列に改行コードが入っているのが前提になっている…前項とほぼ同じ理由ですね。

・1文字入力しか想定していない…決められた文字が文字列の場合チェッカを別に作るのはナンセンスですよね?

・実はこの関数自体があまり必要ない…2と前項にも関わる事ですが'y'と'n'程度ならstrncmp()を使うべきでしょう。

補足

こんな頼みに回答してくれてありがとうございます。
3ですが入力はキーボードだけのつもりでした。
>>文字列に改行コードが入っているのが前提
fgets(mozi,sizeof(mozi),stdin);
としたならば、どんなに長い文字列を入れた場合でもsizeof(mozi)の場所に\0が入るのではないのでしょうか？
>>1文字入力しか想定していない
それは自分も思いました。しかしyesかno、switch文の1～9ぐらいに使うだけなら特に文字列にする必要もないかと・・・
>>実はこの関数自体があまり必要ない
ほとんど同じことを5ヶ所でしていたのでソースが見づらくなったのでそれならとわけて見ました。
ほかの回答は自分も思ってた部分もあります。
しかしここまでつっこまれるとは思ってもいませんでした。
やっぱしこういった無駄なものを作るぐらいならGUIプログラムにしてInputBoxとかでやらしたらいいのでしょうね(それにしたら必然的にYesとNoしか選べなくもなるし)