Linked list definitions assignment C++

Question

One of my lab assignments at university is to read names from a *.txt file into a linked list of structures, and then pass the list to a function which prints the names to screen. The issue seems to be where I have assigned the value of the pointers and passing it to my function. I would appreciate if anyone could point out where I'm going wrong.

person.h:

#ifndef PERSON_H
#define PERSON_H
#include<string>
#include<fstream>
#include<iostream>
using namespace std;

struct Person
{
    Person *next;
    string name;
};

void walklist(Person *head_Ptr);

#endif PERSON_H

person.cpp:

#include "person.h"

void walklist(Person*head_Ptr)
{
    Person *cur;
    for (cur = head_Ptr; cur!=NULL; cur=cur->next)
    {
        cout<< cur->name<<endl;
    }
}

main.cpp

#include<string>
#include<fstream>
#include<iostream>
#include"person.h"

using namespace std;

int main()
{
    string myfilename, names_in;

    cout<<"Please enter the name of the file to open";
    cin>>myfilename;

    fstream infile;
    infile.open(myfilename.c_str());

    if(infile.bad())
    {
        cerr<<"There has been a problem opening the file"<<endl;
        system("PAUSE");
        return -1;
    }

    Person *head_Ptr = NULL, *last_Ptr = NULL, *temp_Ptr;

    while(infile.good())
    {
        getline(infile, names_in);

        temp_Ptr = new Person;
        temp_Ptr->name = names_in;
        temp_Ptr->next = head_Ptr;

        if(last_Ptr != NULL)
        {
            last_Ptr->next = temp_Ptr;
        }
        if(head_Ptr==NULL)
        {
            head_Ptr = last_Ptr;
        }
    }

    walklist(head_Ptr);

    system("Pause");
    return 0;
}

Using `using namespace std;` is not a good idea. http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice — Ed Heal, Jan 31 '16 at 13:19
Run the code step by step with the debugger and you will easily see your issue — Sami Kuhmonen, Jan 31 '16 at 13:19

Support Ukraine · Answer 1 · 2016-01-31T13:38:40.797

2

Shouldn't it be

    temp_Ptr->next = nullptr;  // temp_Ptr will be the new last element
                               // so make sure that its next points to null

    if(last_Ptr != NULL)
    {
        last_Ptr->next = temp_Ptr; // Update next pointer of the current last
                                   // element to point to the new last element
    }

    last_Ptr = temp_Ptr;           // Update last to be the new element

    if(head_Ptr==NULL)
    {
        head_Ptr = temp_Ptr;       // Update head if needed (i.e. when null)
    }

edited Jan 31 '16 at 13:38

answered Jan 31 '16 at 13:22

Support Ukraine

42,271
4
38
63

You just beat me to it :-) although I wans't sure if OP wanted a circular list or not. – Stefan Jan 31 '16 at 13:23
@Stefan - thanks for not just posting the same answer :-) Unfortunately many SO users do that. Even many minutes later. Anyway - regarding a `circular list` I assumed that to be a bug as the `walklist` would be problematic in its current form. – Support Ukraine Jan 31 '16 at 13:35
YOU GUYS ARE AWESOME!!! This was the only lab I've not been able to do this entire year and it was destroying my soul! it! It works perfectly and you've even commented the code. Thank you ya bunch of gents. – J0rdy Jan 31 '16 at 13:36

score 0 · Answer 2 · answered Jan 31 '16 at 13:23

0

Seems like you are on the right track. The error is in this code:

    if(last_Ptr != NULL)
    {
        last_Ptr->next = temp_Ptr;
    }
    if(head_Ptr==NULL)
    {
        head_Ptr = last_Ptr;
    }

The first time you get here, both last_Ptr and head_Ptr are null. So you skip the first assignment, but then assign last_Ptr to head_Ptr. Then they will still both be NULL.

answered Jan 31 '16 at 13:23

Bo Persson

90,663
31
146
203

What would i do instead then? – J0rdy Jan 31 '16 at 13:29
You don't set `last_Ptr` to anything, anywhere. Perhaps when adding the first node, `last_Ptr` and `head_Ptr` should be the same? – Bo Persson Jan 31 '16 at 13:33

Linked list definitions assignment C++

2 Answers2