- 締切済み
プログラムの添削
以下のような数当てゲームを作りました.なるべくうまいプログラムを書けるようになりたいのですが,どのような改善点がありますか?よろしくお願いします. /*数当てゲームを作りなさい.*/ #include<stdio.h> void maegaki(void); /*このように関数を定義しまくることに意味はあるのか?main関数はすっきりするけど.*/ void in_check_out(int i); int main(void) { int i; int j; maegaki(); for(j=0;j<10;j++) { scanf("%d",&i); in_check_out(i); if(!(i-1)) return 0; printf("残り%d回です.\n",9-j); } return 0; } void maegaki(void) { printf("数当てゲームをはじめます.\nぼくの好きな整数を当ててください.\nチャンスは10回です.\nヒントはボゾン\n"); } void in_check_out(int i) { if(!(i-1)) { printf("正解!答えは1です.\n"); } else { printf("残念!\n"); if(i>1) printf("%dより小さいです.\n",i); else printf("%dより大きいです.\n",i); } }
- みんなの回答 (2)
- 専門家の回答
みんなの回答
- k_kota
- ベストアンサー率19% (434/2186)
関数に関しては、同じ処理を繰り返し使う場合には作った方がいいですし、 単発でも長い処理の場合には分けたほうがいいでしょう。 バグ取り時に全部同じ変数空間とか使ってるとやっかいですし、 そもそも処理のブロックを分けて明示的に別々にしてやったほうが後々いいです。 逆にいうとこれ単発で今後一切使わないならどんな書き方でもいいんじゃないでしょうか。 jのループですが、私だったらもうちょっと気の利いた名前にしてカウントダウンにしますかね while( chance-- > 0){ループ内容} なんでかというと、今のやり方だと回数を変更するとき9って数字のところをいじらないといけないのがめんどいのです。 あと、正解が変えるのにも対応させるなら他の方の言うとおり記載済みの整数値を変数にします。 関数自体を整数2つを引数にするか、差分を引数にするかの形式にして処理させる方が良いでしょうね。 この100倍の規模のプログラムなんてザラなんですが、そうなった時に変更がかったるくなる部分は作らないというのが一つのやり方ですかね。
- kmee
- ベストアンサー率55% (1857/3366)
> /*このように関数を定義しまくることに意味はあるのか?main関数はすっきりするけど.*/ ・まさに、そのため。長くてゴチャゴチャした関数は間違いのもと ・関数単位で動作確認できます。 ・同じコードを何度も書かなくて済みます ○scanf エラー対策してください。 人間は入力ミスをします。 http://ja.wikipedia.org/wiki/Scanf#.E7.95.B0.E5.B8.B8.E3.81.AA.E5.85.A5.E5.8A.9B.E3.81.8C.E8.A1.8C.E3.82.8F.E3.82.8C.E3.81.9F.E6.99.82.E3.81.AE.E5.87.A6.E7.90.86 などを参考に。「scanf エラー対策」等で検索を。 > if(!(i-1)) なんか、C言語マスターっぽく見える書き方ですが、この条件が何を意味するかがわからなくなってます。 「1と等しい」という条件なら、素直にi==1と書きましょう > if(!(i-1)) return 0; 同じ判定を2度してます。無駄です。 in_check_out関数の仕様を「正解なら1、不正解なら0を返す」と変更すれば、1回で済みます。 次のようにスッキリして、意味もわかりやすくなります。 if (in_check_out(i)) { break; /* ループから抜けるだけなので、breakでいいでしょう*/ } ○マジックナンバー 正解の1があちこちに直接書かれています。 3ヶ月後、これを5に変えることになったら、正確に変更できる自信はありますか?私にはありません。in_check_out関数とは離れたところにある、main関数の!(i-1)あたりは変更忘れそうです。 こういう定数はマクロや変数を使って名前をつけましょう。
お礼
なるほど!とても参考になりました.どうもありがとうございます!
お礼
回答ありがとうございます. 非常にためになります.