Связанный список, отсортированный программой C

Эта программа должна создавать отсортированный список и сортировать каждого пользователя по имени и фамилии. Я не могу понять, как правильно сортировать имена.

У меня проблема только с функцией append_to_list, остальные функции работают нормально.

Когда я впервые начинаю вводить имена:

user ID:    Last Name:    First Name:
3                Alex          Alex
2                Jones         Alex
1                andrew        john

это нормально, пока я не ввожу имя, которое должно сортироваться между двумя именами, когда я ввожу имя Эндрю, Алекс, это происходит.

 user ID:    Last Name:    First Name:
 4                Andrew        Alex
 3                Alex          Alex
 2                Jones         Alex
 1                andrew        john

но Эндрю, Алекс должен быть между пользователем 2 и 3


#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "user.h"
#include "readline.h"

// function append_to_list takes user input from user, id, name and puts it     to the end of the list
// as well as sorts each name alphabetically
struct user *append_to_list(struct user *family)
{
    struct user *cur, *prev, *new_node;

    // generate memory
    new_node = malloc(sizeof(struct user));

    if (new_node == NULL) { 
        printf("malloc failed in append\n");
        return family;
    }

    printf("Enter user ID: \n");
    scanf("%d", &new_node->number);
    printf("Enter user last name: \n");
    read_line(new_node->last_name,NAME_LEN);
    printf("Enter user first name: \n");
    read_line(new_node->first_name,NAME_LEN);

    for( cur=family; cur != NULL; cur = cur->next) {
        if (new_node->number == cur->number) {
                printf("user already exists: %s, %s\n",new_node->first_name,     new_node->last_name);
                free(new_node);
                return family;
        }   
    }       
    for (cur=family, prev = NULL; cur != NULL 
        && (strcmp(new_node->first_name,cur->first_name) < 0) 
        && (strcmp(new_node->last_name,cur->last_name) < 0); 
            prev = cur, cur = cur->next) { 

        if((strcmp(new_node->last_name,cur->last_name) < 0)) break;

        if((strcmp(new_node->first_name,cur->first_name) == 0)) 

        if((strcmp(new_node->first_name,cur->first_name) < 0)) break;
        ;
        }
        // use strcmp == 0 to see if name already exists
        if (cur != NULL && (strcmp(new_node->first_name,cur->first_name) == 0)
            && (strcmp(new_node->last_name,cur->last_name)) == 0) 
        {
                printf("user already exists: %s, %s\n",new_node->first_name, new_node->last_name);
                free(new_node);
                return family;
        }

    // append the linkedlist to the end
    new_node->next = cur;
    // check to see if the node is empty
    if (prev == NULL) {
        return new_node;
    } else  {
        prev->next = new_node->next;
        return family;
    }
}
// function delete_from_list removes a user from the family
struct user* delete_from_list(struct user *family) 
{
    struct user *prev, *cur;
    int id;
    int not_found = 0;
    printf("Enter user ID: \n");
    scanf("%d", &id);

    for (cur = family, prev = NULL; cur != NULL; prev = cur, cur = cur->next) {
        if (id == cur->number) {
                // if only one user on family
            if (prev == NULL) {
                family = cur->next;
            // if user is in the middle of family
            // connects prev node to cur node
            } else {
                prev->next = cur->next;
            }
            printf("user deleted: %s ,%s\n",cur->first_name, cur->last_name);
            free(cur);
        }
        else 
            not_found = 1;
    }
    if (not_found == 1) {
        printf("user not found\n");
    }
    return family;
}   
// function find_user searches the family by ID and matches it with the users name
void find_user(struct user *family)
{
    struct user *p = family;
    int id;
    int count = 0;
printf("Enter user ID: \n");
scanf("%d", &id);

    // compares the family with the user entered ID
    // if the ID is the same count set to 1
    if (p != NULL) {
        for (p = family; p != NULL; p = p->next) {
            if (id == p->number) {
                count = 1;
                break;
            }
        }
    }
    // if count is 1 we know the function found that specific user
    if ( count == 1) {
        printf("user found: %s, %s\n", p->last_name, p->first_name);
    } else {
        printf("user not found");
    }
}

// function printList prints the entire family
void printList(struct user *family)
{
    struct user *p;
    printf("user ID:\tLast Name:\tFirst Name:\n");
    for (p = family; p != NULL; p = p->next) {
        printf("%d\t\t%s\t\t%s\n", p->number, p->last_name, p->first_name);
    }

}

// function clearList clears the entired linked list
void clearList(struct user *family)
{
    struct user *p;
    while (family != NULL) {
        p = family;
        family = family->next;
        if (p != NULL) {
            free(p);
        }
    }
}

