Accessing Class info in a Linked List

[sadCoder]

My question has mostly to do with my printRecord function here.
I have no idea how to access the information within the class to display onto console. I've never tried using a linked list like this before and I wanted to experiment with the idea of a linked list class.

#include <iostream>
#include <string>

using namespace std;


class StudentRecord
    {
        public:
        string name;
        int id;
        double gpa;
    };

struct Node
{
    StudentRecord data;
    Node* next;
};

class LinkedList
    {
private:
    Node *head = nullptr;
    Node *tail = nullptr;
public:
    void addNode(Node*&head, StudentRecord data)
    {
        Node * newNode = new Node;
        newNode->data = data;
        newNode->next = nullptr;

        if (head == nullptr)//if head is empty then the data will be assigned to head
        {
            head = newNode; //since head isn't empy it will move onto else loop
        }
        else
        {
            Node *p = head; //assigning head p to headpointer
            while (p->next != nullptr) // Move the pointer to the last node
            {
                p = p->next; // Move one step at a time
            }
            p->next = newNode; // p points to the last node now. Connect the new node to it.
        }
    }



    };

void newStudent(StudentRecord* n)
{
    cout << "Enter name: ";
    cin >> n->name;
    cout << "Enter ID: ";
    cin >> n->id;
    cout << "Enter GPA: ";
    cin >> n->gpa;
}


void printRecord(Node* p)
{
    int v = 0;
    while (p != nullptr)
    {
        v++;
        cout << v << ". " << "NAME: " << p->next->data.name << ". " << "ID: " << p->next << ". " << "GPA: " << p->next << "." << endl;
        p = p->next;
        if(p == nullptr)
            break;
    }
}

int main()
{
    Node* head = nullptr;
    StudentRecord m;
    LinkedList ll;
    int option;
    while(true)
    {
        cout << "Please choose an option: " << endl;
        cout << "1. Enter new Student Record." << endl;
        cout << "2. Show all Students." << endl;
        cout << "3. End Program." << endl;
        cin >> option;
        if(option == 3)
        {
            cout << "Goodbye!" << endl;
            break;
        }

        switch(option)
        {
        case 1:
            {
                ll.addNode(head, m);
                newStudent(&m);
            }
            break;
        case 2:
            printRecord(head);
            break;

        }
    }

    return 0;
}

[jonnin]

cout << *p.data.name;

don't use next. if you had a list of 1,2,3,4 and you are on 2, you want to print 2, not 3. next is 3. Current, implied (p is current), is 2. 2.next is 3. 3.next is 4. 4.next is null, end of list.

names matter too. if you want to print one record, that is printrecord. You can search for it in a loop by its 'key' (some piece of data to identify it) but your printrecord is really coded like a printlist() ... it is trying to print all the records. Keep the names straight or it will confuse you more as you try to code it.

normally you would have some kind of find-record function that got you the node* you wanted, and printrecord could then print that for you. You split them up because you will want to find records for many reasons, not just print, and print may want to print a record that isnt even in a list, but the result of a find operation or a temp variable not even part of the list, it will still work.

[sadCoder]

cout << *p.data.name; doesn't seem to want to work. I tried it plainly as well. I get a no match for "operator*" error. Unless I was supposed to modify the parameter?
And also I don't need a search findRecord(). This is supposed to be relatively easy where is just prints the entire studentRecord list.

[sadCoder]

OK so it's coming along now. Only problem is that when I want to display the entire list it seems to display a nullptr first then the rest of the list except the last entry.

void printRecord(Node* p)
{
    int v = 0;
    while (p != nullptr)
    {
        v++;
        cout << v << ". " << "NAME: " << p->data.name << ". " << "ID: " << p->data.id << ". " << "GPA: " << p->data.gpa << "." << endl;
        p = p->next;
        if(p == nullptr)
            break;
    }
}

[Ganado]

First, a note about using a Linked list. The fact that a linked list is composed of nodes should not be a detail that the user should need to worry about.
All the user should need to know is that they have linked list, and they can add their data to the linked list (the data in this case being a StudentRecord).

        if(p == nullptr)
            break;

This check is redundant because you're already checking for nullptr at as the while condition.

But anyway,

                ll.addNode(head, m);
                newStudent(&m);

The m that you're modifying in newStudent is the not the same object that is currently in your linked list.
Try switching the order here (call newStudent first).

________________________

Other things:
- You have a class member variable called "head", but you also pass in a parameter called "head" which shadows your class member variable. It would make more sense if you didn't pass in any Node at all. The Node should just be an implementation detail, not needed by the user of the class.
- Your "printRecord" function does not print a single record as the name implies. It prints every item in your linked list.

[sadCoder]

GOT IT! Thank you to you both! Thank you for the guidance :)
Final product

#include <iostream>
#include <string>

using namespace std;


class StudentRecord
    {
        public:
        string name;
        int id;
        double gpa;
    };

struct Node
{
    StudentRecord data;
    Node* next;
};

class LinkedList
    {
private:
    Node *head = nullptr;
    Node *tail = nullptr;
public:
    void addNode(Node*&head, StudentRecord data)
    {
        Node * newNode = new Node;
        newNode->data = data;
        newNode->next = nullptr;

        if (head == nullptr)//if head is empty then the data will be assigned to head
        {
            head = newNode; //since head isn't empy it will move onto else loop
        }
        else
        {
            Node *p = head; //assigning head p to headpointer
            while (p->next != nullptr) // Move the pointer to the last node
            {
                p = p->next; // Move one step at a time
            }
            p->next = newNode; // p points to the last node now. Connect the new node to it.
        }
    }



    };

