2

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

#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
##########################################################################

大家可以看出上面的排序是错误的吗？我做错了什么？

4

1 回答 1

2

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

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

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

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

代码重复和冗余是不好的——你应该总是尝试通过将重复任务（例如复制books结构）拉出到它们自己的函数中来尝试程序分解（这几乎是亵渎神明的“重构”事情......）。命名变量是另一个重要的设计部分。不要调用 a pointer，这对读者没有帮助（在阅读您的代码时，很长一段时间以来，我一直想知道那个指针是指向什么的指针...... ）

您还应该尽可能避免使用全局变量。在您的情况下，绝对不需要任何全局变量，因此没有任何理由真正证明它们的使用是合理的。另一个好的做法是声明私有辅助函数，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]);
            }
        }
    }
}
于 2013-10-23T20:18:41.283 回答