1

I just started learning about linked list and I'm trying to extract certain info from a file and insert it into a linked list using a push function. When I try to view the info to see if it inserted correctly, it just displays the last line of the info over and over. What am I doing wrong? This is my code:

struct Country
 {
  string  name;
  double  population;
 };

struct Node 
 {
  Country ctry;
  Node *next;
 };
Node *world;

void push(Node *&world);

int main ()
{
    push(world);
    return 0;
}

void push(Node *&world)
{

    ifstream inFile("file.csv");

    if (!inFile.fail())
    {
        cout << "File has opened successfully." << endl;
    }

    if (inFile.fail())
    {
        cout << "File has failed to open." << endl;
        exit(1);
    }

   double temp, temp1, temp2, temp3, population;
   string countryName;
   Node *top = new Node;

   for (int i = 0; i < 300; i++)
    {
        if (inFile.eof())
        {
            top->next = NULL;
            break;
        }

        inFile >> temp >> temp1 >> temp2 >> temp3 >> population;
        getline (inFile,countryName);

        top -> ctry.population = population;
        top -> next = world;
        world = top;

        top -> ctry.name = countryName;
        top -> next = world;
        world = top;
    }

    for (int j = 0; j < 5; j++)
    {
        cout << top -> ctry.name << endl;
        top -> next;
    }
}
H.C
  • 9
  • 1
  • 8
  • See [Why is ostream::eof() bad?](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – Thomas Matthews Apr 21 '16 at 01:16

3 Answers3

1

"world" is the start of your linked list.

Node *top = new Node;

You created a new node here. I'll skip the part where you populated its contents.

    top -> next = world;
    world = top;

"world" is the current pointer to the start of the list, as I mentioned. You now saved it in top's next. Then you set world to point to the new node. This became the new start of your list. This is fine.

    top -> next = world;
    world = top;

You accidentally duplicated a few lines of code. Since "top" is, at this time, the same pointer as "world", you just set the node at the top of the list to point to itself. That's your infinite loop.

Sam Varshavchik
  • 84,126
  • 5
  • 57
  • 106
  • Nodes are really confusing for me and I got even more confused reading the last part of your comment. I tried switching top and world in the second part of the loop for the country name. – H.C Apr 21 '16 at 01:39
  • @H.C If the concept of nodes are confusing you, then maybe you should start out writing a linked list of a simple type, like `int`s. And before that, draw with boxes and lines how each operation will look. – PaulMcKenzie Apr 21 '16 at 01:48
0

You only allocate memory for one Node top, and use it all the time. Put below line in the for loop solve your problem:

   Node *top = new Node;
loafbit
  • 33
  • 6
0

Well, there are a few issues with your code I see.

First off, this line doesn't do anything:

top -> next;

If you are iterating through a linked list to read the values stored you would would probably want to do something along the lines of:

for (;;)
{
    cout << world -> ctry.name << endl;
    if (world->next == null) break;
       else world = world -> next;
}

You have a couple duplicated lines and circular link in your creation loop:

    top -> next = world;
    world = top;

    top -> ctry.name = countryName;
    top -> next = world;
    world = top;

I think this is what you meant to do:

world = top;
for (int i = 0; i < 300; i++)
{
    if (inFile.eof())
    {
        top->next = NULL;
        break;
    } else top = top -> next;

    inFile >> temp >> temp1 >> temp2 >> temp3 >> population;
    getline (inFile,countryName);

    top -> ctry.population = population;
    top -> ctry.name = countryName;
    top -> next = new Node;

}

Lastly, save yourself a function call and use an else:

if (!inFile.fail())
{
    cout << "File has opened successfully." << endl;
} else {
    cout << "File has failed to open." << endl;
    exit(1);
}

I hope this helps you get on the right track.

jbrove
  • 121
  • 3
  • I tried your for loops for creating the linked list and displaying the data in the linked list. It would display the last line and then break out of the loop. – H.C Apr 21 '16 at 03:13
  • Ok, the point was to get you started. Not to write your program for you. – jbrove Apr 21 '16 at 04:12