c - 我的输出不稳定

Question

所以基本上我想读取一个文本文件，其中的数字代表拍卖、正在出售的元素数量、有多少报价以及实际报价。我要做的就是输出最高报价作为售价，但由于某种原因，我的程序只打印出最后的价格，而不是最高的价格。

#include <stdio.h>
//main function
int main() {
    //Declaring variables and arrays
    float   numberBids[15], max, sum = 0 ,numberAuc;
    float   bids[10];
    int     i, y, j= 0, x, z = 0;
    char    filename[100]= "";

    //User puts in filename
    printf("Please enter the name of the file.\n");
    scanf("%s",&filename);

    //Opens file
    FILE * finp=fopen(filename,"r");

    //Scans info
    fscanf(finp, "%f" , &numberAuc);

    for(i=0; i < numberAuc; i++){

    fscanf(finp, "%f",&numberBids[i]);
        for (x = 0; x < numberBids[i];x++)

        {

        fscanf(finp, "%f", &bids[i]);
        max = bids[i];
        }
    //Replaces old max with newer one if larger
    for(j; j<numberBids; j++)
        if (bids[i]>max)
        max = bids[i];

    }

    //Sum
    sum += bids[i];
    //Print out to the output
    for ( y = 0; y <  numberAuc; y++ )
    {
        y = y+ 1;
        printf("Auction %d was sold for $%.2f\n", y, bids[z]);
        z++;
        y = y- 1;
    }
    //Close
    fclose(finp);

return 0;

}

这是文本文件的内容

5

4

100 500 250 300

1

700

3

300 150 175
2

920 680

8

20 10 15 25 50 30 19 23

Answer 1

您没有重新分配实际的出价金额。代码中有很多未使用和无意义的语句(我会用更多内容更新)，但主要部分是你并没有真正用你的 max 做任何事情 变量。

您的代码(带有一些添加的注释):

for (x = 0; x < numberBids[i];x++){
    /*Why are you using the `i` variable in a loop of `x`?*/
    fscanf(finp, "%f", &bids[i]);
    /*Setting the max to each as it's read in - doesn't do anything except waste cycles*/
    max = bids[i];
}

/*
* You are comparing j to a pointer, numberBids here.
* You're saying: "while j is less than some memory address"
*/
for(j; j<numberBids; j++){
    /*
    * You aren't using the j variable anyways, so you're comparing the same 
    * two numbers here every iteration (max and bid[i], which doesn't change
    * until the next auction since you are using `i`) 
    */
    if (bids[i]>max){
        max = bids[i];
    }
}

据我所知，您的代码只需要执行以下操作:

for each auction:
    get number of bids
    get bid amounts
    get the maximum of these bids

所以代码应该如下所示:

/*for each auction*/
for(int i=0; i < numberAuc; i++) {

    /*get number of bids*/
    fscanf(finp, "%f", &numberBids[i]);

    float max = 0;
    for (int x = 0; x < numberBids[i]; x++){
        /*get bid amounts*/
        fscanf(finp, "%f", &bids[x]);

        /*find the maximum*/
        max = bids[x] > max ? bids[x] : max;
    }

    printf("Auction %d was sold for $%.2f\n", i, max);
}

Full code file (with all fixes mentioned below)

<小时/>

其他注释(特别是关于代码风格/可读性):

• 与大括号保持一致。其中一些位于下一行，而其他则与需要确定作用域的语句(for 循环等)位于同一行。另外，它可能看起来更干净，但我个人建议在一行语句周围使用大括号，例如 for/if。如果您稍后需要扩展它，它们就在那里，而且如果您正确对齐，它还可以提高可读性...

• 可以在您使用变量的范围内声明变量，而不是在顶部全部声明。如今的编译器已经足够智能，可以优化初始化，因此您无需担心在循环中创建 100500 个 int ，并且预先声明预留空间现在也不必太担心。这是需要保持一致的其他事情 - 因此，如果您坚持将所有范围的声明放在函数的顶部，请对所有内容都这样做。

• 特别是因为您是初学者 - 将编译器警告视为错误(事实上，编译器有自动执行此操作的标志，以强制您修复它们)。它可以避免代码中的一些问题(例如将迭代整数与指针进行比较)。

• 获取文件名的方法被认为是不安全的 (scanf)。

这样做:

char filename[100];
fgets(filename, sizeof(filename), stdin);

使用 fgets 从 stdin 获取用户输入有一个警告 - 它包括按 ENTER 后的 \n 换行符。只需在该符号处终止字符串即可:

unsigned len = strlen(filename)-1;
filename[len] = '\0';

• 打开文件时，执行某种错误处理，以防 fopen 返回 NULL。如果它没有正确打开，则该程序中不会发生任何其他事情，因为它全部依赖于文件中的内容，而且您还会抛出一个空指针。

• 最后，使用int来存储整数。 float 可以工作，但是 float 可能......变化无常。 numberBids[15]、bids[10] 和 numberAuc 都可以而且应该是整数。