Связанный список показывает только первый элемент узла при печати

Я пытаюсь создать связанный список, чтобы улучшить свои концепции указателей и адресов. Мне нужно создать связанный список следующим образом:

(1) Прочитайте все узлы сразу на терминале.

(2) Затем покажите окончательный связанный список, сформированный таким образом.

Как я пытаюсь это сделать? Сначала я читаю размер связанного списка (общее количество узлов, которые необходимо ввести). Затем я читаю все узлы по одному в цикле do-while. Прочитав все узлы, я пытаюсь создать связанный список. Я различаю случай, когда узел является первым узлом, созданным переменной count, которая будет иметь count=0, когда узел является первым узлом, после чего он будет в другом цикле.

Вывод, который я получаю, выглядит следующим образом:

 enter the size of node
4
start entering the number of elements until your size
2
3
4
5
Printing linked list
2-> //It don't print the other nodes, Just first one
hp@ubuntu:~/Desktop/pointer$ 

Мой полный код для этого:

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

struct node 
{
    int freq;
    struct node * next;
};
typedef struct node node;
node * tree;

void main() 
{
    int size, data;
    int count = 0; //this count flag is to check is it's first node or not inside the do-while loop.
    tree = NULL;
    printf("enter the size of node\n");
    scanf("%d", & size);
    printf("start entering the number of elements until your size\n");
    node * temp3 = tree;
    node * prev;
    //Problem creating area is below
    do
    {
        scanf("%d", & data);
        if (count == 0)
        {
            node * temp;
            temp = (node * ) malloc(sizeof(node));
            temp-> freq = data;
            temp-> next = NULL;
            prev = temp;
        } 
        else if (count != 0) 
        {
            node * temp;
            temp = (node * ) malloc(sizeof(node));
            temp-> freq = data;
            temp-> next = NULL;
            prev-> next = temp;
        }
        size--;
        ++count;
    }
    while (size > 0);

    printf("Printing linked list\n");
    node * temp1;
    temp1 = prev;
    //there may be problem here
    while (temp1-> next != NULL) 
    {
        printf("%d-> ", temp1-> freq);
        temp1 = temp1-> next;
    }
    printf("\n");
}

Может ли кто-нибудь помочь мне распечатать полный связанный список, указав мне на ошибку с ее решением?


person user252990    schedule 06.03.2014    source источник
comment
Несколько замечаний: main должен что-то возвращать или иметь значение void, по моим подсчетам у вас есть три неиспользуемых указателя (дерево, temp3 и хранилище), а в вашем else if вы устанавливаете prev node на temp, а затем выделяете memory для temp и в основном перезаписывает то, на что он указывает.   -  person James H    schedule 06.03.2014
comment
Отладчик, отладчик, отладчик...   -  person Martin James    schedule 06.03.2014
comment
@JamesHostick Я сохранил void main() в своем редактировании. Но проблема в том, как избежать этой перезаписи узлов, потому что я читаю все узлы вместе.   -  person user252990    schedule 06.03.2014
comment
Я скажу, начните с разделения большого количества кода на несколько функций.   -  person Yu Hao    schedule 06.03.2014
comment
А также лучше не использовать void main(), просто использовать int main().   -  person James H    schedule 06.03.2014
comment
Немного сбивает с толку то, что вы полагаетесь на prev, чтобы указать на первый элемент. Это на самом деле здесь, но не должно ли оно действительно быть обновлено до значения temp и, таким образом, указывать на предыдущий элемент, как следует из названия?   -  person 500 - Internal Server Error    schedule 06.03.2014
comment
@JamesHostick Спасибо, я удалил node * temp= prev; в условии else if даже тогда он печатает первый узел.   -  person user252990    schedule 06.03.2014
comment
@ 500-InternalServerError извините, я не понял, не могли бы вы объяснить подробнее?   -  person user252990    schedule 06.03.2014
comment
На самом деле ответ Арчи ниже показывает, к чему я клоню.   -  person 500 - Internal Server Error    schedule 06.03.2014
comment
@ 500-InternalServerError эта переменная указателя запуска должна быть временной в моем случае (если я объявляю node * temp) вне цикла do-while? Я пробовал, что в этом случае он даже не печатает первый узел (ничего не печатает)   -  person user252990    schedule 06.03.2014


Ответы (2)


Хорошо, есть некоторые ненужные указатели и несколько ошибок указателей, для простоты ответа я переписал ваш код, я попытаюсь объяснить, что я сделал здесь:

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

struct node
{
    int freq;
    struct node * next;
};
typedef struct node node;
//only need two pointers when building a linked list, one for the top and one for the
//current node
node *tree = NULL, *curr = NULL; //init both pointers to NULL initially

