双链表。我做的还好吗？

Question

我是 C 编程的初学者。我的任务是使用双链表创建学生列表。该应用程序应该有三点：显示列表、添加新学生和通过学生 ID 号删除学生。 我做到了并且运行得很好。 我想问几个问题：

1. 使用的东西是否不合适？
2. 如果有缩短代码的空间，我将很高兴收到任何建议。

这是我的代码：

struct student
{
    char first_name[20];
    char last_name[20];
    char ID_NO[14];
    struct student* previous;
    struct student* next;
};


void Menu();
void Show_List(struct student*);

struct student* Add_Student(char [], char [], char [], struct student*);
struct student* Erase_Student(char [], struct student*);

void main(void)
{
    char student_first_name[20];
    char student_last_name[20];
    char personal_code[14];

    struct student* first = NULL;
    struct student* node0 = NULL;

    int x = 0;
    int opt;

    Menu();
    for(; ;)
    {
        printf("\nEnter the operation you want to do: \n");
        scanf("%d", &opt);

        switch(opt)
        {
        case 1:
            Show_List(first);           
            break;
        case 2:
            printf("\nEnter the student's first name: ");
            scanf("%s", &student_first_name);
            printf("\nEnter the student's last name: ");
            scanf("%s", &student_last_name);
            printf("\nEnter the student's personal code: ");
            scanf("%s", &personal_code);
            node0 = Add_Student(student_first_name, student_last_name, personal_code, first);
            first = node0;
            break;
        case 3:
            printf("\nEnter the code of the student you want to erase: ");
            scanf("%s", &personal_code);
            node0 = Erase_Student(personal_code, first);
            first = node0;
            break;
        default:
            printf("\nYou entered an invalid option!!! Please try again.\n");
            Menu();
            break;
        }

    }
    scanf("%d", &x);

}

void Menu()
{
    printf("\nSelect from the Menu the operation you want to execute:\n");
    printf("\n1) Show students list;");
    printf("\n2) Add a student in the list;");
    printf("\n3) Erase a student from the list.");
}

void Show_List(struct student* firstNode)
{
    struct student* firstNodeCopy = firstNode;
    int number = 0;

    if(firstNode == NULL)
        printf("\nThe list is empty.\n");

    while(firstNodeCopy)
    {
        printf("\n%d. %s ", ++number, firstNodeCopy->first_name);
        printf("%s %s\n", firstNodeCopy->last_name, firstNodeCopy->ID_NO);
        firstNodeCopy = firstNodeCopy->next;
    }
}

struct student* Add_Student(char name_1[], char name_2[], char ID[], struct student* firstNode)
{
    struct student* start = firstNode;
    struct student* last = NULL;
    struct student* addNode = (struct student*) malloc(sizeof(struct student));

    if(firstNode == NULL)
    {
        firstNode = (struct student*) malloc(sizeof(struct student));
        strcpy(firstNode->first_name, name_1);
        strcpy(firstNode->last_name, name_2);
        strcpy(firstNode->ID_NO, ID);

        firstNode->next = NULL;
        firstNode->previous = NULL;
        return firstNode;
    }
    else
    {
        strcpy(addNode->first_name, name_1);
        strcpy(addNode->last_name, name_2);
        strcpy(addNode->ID_NO, ID);

        while(start)
            {
                if(strcmp(addNode->first_name, start->first_name) > 0)
                {
                    if(start->next == NULL)
                    {
                        start->next = addNode;
                        addNode->previous = start;
                        addNode->next = NULL;
                        return firstNode;
                    }
                    else
                    {
                        last = start;
                        start = start->next;
                    }
                }

                if(strcmp(addNode->first_name, start->first_name) < 0)
                {
                    if(last == NULL)
                    {
                        addNode->next = start;
                        start->previous = addNode;
                        return addNode;
                    }
                    else
                    {
                        addNode->next = start;
                        addNode->previous = last;
                        last->next = addNode;
                        start->previous = addNode;                  
                        return firstNode;                   
                    }
                }

                if(strcmp(addNode->first_name, start->first_name) == 0)
                {
                    if(strcmp(addNode->last_name, start->last_name) < 0)
                    {
                        if(last == NULL)
                        {
                            addNode->next = start;
                            start->previous = addNode;
                            return addNode;
                        }
                        else
                        {
                            addNode->next = start;
                            addNode->previous = last;
                            last->next = addNode;
                            start->previous = addNode;                  
                            return firstNode;
                        }
                    }

                    if(strcmp(addNode->last_name, start->last_name) > 0)
                    {
                        if(start->next == NULL)
                        {
                            start->next = addNode;
                            addNode->previous = start;
                            addNode->next = NULL;
                            return firstNode;
                        }
                        else
                        {
                            last = start;
                            start = start->next;
                        }
                    }

                    if(strcmp(addNode->last_name, start->last_name) == 0)
                    {
                        if(last == NULL)
                        {
                            addNode->next = start;
                            start->previous = addNode;
                            return addNode;
                        }
                        else
                        {
                            addNode->next = start;
                            addNode->previous = last;
                            last->next = addNode;
                            start->previous = addNode;                  
                            return firstNode;
                        }
                    }
                }
            }
    }
}