void newStudent(StudentRecord* n)
{
    cout << "Enter name: ";
    cin >> n->name;
    cout << "Enter ID: ";
    cin >> n->id;
    cout << "Enter GPA: ";
    cin >> n->gpa;
}


void printList(Node* p)
{
    int v = 0;
    while (p != nullptr)
    {
        v++;
        cout << v << ". " << "NAME: " << p->data.name << ". " << "ID: " << p->data.id << ". " << "GPA: " << p->data.gpa << "." << endl;
        p = p->next;
    }
}

int main()
{
    Node* head = nullptr;
    StudentRecord m;
    LinkedList ll;
    int option;
    while(true)
    {
        cout << "Please choose an option: " << endl;
        cout << "1. Enter new Student Record." << endl;
        cout << "2. Show all Students." << endl;
        cout << "3. End Program." << endl;
        cin >> option;
        if(option == 3)
        {
            cout << "Goodbye!" << endl;
            break;
        }

        switch(option)
        {
        case 1:
            {
                newStudent(&m);
                ll.addNode(head, m);
            }
            break;
        case 2:
            printList(head);
            break;

        }
    }

    return 0;
}

@Ganado I'm curious, how I could rewrite that function then? This is how I was taught to implement a linked list, which has always been the most confusing subject for me next to recursion.

[jonnin]

glad you got it.
I missed the (), this is correct:

(*p).data.name
p->data.name is also correct
p[0].data.name is also correct

*p. is not, it needs the ()

[Ganado]

how I could rewrite that function then? This is how I was taught to implement a linked list

As you have it right now, you never use the member variables of your class:

    Node *head = nullptr;
    Node *tail = nullptr;

So why do you have those there if you are not using them?


What I'm saying is that the end user shouldn't need to care about what the head or tail of a linked list is. They just need to know that a linked list holds StudentRecord objects.

So all you really need is:

LinkedList records;

while (true)
{
    // menu ... option 1
    StudentRecord record;
    enterStudent(&record); // slightly better name than "newStudent", but just my opinion.
    records.add(record); // the user doesn't need to worry about individual nodes like "head"

    // menu ... option 2
    records.print(); // the class already should know what the head node is, you don't need to pass it in
}

They way you have it now is probably fine for your assignment, but it is harder for the user to manage because it requires them to make both a "Node" and "LinkedList".

[seeplus]

There doesn't seem to be anything to free up the allocated memory used for the linked list? Whenever memory is allocated, it should be freed when no longer required. A possible skeleton class for LinkedList could be:

#include <string>
#include <iostream>

struct StudentRecord
{
	std::string name;
	int id;
	double gpa;
};

std::ostream& operator<<(std::ostream& os, const StudentRecord& data)
{
	// insert data into stream os as required
	return os  << "NAME: " << data.name << ". " << "ID: " << data.id << ". " << "GPA: " << data.gpa << "." << std::endl;
}

using T = StudentRecord;

class LinkedList {
public:
	LinkedList() {};

	~LinkedList() {
		// code here to traverse list and free all allocated memory
	}

	void addhead(const T&) {
		// code here to add new item at head of list
	}

	void addtail(const T& data) {
		// code here to add new item at tail of list
		// good practice is to keep tail pointing to the last mode of the list so that insertion
		// is done after tail so don't need to traverse the list to find the end to insert
		const auto newNode = new Node (data);

		if (head == nullptr)			// if head is empty then the data will be assigned to head
			head = newNode;		    // since head isn't empy it will move onto else loop
		else {
			auto p = head;				// assigning head p to headpointer

			while (p->next != nullptr)	// Move the pointer to the last node
				p = p->next;			// Move one step at a time

			p->next = newNode;		// p points to the last node now. Connect the new node to it.
		}
	}

	void displayall() const
	{
		// iterate the list and for each node display using std::cout << data
		for (auto nde = head; nde != nullptr; nde = nde->next)
			std::cout << nde->data;
	}

private:
	struct Node {
		Node* next = nullptr;
		T data {};

		Node(const T& _data) : data(_data) {}
	};

	Node* head = nullptr;
	Node* tail = nullptr;
};

int main()
{
	LinkedList list;

}

Also, LinkedList should have a copy constructor, a move constructor, a copy assignment and a move assignment as the class uses dynamic memory - so a deep copy is required and not a shallow copy undertaken by the defaults.

Also, normally LinkedList would be a templated class, but I doubt templates have been covered yet? This would allow LinkedList to work with any struct/class without knowing about ir. In the above example I just used a simple using statement to equate type T to StudentRecord.

[sadCoder]

@Ganado Oooh wow never knew it could be done like that. You're right. It does make it a LOT easier to manage. Thanks for the tip!

@seeplus this program was meant to be super simple. I could've used a 2d array to do the same thing but I wanted to give LL a try since I've always struggled with it. I've gone over templated classes already but I wanted to challenge myself a little with this.

[jonnin]

if you want some additional exercise, try making a binary tree; its similar to lists but in depth ;) (bad pun).

your program misses a key point. What is a list's strengths and weaknesses?
I know this was practice but to see the power of a list, you need to play to its strong points in your practice (my 2 cents, of course).

[seeplus]

@sadCoder So now extend the LinkedList class to add the copy/move functions required for a useful class - and the class destructor. Then add bi-directional links, node deletion etc. You'll learn a lot about C++ and move semantics.

But also note that in practice if a program requires use of a list, then the C++ STL list class would be used (or stack or queue etc).