person lodam    schedule 24.11.2015    source источник
comment
Почему у вас есть сравнения имен и в условии завершения цикла, и в теле цикла в append_to_list()?   -  person John Bollinger    schedule 25.11.2015
comment
В любом случае, я думаю, что сравнения в условии завершения и в теле цикла оба неверны. Как минимум, случай с одинаковой фамилией обрабатывается неправильно в обоих местах.   -  person John Bollinger    schedule 25.11.2015


Ответы (4)


Проблема в том, как вы сравниваете узлы:

for (cur=family, prev = NULL; cur != NULL 
    && (strcmp(new_node->first_name,cur->first_name) < 0) 
    && (strcmp(new_node->last_name,cur->last_name) < 0); 
        prev = cur, cur = cur->next) { ... }

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

for (cur=family, prev = NULL;
     cur != NULL 
     && ((strcmp(new_node->first_name, cur->first_name) < 0) 
     ||  ((strcmp(new_node->first_name, cur->first_name) == 0)
     &&   (strcmp(new_node->last_name,cur->last_name) < 0))); 
     prev = cur, cur = cur->next) { ... }

И упростить последующий код. На самом деле вам не нужен никакой код. Цикл остановится в точке вставки. Просто проверьте, существуют ли одинаковые фамилия и имя (но что, если есть 2 John Does?) и вставьте между prev и cur или перед family, если prev равно NULL.

Цикл for выглядит уродливо: его трудно читать, легко ошибиться. Напишите отдельную функцию сравнения, которая принимает 2 узла и возвращает -1, 0, +1 в соответствии с порядком телефонной книги. Вы будете использовать эту функцию для append_to_list и delete_from_list, написав меньше кода, более удобочитаемого и последовательного.

person chqrlie    schedule 24.11.2015
comment
Функция сравнения, которую вы упомянули, на некоторых языках иногда называется оператором «космического корабля» ‹=›. - person ChuckCottrill; 25.11.2015
comment
@ChuckCottrill: Оператор космического корабля пришел из Perl. Я не уверен, что сожалею об отсутствии его в C... <=> был бы более полезен в C как оператор обмена между двумя lvalues. - person chqrlie; 25.11.2015
comment
Да, perl (числа), ruby ​​(stackoverflow.com/ вопросов/26581619/), php, python (обозначается как cmp) и ocaml (обозначается как сравнение). - person ChuckCottrill; 25.11.2015
comment
Оператор подкачки тоже не помешал бы. - person ChuckCottrill; 25.11.2015
comment
@ChuckCottrill: perl предшествовал всем этим, за исключением, возможно, caml, если бы у него вообще был этот оператор. Я не сторонник perl: если бы perl не существовало, его не следовало бы изобретать. Оператор подкачки был бы хорош для C и в духе C, поскольку он часто доступен в виде машинной инструкции и довольно громоздок для реализации в виде функции. - person chqrlie; 25.11.2015
comment
Оператор сравнения a <=> b может быть написан таким образом на C: (a > b) - (a < b), но аргументы оцениваются дважды, поэтому он не подходит для макроса. - person chqrlie; 25.11.2015

Я думаю, что лучше перекрыть операторы сравнения, а затем отсортировать список с помощью некоторых из существующих алгоритмов.

person Luis Daniel    schedule 24.11.2015

У вас проблема с условиями в этом цикле:

 for (cur=family, prev = NULL;
        cur != NULL && (strcmp(new_node->first_name,cur->first_name) < 0) 
                    && (strcmp(new_node->last_name,cur->last_name) < 0); 
                prev = cur, cur = cur->next) 

Цикл не будет выполняться:

(cur=family, family exist)
cur != NULL -> True
(Alex == Alex)
((strcmp(new_node->first_name,cur->first_name) -> 0) < 0 -> False
(Andrew > Alex)
((strcmp(new_node->last_name,cur->last_name) -> 1) < 0 -> False
True && False && False -> False

В результате вы добавите новую запись в начало.

Кроме того, в цикле у вас плохой код «если»:

if((strcmp(new_node->first_name,cur->first_name) == 0)) 

if((strcmp(new_node->first_name,cur->first_name) < 0)) break;
;

Док. стркмп

Совет. Удалите ненужный код. Ура!

person Community    schedule 24.11.2015

Цикл for остановится прямо в точке вставки (между предыдущим и текущим узлами).

Следующая часть будет сортировать список, даже если new_node больше, чем последний узел. Если новый узел больше, чем последний узел, cur становится нулевым, // что присваивается new_node_>next. поэтому prev равно cur; таким образом, предыдущий->следующий = новый_узел;

           for (cur=family, prev = NULL; cur!=NULL && ((strcmp(new_node->first_name,
           cur->first_name)>0) || ((strcmp(new_node->first_name,
            cur->first_name)==0)&&(strcmp(new_node->last_name,
            cur->last_name)>0))); prev = cur, cur = cur->next);

      new_node->next = cur;
           if(prev==NULL)
            return new_node;
           else {
           prev->next = new_node;
           return family;
           }
person LSSJ Λlpha    schedule 19.04.2020