C言語 構造体のソート

Question

C言語の構造体のソートについての質問です。

下記のtxtデータを構造体に入れてランキングを出すコードが上手く出力されません。
txtデータの詳細は、1列目が参加者ID,2-5列目がそれぞれ,勝ち,負け,引き分けの数と前回の順位になっていて、それぞれ空白区切りになっています。

A 28 42 30 6
B 40 60 0 7
C 45 45 10 1
D 36 54 10 4
E 63 27 10 3
F 36 54 10 5
G 56 24 20 2

ソートの仕方は以下の基準に基づいてソートをします。
基準 1:勝率が高い順.勝率は,勝ち数/(勝ち数 + 負け数) で算出し,小数点以下第 4 位を四捨五入するもの
基準 2:基準 1 で勝率が同じ場合,勝ち数が多い順.
基準 3:基準 1 で勝率が同じで,基準 2 で勝数も同じ場合,前回の順位が良い順.

下記のソースコードの出力だとこうなります。

E G C B F D A 

正しい出力は以下のとおりです。

E G C B D F A 

わかるかた、よろしくおねがいします。

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define N 256
typedef struct {
  char* name;
  int val[4];
  float winrate;
} item;

void print(item items[50], int id){
  for(int i = 0; i < id; i++) {
	printf("%s ", items[i].name);
  }
  printf("\n");
}

void Swap(item* item1, item* item2) {
  item tmp;
  tmp = *item1;
  *item1 = *item2;
  *item2 = tmp;
}

void sort(item items[50], int id) {
  for(int i = 0; i < id; i++) {
	for(int j = i + 1; j < id; j++) {
		if(items[i].winrate < items[j].winrate) {
		  Swap(&items[i], &items[j]);//基準1
          //printf("kijyun1\n");
		}else if(items[i].winrate == items[j].winrate){
            if(items[i].val[0] < items[j].val[0]){
                Swap(&items[i], &items[j]);//基準2
                //printf("kijyun2\n");
            }else if(items[i].val[0] == items[j].val[0]){
                if(items[i].val[3] < items[j].val[3]){
                    Swap(&items[i], &items[j]);//基準3
                    //printf("kijyun3\n");
                }
            }
        }
    }
  }
}


int main(int argc,char *argv[]){
	FILE *fp;
	item items[50];
	char fname=argv;
	char line[N];
	int i,id = 0;

	fp = fopen(argv[1], "r"); // ファイルを開く。失敗するとNULLを返す。
	if(fp == NULL) {
		printf("file not open!\n");
		return -1;
	}
	while(fgets(line, N, fp) != NULL) {
		items[id].name = (char*)malloc(sizeof(char) * 10);
		sscanf(line, "%s %d %d %d %d",  items[id].name, &items[id].val[0], &items[id].val[1], &items[id].val[2], &items[id].val[3]);
		items[id].winrate = 0;
        items[id].winrate = (float) items[id].val[0]/(items[id].val[0]+items[id].val[1]);
        //printf("%s:",items[id].name);
        //printf("%f\n",items[id].winrate);
        id++;
	}
	fclose(fp); // ファイルを閉じる
    sort(items, id);
    print(items, id);
	return 0;
}

Answer 1

まずアタリをつけるために
プログラムが出した順位と正しい順位を見比べると
基準３の前回の順位が良い順という部分で間違えている事が分かります

んで、それを基に判定部分を見ると

if(items[i].val[3] < items[j].val[3]){

上位が下位よりも前回の順位が小さければ交換しています。

順位は普通小さい方が良いので、ここの判定が逆だと分かります。

Comments

[Minochan]

回答ありがとうございます
単純なミスでしたね。ご指摘感謝します。

Answer 2

原因としては既にasmさんが回答されているとおりです。

---

このような比較関数を用意しておくと、ロジックがわりあいシンプルになりますよ。

int item_compare(item const *arg1, item const *arg2) {
    if(arg1->winrate < arg2->winrate) return  1;
    if(arg1->winrate > arg2->winrate) return -1;

    if(arg1->val[0] < arg2->val[0])   return  1;
    if(arg1->val[0] > arg2->val[0])   return -1;

    if(arg1->val[3] > arg2->val[3])   return  1;
    if(arg1->val[3] < arg2->val[3])   return -1;

    return 0;
}

void sort(item items[], int id) {
    for(int i = 0; i < id; i++) {
        for(int j = i + 1; j < id; j++) {
            if(item_compare(&items[i], &items[j]) > 0) {
                Swap(&items[i], &items[j]);
            }
        }
    }
}

少し改造するとqsortのコールバック関数としても扱えます。

---

ついでに。mallocに対するfreeがないことが非常に気になります。
この場合参加者IDの最大長を決め打って、配列で確保した方が楽なのでは。

Comments

[Minochan]

回答ありがとうございます。
このようにすれば簡略化になるのですね！
ご指摘感謝します。