0
#include <iostream>
#include <string>
using namespace std;

class Person{
public:
    string name;
    int age, height, weight;

    Person(string name = "empty", int age = 0, int height = 0, int weight = 0) {
        this->name = name;
        this->age = age;
        this->height = height;
        this->weight = weight;
    }
    Person operator = (const Person &P) {
        name = P.name;
        age = P.age;
        height = P.height;
        weight = P.weight;

        return *this;
    }

    void setAge(int a){
        age = a;
    }
    int getAge(){
        return age;
    }

    friend ostream& operator<<(ostream& os, const Person& p);
};

ostream& operator<<(ostream& os, Person& p) {
    os << "Name: " << p.name << "    " << "Age: " << p.age << "       " << "Height: " << p.height << "    " << "Weight: " << p.weight << "\n";
    return os;
};

class Node {
public:
    Person* data;
    Node* next;
    Node(Person*A) {
        data = A;
        next = nullptr;
    }
};

class LinkedList {
public:
    Node * head;
    LinkedList() {
        head = nullptr;
    }

    void InsertAtHead(Person*A) {
        Node* node = new Node(A);
        node->next = head;
        head = node;
    }
    void InsertAtEnd(Person*A) {
        if (head == nullptr) {
            InsertAtHead(A);
        }
        else {
            Node* node = new Node(A);
            Node* temp = head;
            while (temp->next != nullptr) {
                temp = temp->next;
            }
            temp->next = node;
        }
    }
    void InsertAtPosition(Person*A, int pos) {
        if (head == nullptr) {
            InsertAtHead(A);
        }
        else {
            Node* node = new Node(A);
            Node* temp = head;
            for (int i = 1; i < pos - 1; i++) { temp = temp->next; }
            node->next = temp->next;
            temp->next = node;
        }
    }
    void DeleteByValue(string search_name) {
        Node* temp = head;
        Node* prev = nullptr;
        while (temp != nullptr) {
            if (temp->data->name == search_name) {
                if (prev != nullptr) {
                    prev->next = temp->next;
                }
                else {
                    head = temp->next;
                }
                delete temp;
                temp = nullptr;
            }
            else {
                prev = temp;
                temp = temp->next;
            }
        }
    }
    void DeleteFromHead() {
        if (head != nullptr) {
            Node* temp = head;
            head = head->next;
            delete temp;
        }
    }
    void DeleteFromEnd() {
        Node* prev = nullptr;
        Node* temp = head;
        if (head == nullptr) { cout << "Nothing to delete" << endl; }
        else if (head->next == nullptr) { DeleteFromHead(); }
        else {
            while (temp->next != nullptr) {
                prev = temp;
                temp = temp->next;
            }
            prev->next = nullptr;
            delete temp;
        }
    }
    void DeleteAtPosition(int pos) {
        Node* prev = nullptr;
        Node* temp = head;
        if (head == nullptr) { cout << "Nothing to delete" << endl; }
        else if (pos == 1) { DeleteFromHead(); }
        else {
            for (int i = 1; i < pos; i++) {
                prev = temp;
                temp = temp->next;
            }
            prev->next = temp->next;
            delete temp;
        }
    }
    void UpdateAtPosition(Person*A, int pos) {
        if (head == nullptr) { cout << "No element in the list"; return; }
        if (pos == 1) { head->data = A; }
        else {
            Node* temp = head;
            for (int i = 1; i < pos; i++) {
                temp = temp->next;
            }
            temp->data = A;
        }
    }
    void UpdateByValue(string name, int newAge) {
        Node* temp = head;
        Person* p = new Person();

        while(temp != nullptr){
            if(temp->data->name == name){
                p->setAge(newAge);
            }else{
                temp = temp->next;
            }
        }
    }

    void Print() {
        Node* temp = head;
        while (temp != nullptr) {
            cout << *(temp->data);
            temp = temp->next;
        }
        cout << endl;
    }
};
int main() {
    LinkedList* list = new LinkedList();
    list->InsertAtHead(new Person("Samantha", 20, 63, 115));                  list->Print();
    list->InsertAtEnd(new Person("Chris", 19, 70, 200));                      list->Print();
    list->DeleteByValue("Chris");                                             list->Print();
    list->UpdateByValue("Samantha", 21);                                      list->Print();

    return 0;
}

I am new to C++ so excuse any poorly written code, but I am trying to use the function UpdateByValue to update the age of Samantha. It may look very wrong right now, but I have tried 20 different things and cannot figure out what I am doing wrong. I used to go to school at a community college where I learned Java so Im catching up to everyone in C++. A lot of it is similar but I struggle with little things like this. Could anyone explain to me how to fix the UpdateByValue function so that it will change the age of my Person object of choice? I want to be able to type the name as the first parameter and change the age of that person with the second parameter. If something is unclear and needs more explaining please let me know, I just need help. Thanks in advance, and please feel free to give any other constructive criticism. I am trying to get as good as I can.

