0

for this school project we had to make a linked list out of information we got from a file with this information. 

Name:       ID:         GPA:
Alice       8234        2.7
mark        2672        3.0
Joanne      1234        3.7
John        3291        2.0
Tim         3212        3.8
Alford      1034        2.7
Benson      6290        2.5
Nancy       4234        3.7
Oscar       1672        1.0
Larry       1004        2.7

I got this information into a data structure. Now i am trying to use the id variable of the structure as the sorting key to the linked list, but i keep getting an error in my void list::addnode(node key) function with the variable key

#include<iostream>
#include<string>
#include<fstream>
#include<cstdlib>
using namespace std;

typedef struct node
    {
        string name;
        int id;
        float gpa;
        node *next;
    }*nodePtr; 
    node nodeinfo[10];

    nodePtr head;
    nodePtr current;
    nodePtr temp;
class list {
public:

    list();
    void addNode(node);
    void deleteNode(int delData);
    void displayList();
};
list::list() {
    head = NULL;
    current = NULL;
    temp = NULL;
}
void list::addNode(node key) {
    nodePtr n = new node;
    n->next = NULL;
    n->id = key;

    if (head != NULL)
    {
        current = head;
        while (current->next != NULL)
            current = current->next;

        current->next = n;
    }
    else
        head = n;
}
void list::deleteNode(int delData) {
    nodePtr delPtr = NULL;
    temp = head;
    current = head;
    while (current != NULL && current->id != delData) {
        temp = current;
        current = current->next;
    }
    if (current == NULL) {
        cout << delData << " is not in the list\n";
        delete delPtr;
    }
    else
    {
        delPtr = current;
        current = current->next;
        temp->next = current;
        delete delPtr;
        cout << "the value " << delData << "is no longer in the list\n";
    }
}
void list::displayList() {
    current = head;
    while (current != NULL)
    {
        cout << current->id << endl;
        current = current->next;
    }
}
int main() {
    fstream datafile("studentdata.txt");
    list studentinfo;
    node student;

    if (!datafile)
    {
        cout << "Error!!";
        exit(1);
    }
    for (int i = 0; i < 10; i++) {
        datafile >> nodeinfo[i].name >> nodeinfo[i].id >> nodeinfo[i].gpa;
    }
    studentinfo.addNode(student);

    return 0;
}
Joshua Mckinney
  • 1
  • 3
  • 6
    By making the `head` (and other variables) global, you can only have one list. Why don't you make those variables members inside the `list` class? – Some programmer dude Feb 04 '20 at 08:27
  • 4
    Your indentation is a bit misleading. Use an autoformatter (most IDEs have one) to get the structure clearer. – Ulrich Eckhardt Feb 04 '20 at 08:30
  • About [using namespace std](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice)... – and please don't typedef pointers, that's just information hiding without any real benefit. – Aconcagua Feb 04 '20 at 08:32
  • 1
    The error is because `n->id` is an int type to which your trying to assign your node type(`key`). – tangy Feb 04 '20 at 08:34
  • `n->id = key;` cannot work, `id` is of type `int`, `key` is of type `node`. If at all, you could do `*n = key`, but that would overwrite next pointer just set before as well. Why don't you just accept a `std::string` (as const reference) and an `int` as parameters? – Aconcagua Feb 04 '20 at 08:35
  • Your `deleteNode` function doesn't adjust the `head` pointer if you have to delete the very first element in the list. – Aconcagua Feb 04 '20 at 08:38
  • Just a recommendation: If you store the last node in an additional `tail` pointer, you don't have to iterate through the entire list to append a new node all the time... – Aconcagua Feb 04 '20 at 08:39
  • @Aconcagua do you think i should get rid of the class or combine the data structure with the class? – Joshua Mckinney Feb 04 '20 at 16:50
  • The `list` class? You absolutely should keep it. As the nodes don't really make sense without the surrounding list, you might consider making them a nested class, possibly even private. `head` (and `tail`, if you decide to implement) should be a class member of `list`, `current` and `temp` should only be *local* variables of the functions you use them in. Something like `class list { struct node { /*...*/ }; node* head; node* tail; public: void deleteNode() { node* current; node* temp; /* ... */ } };` – Aconcagua Feb 04 '20 at 16:55
  • As the two pointers would get member variables: Get used to implement the constructor's initialiser list (not to be confused with `std::initializer_list`) right from the start: `list::list() : head(nullptr), tail(nullptr) { }` – you prefer direct initialisation by value over default initialisation + separate assignment that way. For complex types, that can make a significant difference. Even more important: Some types *only* can be initialised that way (references, const members, non-default-constructible types). – Aconcagua Feb 04 '20 at 16:59
  • Please [edit] your code to reduce it to a [mcve] of your problem. Your current code includes much that is peripheral to your problem - a minimal sample normally looks similar to a good unit test: only performing one task, with input values specified for reproducibility. – Toby Speight Feb 04 '20 at 17:46

0 Answers0