scanfで文字列と整数を読み込む

Question

前提・実現したいこと

C言語で以下のことをするプログラムを作成しました。
「任意数の人が異なる正の整数が書かれたくじを引き、くじの番号を比較して小さい人から1位、2位とし、その一位と二位の人の名前を出力する。」
[入力例]
yamada 2560
okada 195
ishida 4532
osaki 210
[出力例]
okada
osaki

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

下に張り付けたコードを実行すると、どの場合でも二番目に入力した人の名前が二つ続けて出力されてしまいます。
上の例でいえば、
okada
okada
と出てしまいます。
なぜこうなるのか、そのようにすれば解決できるのか見当がつかないので教えていただきたいです。
どうかよろしくお願いいたします。
コンパイルと実行はできます。

該当のソースコード

#include <stdio.h>

struct member{ //くじを引いた人とそのくじの番号をセットにするための構造体
  char name[100];
  int lot;
};

int main(void){
    struct member people[5000]; //くじを引いた人を格納する配列
    int i = 0; //ループのための変数
    while(1){ //入力終了までループ回す
        scanf("%s %d",people[i].name,&people[i].lot);
        if(scanf("%s %d",people[i].name,&people[i].lot) == EOF){
            break;
        }
        i++;
    }
    int min = 10000;
    int j = 0;
    for(j = 0; j < i; j++){ //くじの値が最小である配列の番号をminに代入
        if(min > people[j].lot){
            min = j;
        }
    }
    int secmin = 10001;
    int k = 0;
    for(k = 0; k < i; k++){ //くじの値が二番目に小さい配列の番号をsecminに代入
        if(min < secmin && secmin > people[k].lot){
            secmin = k;
        }
    }
    printf("%s\n",people[min].name);
    printf("%s",people[secmin].name); //一位の人と二位の人の名前を出力
    return 0;
}

試したこと

nameとlotでscanfをわけたらだめなのかなと思ったので一緒にしてみたりしました。
またscanfの入力終了はEOFで表現できると知ったので使用しました。

Answer 1