Fall0ut
  • 71
  • 1
  • 12
  • 2
    [Your rubber duck](https://en.wikipedia.org/wiki/Rubber_duck_debugging) wants to know why after finding the correct `temp` you change the age of `p`. – user4581301 Mar 09 '18 at 23:49
  • I expect that your code is running forever? 2 things. 1: learn to use a debugger - stepping through the code will find this. 2: look at the loop in `UpdateByValue`, is that loop guaranteed to terminate? You will find that that is not the case – Justin Mar 09 '18 at 23:49
  • @user4581301 like I said it may be rather messed up right now but I am very stuck. I had other ideas I already tried. – Fall0ut Mar 09 '18 at 23:49
  • 1
    @Justin good point. So obviously I need to set temp to nullptr once name is found, and will setAge work? I do know I need to learn to use a debugger, thats my goal for the weekend. – Fall0ut Mar 09 '18 at 23:51
  • 1
    Ok, stating the problem more-plainly, You want to update Samantha. You find Samantha. And then rather than updating Samantha, you update something else. Why would you do this? Why would you not update Samantha? – user4581301 Mar 09 '18 at 23:51
  • @user4581301 well that is part of my issue. I realize that, thats why I have a getAge, setAge, but I cant figure out how to delete or override the current age of Samantha. – Fall0ut Mar 09 '18 at 23:53
  • @Justin okay thank you that fixed it from crashing, but now I need help setting the age which was my primary issue. Im guessing its within that same loop that needs to be fixed. Do I need setters/getters or am I having a brain fart? – Fall0ut Mar 09 '18 at 23:54
  • @Fall0ut I believe you missed the point. In the loop, you find Samantha, but then rather than saying, "Samantha, your age is now `newAge`", you are saying, "SomeOtherPerson, your age is now `newAge`" – Justin Mar 09 '18 at 23:54

1 Answers1

2

Let's take a walk through UpdateByValue. I'll comment as we go.

void UpdateByValue(string name, int newAge) {
    Node* temp = head;
    Person* p = new Person();

    while(temp != nullptr){ // keep looking until end of list
        if(temp->data->name == name){ // found node with name
            p->setAge(newAge); // update a different node
            // never advance node so we can't exit function
        }else{
            temp = temp->next;
        }
    }
}

Try instead

void UpdateByValue(string name, int newAge) {
    Node* temp = head;
    // Person * p   is not needed 

    while(temp != nullptr){ // keep looking until end of list
        if(temp->data->name == name){ // found node with name
            temp->data->setAge(newAge); // update the found node
            return; // done searching. Exit function
        }else{
            temp = temp->next;
        }
    }
}
user4581301
  • 33,082
  • 7
  • 33
  • 54
  • YES, this worked. I was unaware that I could write temp->data->setAge. Im still trying to understand the difference between temp.data and temp->data. In Java I only ever used the period reference. Is writing return better than setting temp to nullptr? – Fall0ut Mar 09 '18 at 23:58
  • 1
    Both would do the same thing. On one hand there is a school of thought that believes there must only be one `return` statement and that it must be at the end of the function, so setting temp to `nullptr` is the right decision. On the other hand, I believe that a `return` statement is more descriptive of the intent and will lead to fewer interpretation mistakes by other programmers. – user4581301 Mar 10 '18 at 00:03
  • thank you for all your help on this question, very much appreciated. – Fall0ut Mar 10 '18 at 00:06
  • 1
    @Fall0ut It's pretty widely accepted in this time that `return` is the better choice. Also, the difference between `->` and `.` is roughly that you have to use `->` with pointers and `.` with non-pointers. You may know that `*pointer` *dereferences* the pointer; `pointer->member` is equivalent to `(*pointer).member` – Justin Mar 10 '18 at 00:07
  • 1
    In Java just about everything is a pointer, but they don't tell you and hide this by having everything use the same syntax. There's a certain beauty in that, but also some potential pain. A node may not actually contain data, and this extra pointer chaising can really slow a program down. But in C++ you have the ability to build the data right into the node, so if you have the node, you have the data. Eg: `class Node { public: Person data; Node* next; };`. When you construct a `Node` you also construct a `Person` for a one-time cost, but now you know that `Person` is there. – user4581301 Mar 10 '18 at 00:12
  • Yeah I wish I had just started with C++ because it makes everything much more understandable. Im definitely understanding things a lot better in C++ than in Java. I more memorized the code in Java where in C++ I now understand how and why things are being written so I can rewrite them myself. Also thank you Justin. I also keep being told by other I need a deconstructor which I never used in Java (probably doesnt exist from what I am aware) and that I keep having memory leakage. – Fall0ut Mar 10 '18 at 00:16
  • 1
    You don't have to keep chasing it down every single time you iterate through the node. It's disgusting how expensive it can be to have to go get something from RAM. When you release the `Node`, it also releases the `Person` You can't screw up and forget. In Java you can't screw up and forget because Java's memory management is based around the idea you [have infinite memory](https://blogs.msdn.microsoft.com/oldnewthing/20100809-00/?p=13203) and the garbage collector gobbles it up. – user4581301 Mar 10 '18 at 00:22
  • 1
    In C++ there is a concept called [Resource Allocation is Initialization](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii). Use it to tie down ownership of (who is responsible for managing and ultimately releasing) storage and other resources. It is one of your best friends because it makes everything dead predictable. In Java you never know when a destructor will run, but in C++ you can. Let a file object go out of scope and the file closes. Right there. No calling `close`. No `finally` block. It's all done in the destructor. – user4581301 Mar 10 '18 at 00:26
  • @Fall0ut Sorry I went on a rant there. – user4581301 Mar 10 '18 at 00:27
  • 1
    No I greatly appreciate it. I really love coding and I'm still a noob in college, but I am working my ass off, so I love getting as much tips as possible. A guy yesterday told me it was bad practice to use "using namespace std;" All these tips will put me ahead of the game im hoping. I learned operation overloading and we haven't done that in college yet. Its been a huge help. – Fall0ut Mar 10 '18 at 00:29
  • Herb Sutter had a great presentation a while back called leak free by design. About an hour long, but good watching. That was hard: https://www.youtube.com/watch?v=JfmTagWcqoE The stuff he goes over will save you insane amounts of debugging. – user4581301 Mar 10 '18 at 00:31