c - C 结构数组上的冒泡排序未正确排序

Question

我有以下书籍记录程序，我想按书籍的名称对记录进行排序。该代码没有显示任何错误，但它没有对所有记录进行排序。

#include "stdio.h"
#include "string.h"
#define SIZE 5

struct books{                                      //define struct
        char name[100],author[100];
        int year,copies;
    };

struct books book1[SIZE],book2[SIZE],*pointer;          //define struct vars

void sort(struct books *,int);                      //define sort func

main()
{
    int i;
    char c;

for(i=0;i<SIZE;i++)             //scanning values
{
    gets(book1[i].name);
    gets(book1[i].author);
    scanf("%d%d",&book1[i].year,&book1[i].copies);
    while((c = getchar()) != '\n' && c != EOF);
}
pointer=book1;
sort(pointer,SIZE);                 //sort call

i=0;                        //printing values
while(i<SIZE)
{
    printf("##########################################################################\n");
    printf("Book: %s\nAuthor: %s\nYear of Publication: %d\nNo of Copies: %d\n",book1[i].name,book1[i].author,book1[i].year,book1[i].copies);
    printf("##########################################################################\n");
    i++;
}
}

void sort(struct books *pointer,int n)
{
    int i,j,sorted=0;
    struct books temp;
for(i=0;(i<n-1)&&(sorted==0);i++)       //bubble sort on the book name
{
    sorted=1;
    for(j=0;j<n-i-1;j++)
    {
        if(strcmp((*pointer).name,(*(pointer+1)).name)>0)
        {
            //copy to temp val
            strcpy(temp.name,(*pointer).name);
            strcpy(temp.author,(*pointer).author);
            temp.year=(*pointer).year;
            temp.copies=(*pointer).copies;

            //copy next val
            strcpy((*pointer).name,(*(pointer+1)).name);
            strcpy((*pointer).author,(*(pointer+1)).author);
            (*pointer).year=(*(pointer+1)).year;
            (*pointer).copies=(*(pointer+1)).copies;

            //copy back temp val
            strcpy((*(pointer+1)).name,temp.name);
            strcpy((*(pointer+1)).author,temp.author);
            (*(pointer+1)).year=temp.year;
            (*(pointer+1)).copies=temp.copies;

            sorted=0;
        }
                *pointer++;
    }
}
}

我的输入

The C Programming Language
X Y Z
1934
56
Inferno
Dan Brown
1993
453
harry Potter and the soccers stone
J K Rowling
2012
150
Ruby On Rails
jim aurther nil
2004
130
Learn Python Easy Way
gmaps4rails
1967
100  

和输出

##########################################################################
Book: Inferno
Author: Dan Brown
Year of Publication: 1993
No of Copies: 453
##########################################################################
##########################################################################
Book: The C Programming Language
Author: X Y Z
Year of Publication: 1934
No of Copies: 56
##########################################################################
##########################################################################
Book: Ruby On Rails
Author: jim aurther nil
Year of Publication: 2004
No of Copies: 130
##########################################################################
##########################################################################
Book: Learn Python Easy Way
Author: gmaps4rails
Year of Publication: 1967
No of Copies: 100
##########################################################################
##########################################################################
Book: 
Author: 
Year of Publication: 0
No of Copies: 0
##########################################################################

我们可以看到上面的排序是错误的？我做错了什么？

Answer 1

代码存在很多问题。最明显的语义错误是您弄乱了指针(您在内部循环中递增了指针，但从未重新设置它)，因此代码将通过读取和写入未分配的内存来在第二次迭代时调用未定义的行为.

第二个问题是你的冒泡排序算法是错误的——如果你正在进行冒泡排序，那么你必须从n - 1运行外循环。至0向下，否则第一次迭代将没有机会将第一个(也可能是最大的)元素移动到位。外循环的后续迭代继承相同的行为。

其余问题与可读性、风格、设计和安全性有关。一是你写了(*pointer).member看在上帝的份上，应该写成 pointer->member 。另一个类似的是 (*(pointer + index)).member ...那可能就是pointer[index].member 。缺乏正确的格式、缩进和空格是第三个问题，使用 #include ""而不是#include <>对于标准库头来说，第四个是。

使用gets()也是一个坏主意，因为它不允许您指定导致潜在缓冲区溢出的缓冲区大小。 fgets()应该始终是首选。

代码重复和冗余是不好的——你应该总是尝试通过将重复任务(例如复制 books 结构)拉到自己的结构中来进行程序分解(这是近乎亵渎的“重构”事情......)功能。命名变量是设计的另一个重要部分。不要称某物为 pointer ，这对读者没有帮助(在阅读您的代码时，我很长一段时间都想知道该指针什么是指向...的指针)

您还应该尽可能避免全局变量。就您而言，绝对不需要任何全局变量，因此没有什么真正证明它们的使用是合理的。另一个好的做法是将私有(private)辅助函数声明为 static这样就可以降低名称冲突的风险。

最后一点是:不要滥用评论。像这样的一行

sort(books, SIZE); // call sort function

没有帮助。还有什么可以sort(books, SIZE) 可能意味着(无论如何在精心设计的代码库中)比对 books 进行排序数组？

总而言之，这就是我编写代码的方式:

#include <stdio.h>
#include <string.h>

#define SIZE 5

struct book {
    char title[100];
    char author[100];
    int year;
    int copies;
};

static void sort(struct book *books, int n);

int main()
{
    int i;
    int c; // EOF is out-of-range for `char`, this MUST be an int
    struct book books[SIZE];

    for (i = 0; i < SIZE; i++) {
        fgets(books[i].title, sizeof books[i].title, stdin);
        fgets(books[i].author, sizeof books[i].author, stdin);
        scanf("%d%d", &books[i].year, &books[i].copies);
        while ((c = getchar()) != '\n' && c != EOF);
    }

    sort(books, SIZE);

    for (i = 0; i < SIZE; i++) {
        printf("##########################################################################\n");
        printf("Book: %s\nAuthor: %s\nYear of Publication: %d\nNo of Copies: %d\n", books[i].title, books[i].author, books[i].year, books[i].copies);
        printf("##########################################################################\n");
    }
}

static void swap(struct book *a, struct book *b)
{
    struct book tmp = *a;
    *a = *b;
    *b = tmp;
}

static void sort(struct book *books, int n)
{
    int i, j;
    for (i = n - 1; i >= 0; i--) {
        for (j = 0; j < i; j++) {
            if (strcmp(books[j].title, books[j + 1].title) > 0) {
                swap(&books[j], &books[j + 1]);
            }
        }
    }
}