int main()
{
    int size, data; //dont need count, you'll see in a minute why
    printf("enter the size of node\n");
    scanf("%d", & size);
    printf("start entering the number of elements until your size\n");

    //Problem creating area is below
    do
    {
        scanf("%d", &data);
        if (tree == NULL) //just test for top node being NULL instead of using count
        {
            node *temp;
            temp = malloc(sizeof(node));
            temp->freq = data;
            temp->next = NULL;
            //stylistically i like using curr rather than prev, just a style choice
            tree = temp; //set tree to first node
            curr = tree; //make the top node the current node
        }
        else //don't need else if, there are only two conditions
        {
            node *temp = malloc(sizeof(node));
            temp->freq = data;
            temp->next = NULL;
            curr->next = temp; //set the next node in list to the new one
            curr = curr->next; //here's where you had pointer issues, move the current
                               //to the newly created node
        }
        size--;
    }
    while (size > 0);

    printf("Printing linked list\n");
    curr = tree; //reuse curr, no need to make a new pointer

    //test for the current node being NULL, takes care of special case of empty list
    //causing a segfault when you attempt to access a member of an invalid pointer
    while (curr != NULL)
    {
        printf("%d->", curr->freq);
        curr = curr->next; //move to next item in list
    }
    printf("\n");
    return 0;
}

Я запустил пробный запуск с размером 3 и входными данными 1, 2 и 3, и я получаю на выходе: 1-> 2-> 3->

person James H    schedule 06.03.2014
comment
Я не проверял ошибки с помощью malloc, но хорошей практикой является проверка указателя после malloc, чтобы убедиться, что он выделил память, иначе вы должны вывести какое-то сообщение об ошибке. - person James H; 06.03.2014
comment
Нет проблем, я люблю указатели, я ненавижу указатели и люблю ненавидеть указатели. - person James H; 06.03.2014
comment
почему нам нужно переместить cur на новый узел, выполнив это curr = curr-›next? мы уже установили связь cur с новым узлом строкой curr-›next = temp; этого шага должно быть достаточно, чтобы соединить связь между двумя узлами. не могли бы вы объяснить немного подробнее? Спасибо - person user252990; 06.03.2014
comment
цель curr — отслеживать, где вы сейчас находитесь в списке. строка curr-›next = temp добавляет вновь созданный узел в конец списка. Затем curr = curr-›next перемещает вашу текущую позицию на последний узел в списке, то есть на новый узел, который мы только что добавили в конец списка. В противном случае вы в конечном итоге перезапишете текущий узел, а не добавите его в конец (именно поэтому вы получили только один узел в списке, независимо от того, сколько элементов вы вводили пользователю). - person James H; 06.03.2014
comment
@JamesHostick спасибо еще за одно сомнение, в чем разница между while (curr != NULL) и while (curr-›next!= NULL)? Я попытался узнать, напечатав результаты, поэтому в случае while (curr-› next != NULL) последний элемент не печатается. Но это должно было быть сделано, потому что мы делаем это до тех пор, пока следующий из последних узлов не станет NULL. - person Sss; 06.03.2014
comment
Заботится об особом случае, когда список пуст (top равен нулю), в противном случае, если список пуст, у вас возникнут проблемы при попытке сказать temp-›anything, потому что указатель не будет действительным. - person James H; 06.03.2014

У тебя две проблемы.

else if (count != 0) 
    {
        node * temp = prev;
        temp = (node * ) malloc(sizeof(node));
        temp-> freq = data;
        temp-> next = NULL;
        prev-> next = temp;
    }

Вы не меняете prev, чтобы он указывал на новый узел. В вашем сценарии он по-прежнему указывает на «2», и у вас никогда не будет более двух узлов в списке.

Попробуйте что-то вроде

else if (count != 0) 
    {
        /* node * temp = prev; */ //This code is not doing anything useful
        temp = (node * ) malloc(sizeof(node));
        temp-> freq = data;
        temp-> next = NULL;
        prev-> next = temp;
        prev = temp;
    }

Затем ваш цикл печати, вероятно, должен быть

node* temp1 = start; //You need a variable that points to the first node in the list
do 
{
    printf("%d-> ", temp1-> freq);
    temp1 = temp1-> next;
} 
//The last item will always have next == NULL, and must be included
while (temp1-> next != NULL); 
person Archa5238    schedule 06.03.2014
comment
Одно примечание для третьего блока кода: при доступе к членам указателя структуры вы должны убедиться, что указатель действителен, прежде чем выполнять доступ. В противном случае вы получите segfault. Скорее всего, этого не произойдет, если start является действительным указателем, но вы никогда не можете быть уверены, что пользователь ничего не напутал. :) - person James H; 06.03.2014
comment
@Archa Спасибо, но я попробовал оба, которые вы только что сказали, ни один из них не сработал - person user252990; 06.03.2014
comment
Я не использую start , я использую дерево. - person user252990; 06.03.2014