struct student* Erase_Student(char ID[], struct student* firstNode)
{
    struct student* start = firstNode;
    struct student* last = NULL;

    if(start == NULL)
    {
        printf("\nThe list is empty.\n");
        return NULL;
    }
    else
    {
        while(start)
        {
            if(strcmp(ID, start->ID_NO) == 0)
            {
                if(last == NULL)
                {
                    start = start->next;
                    return start;
                }
                else
                {
                    last->next = start->next;
                    return firstNode;
                }
            }
            else
            {
                last = start;
                start = start->next;
            }
        }
        printf("\nYou entered a WRONG personal ID number to erase!!! Please try again.\n");
        return firstNode;
    }
}

Answer 1

• 当您添加第一个节点时，您会发生内存泄漏，您分配了 addNode 和 firstNode
• 当您删除一个学生时，您会发生内存泄漏，您需要 free() 删除的节点。
• 您应该使用函数来比较名称而不是重复的代码。类似 intcompareStudents(char * LeftFirstName, char * LeftLastName, char * RightFirstName, char * RightLastName);

除此之外，逻辑很好。

Answer 2

你一切都做得非常好！干得好，刚开始！继续学习。

Answer 3

这其实非常好！我承认，在你的介绍之后，我希望事情会变得一团糟。

你在那里做得很好，伙计。无论你能改进什么，你都会在适当的时候学到:)。继续这样工作吧！

我本来打算建议您查找什么是“抽象数据类型”，以及如何公开掩盖实现的接口，但正如我所说 - 您很快就会学到这一点！

干杯。

Answer 4

快速浏览一下 Add_Student(...) 函数。看起来像是一个小内存泄漏。如果您愿意，我可以更具体，但我认为您可能需要练习。

Answer 5

• 您对 scanf 的使用是不安全的（输入可能会溢出目标字符数组）。尝试 scanf("%19s", &student_first_name);
• 使用 strcpy 也是不安全的。看看strncpy.

Answer 6

您选择的多个变量名称会使代码不必要地混乱。例如，在 Show_List 中，您有一个名为 firstNodeCopy 的内容，它不是任何节点的副本，并且并不总是引用第一个节点。

在 Add_Student 中，您进行了很多不同的比较，我什至不确定您真正想要完成什么，但我很确定您所拥有的内容还有待改进。同样，您有一些变量（例如，start），其名称似乎并不能非常明确地表明它们真正应该做什么/是什么。

我建议稍微改变一下结构：创建一个函数compare_students（或类似的东西），它只是处理一个学生与另一个学生的比较，并（例如）告诉你两个学生 A 和 B 是有序的还是无序的。我可能还会添加一个“创建学生”来获取一组输入并从中创建一个学生结构。从那里开始，插入算法看起来像这样：

if list is empty, put new node in list
if new node precedes first item in list, insert at head of list
Walk through list until end of list or current node precedes new node
      if at end of list, add new node to end
      else insert new node before current node

Answer 7

我发现了一些问题

• #include：在此程序中，您需要 、 和 >
• 幻数：使用 enum 或 #define 而不是在源代码中添加“幻数”，
• 使用时要注意缓冲区溢出scanf 和检查返回值
• 样式：更喜欢在输出末尾使用 '\n'：printf("...\n" ) vs printf("\n...")
• 尽量不要重复 strcmps

编译，并将警告级别推至 MAX，并尝试获取摆脱所有警告。

Answer 8

您可以将 void Menu() 的原型更改为 void Menu(void)。这样编译器就可以检查函数是否被正确调用。如果在编译时就可以发现错误，那总是好的。