まず、入力部分ですが、

    while(1){ //入力終了までループ回す
        if(scanf("%s %d",people[i].name,&people[i].lot) == EOF){

のように scanf はループ毎に一回入力するようにします。

　あと、min と secmin には、くじの値を入れていくべきですが、people のインデックスで更新してしまっているのでおかしくなっています。インデックスは、別の変数で保存するようにすると良いと思います。

【追記】
上記の処置を加えてもまだ２番目の人を選択する条件がおかしいです。

if(min < secmin && secmin > people[k].lot){

を

if(min < people[k].lot && secmin > people[k].lot){ 

に修正し、最も小さい値と対象者のくじの値と比べるようにします。

Comments

[PC_breakman]

#include <stdio.h>

struct member{
char name[100];
int lot;
};

int main(void){
struct member people[5000];
int i = 0;
while(1){
if(scanf("%s %d",people[i].name,&people[i].lot) == EOF){
break;
}
i++;
}
int min = 10000;
int minnum; //一位のインデックス保存用変数
int j = 0;
for(j = 0; j < i; j++){
if(min > people[j].lot){
min = people[j].lot;
minnum = j;
}
}
int secmin = 10001;
int secminnum; //二位のインデックス保存用変数
int k = 0;
for(k = 0; k < i; k++){
if(min < secmin && secmin > people[k].lot){
secmin = people[k].lot;
secminnum = k;
}
}
printf("%s\n",people[minnum].name);
printf("%s",people[secminnum].name);
return 0;
}
回答ありがとうございます。
ご指摘を受け、上記のように修正したのですが、今度は一位の人の名前が二回出力されるようになりました。一位の人のくじの値と二位の人のくじの値が同じになるはずはないのに、なぜこうなってしまうかわかりません。

[Yasumichi]

２番目のループの中での条件がおかしいです。

if(min < secmin && secmin > people[k].lot){
↓
if(min < people[k].lot && secmin > people[k].lot){

[PC_breakman]

解決いたしました。本当にありがとうございます。

[Yasumichi]

ちなみに secmin と secminnum の初期化を一つ目のループの前に移動し、一つめのループで min より小さい人が見つかった時点で前の値を

secmin=min;
secminnum=minnum;

のように退避すれば、一つ目のループだけで終わります。
　その場合、minnum と secnum は 5000 と 5001 ぐらいで初期化しておけば、良いでしょう。

[PC_breakman]

ご指摘の通りつくり変えてfor文一個バージョンでも書くことができました。とても勉強になります。本当に感謝しています。ありがとうございます。

Answer 2

nameとlotでscanfをわけたらだめなのかなと思った

なぜそう思ったのでしょう? 実際、そんなことはありません。
(scafが2行になったのはここの修正の仕方を間違ったのだと思いますが)

ついでに、お題の範囲なら配列も必要ありません。
以下のようにすれば、今どきのPCであれば20億人ぐらいは扱えます。
(「くじ」というのは同じ数は2つはでない、という理解をしましたがよろしいですか?)

#include <limits.h>
#include <stdio.h>

struct member {  //くじを引いた人とそのくじの番号をセットにするための構造体
  char name[100];
  int lot;
};

int main(void) {
  struct member peopleEntry, //入力用変数
  peopleMin = {.lot = INT_MAX}, //最小候補。intの最大数で初期化しておく
  peopleSecmin = {.lot = INT_MAX}; //2位候補
  while (1) {  //入力終了までループ回す
    scanf("%s", peopleEntry.name); //一度EOFが設定されると以降scanfは全てEOF
    if (scanf("%d", &peopleEntry.lot) == EOF) {
      break;
    }
    if (peopleMin.lot > peopleEntry.lot) { //最小更新?
      peopleSecmin = peopleMin; //今までの最小が2位にランクダウン
      peopleMin = peopleEntry; //最小更新
    } else if (peopleSecmin.lot > peopleEntry.lot) { //2位更新?
      peopleSecmin = peopleEntry; //2位更新
    }
  }
  printf("%s\n", peopleMin.name); //一位の人と二位の人の名前を出力
  printf("%s\n", peopleSecmin.name);
  return 0;
}

質問があればどうぞ。

Comments

[PC_breakman]

判定までループに入れてしまえば配列を用いる必要はないということですか?

[thkana]

n番目までで最小と2番目だけがわかっていればn+1番目のデータまで入力されたときの最小と2番目はわかりますから過去全部のデータを取っておく必要はありません。

[PC_breakman]

確かにそうですね。勉強になります。回答していただきありがとうございます。

[PC_breakman]

ご指摘の通り配列を作らない形でプログラムを組み、正常に動作させることができました。また、INT_MAX関数というのも知らなかったので今後は使ってみようと思います。
回答いただき本当にありがとうございました。

[thkana]

INT_MAXは関数ではありません。マクロ(数値の置き換え)です。
それほどは使う機会はないと思いますが...

[PC_breakman]

そうなんですね、勉強になります。本当にありがとうございます。

Answer 3

まず、(既に指摘がありますが)

    scanf("%s %d",people[i].name,&people[i].lot);
   if(scanf("%s %d",people[i].name,&people[i].lot) == EOF){

一行ごとの読み込みとなります。(奇数行が捨てられる)

min = j;

これでは、該当のインデックス(j)は保持されますが、最小値ではありません。従って min は、最初に 0が入って、それ以降は更新される事はありません。(最初のokada の原因。
それは、次の secmin を求めるループも同様。

あとは考えてみましょう。

[追記]
おっとこっちも忘れてた。 (既に指摘が出ていたが、、、)

if(min < secmin && secmin > people[k].lot){

デバッガで追うとかして、途中経過をみるようにしましょう。
デバッガ使えなければ、printf()とかで途中経過を出力する。
そうしないと、何が起きているか分かりません。

Comments

[PC_breakman]

解決いたしました。本当にありがとうございます。

Answer 4

まず、scanfが2つあるので、1つ目のscanfを削除します。

また、最小値を探すとろろで、minに「最小値自体」を入れるのか「最小値のインデックス」を入れるのかが混乱しています。
インデックスと値を比べても無意味です。

Comments

[PC_breakman]

解決いたしました。本当にありがとうございます。