Двойной связанный список.Я делаю это хорошо? - PullRequest
2 голосов
/ 09 марта 2011

Я новичок в программировании на Си. У меня была задача создать список студентов, используя двойной связанный список. Приложение должно иметь три пункта: отобразить список, добавить нового студента и удалить студента по его идентификационному номеру. Я сделал это, и это работает очень хорошо. Я хотел бы задать несколько вопросов:

  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;
    }
}

Ответы [ 8 ]

2 голосов
/ 09 марта 2011
  • У вас есть утечка памяти, когда вы добавляете первый узел, вы выделяете оба addNode и firstNode
  • У вас есть утечка памяти, когда вы стираете студента, вам нужно free() удалить узел.
  • Вы должны использовать функцию для сравнения имен вместо дублированного кода. Что-то вроде int compareStudents(char * LeftFirstName, char * LeftLastName, char * RightFirstName, char * RightLastName);

Кроме того, логика хороша.

0 голосов
/ 22 июня 2011

Вы можете изменить прототип void Menu() на void Menu(void). Таким образом, компилятор может проверить, правильно ли вызвана функция или нет. Всегда хорошо, если ошибки уже могут быть найдены во время компиляции.

0 голосов
/ 09 марта 2011

Некоторые щупальца, которые я нашел

  • #include: в этой программе вам понадобятся <stdio.h>, <stdlib.h> и <string.h>
  • магические числа: используйте enumили #define вместо того, чтобы облагораживать ваш источник "магическими числами"
  • обращать внимание на переполнение буфера при использовании scanf и проверить возвращаемое значение
  • стиль: предпочитать '\n' в конце вывода: printf("...\n") против printf("\n...")
  • стараться не повторять strcmp s

скомпилировать с уровнем предупреждения, установленным на МАКС, и попытатьсяизбавиться от всех предупреждений.

0 голосов
/ 09 марта 2011

Выбор нескольких имен переменных делает код излишне запутанным. Например, в Show_List у вас есть что-то с именем firstNodeCopy, которое не является копией какого-либо узла и не всегда ссылается на первый узел.

В Add_Student у вас так много разных сравнений, я даже не уверен, чего вы на самом деле пытаетесь достичь, но я почти уверен, что у вас есть возможность для улучшения. Опять же, у вас есть некоторые переменные (например, start), имена которых, кажется, не очень убедительно указывают на то, что они действительно должны делать / быть.

Я бы посоветовал немного изменить структуру: создайте функцию compare_students (или что-то в этом роде), которая просто обрабатывает сравнение одного ученика с другим и (например) говорит вам, являются ли два ученика А и В в порядке или не в порядке. Возможно, я бы также добавил «создать учащегося», чтобы получить набор входных данных и создать из них структуру студента. Оттуда алгоритм вставки выглядит примерно так:

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
0 голосов
/ 09 марта 2011
  • Использование scanf небезопасно (входные данные могут переполнять целевой массив символов).Попробуйте scanf("%19s", &student_first_name);
  • Использование strcpy также небезопасно.Посмотрите на strncpy.
0 голосов
/ 09 марта 2011

Кратко рассмотрим функцию Add_Student (...). Похоже, небольшая утечка памяти. Я могу быть более конкретным, если хотите, но подумал, что вам может понадобиться практика.

0 голосов
/ 09 марта 2011

Это на самом деле очень хорошо!Признаюсь, после вашего представления я надеялся на беспорядок.

Вы проделали там очень хорошую работу, чувак.Все, что вы можете улучшить, вы узнаете в свое время :).Продолжайте в том же духе!

Я собирался предложить вам посмотреть, что такое «абстрактные типы данных» и как выставить интерфейсы, затрудняющие реализацию, но, как я уже сказал, вы узнаете об этом достаточно скоро!1005 *

Приветствия.

0 голосов
/ 09 марта 2011

Вы все отлично сделали! Хорошая работа, для начала! Продолжайте учиться.

...