I Like
I Like

Reputation: 1847

Linked List node tail doesn't update when adding new nodes

I'm playing around with pointers for a Linked List class project, and can't wrap my head around how to create a link to a new node. I have a Linked List class which contains methods like append to manipulate the data structure. I want the nodes to be bids which are read from a csv file.

When I load all the data from the csv, I would like to

  1. create a new bid
  2. pass the new bid to the append function
  3. set the nextBid pointer of the Bid object and update the Linked List's tail

I would appreciate any pointers on creating a new address for each Bid object because now the tail node only 'remembers' the first bid's address.

Old Tail: 0x7ffeefbfee48
New Tail: 0x7ffeefbfee48
Old Tail: 0x7ffeefbfee48
New Tail: 0x7ffeefbfee48

I have copied my code below omitting the parts that weren't relevant like loading bids from csv file:

#include <algorithm>
#include <iostream>
#include <time.h>

#include "CSVparser.hpp"

using namespace std;

// forward declarations
double strToDouble(string str, char ch);

// define a structure to hold bid information
struct Bid {
    string bidId; // unique identifier
    string title;
    string fund;
    double amount;
    Bid* nextBid; //each bid has a pointer that can point to another bid
    Bid() {
        amount = 0.0;
    }
};

class LinkedList {
    
private:
    // FIXME (1): Internal structure for list entries, housekeeping variables
    Bid* head;
    Bid* tail;
    
public:
    LinkedList();
    virtual ~LinkedList();
    void Append(Bid bid);
    void Prepend(Bid bid);
    void PrintList();
    void Remove(string bidId);
    Bid Search(string bidId);
    int Size();
};

LinkedList::LinkedList() {
    // FIXME (2): Initialize housekeeping variables
    head=nullptr; //initialize head to point to nothing
    tail=nullptr;
}

void LinkedList::Append(Bid bid) { //<---I'm having trouble with this method
    // FIXME (3): Implement append logic
    if (this->head==nullptr){ //first node in a linked list
        cout << "initialize head and tail" << endl;
        this->head=&bid; //point to the bid
        this->tail=&bid;
    }
    else {
        cout << "Old Tail: " << this->tail << endl;
        this->tail->nextBid=&bid; //this refers to bid
        this->tail=&bid; //update last bid
        cout << "New Tail: " << &bid << endl;
        this->tail->nextBid=nullptr; //set pointer after last bid to null
    }
}

void displayBid(Bid bid) {
    cout << bid.bidId << ": " << bid.title << " | " << bid.amount
    << " | " << bid.fund << endl;
    return;
}

void LinkedList::PrintList() {
    // FIXME (5): Implement print logic
    //dont loop with the head, loop with bid name, because you dont want head pointer to change
    Bid* bid = this->head; //start at list's beginning
    cout << "List Head: " << this->head << endl;
    while(bid!=nullptr){
        displayBid(*(bid));
        cout << "Printing Address: " << bid << endl;
        bid = bid->nextBid; //move to the next bid
    }
}

Bid getBid() {
    Bid bid;
    //enter bid title, amount, etc.    
    return bid;
}

int main(int argc, char* argv[]) {
    
    // process command line arguments
    string csvPath = "eBid_Monthly_Sales_Dec_2016.csv";
    
    LinkedList bidList;
    
    Bid bid;
    
    int choice = 0;
    while (choice != 9) {
        cout << "Menu:" << endl;
        cout << "  1. Enter a Bid" << endl;
        cout << "  2. Load Bids" << endl;

        switch (choice) {
            case 1:{
                Bid addBid;
                cout << "new Bid Object's address is " << &addBid << endl; //address of the pointer
                bidList.Append(addBid);
//                displayBid(bid);
                bidList.PrintList();
                break;
            }
                

Upvotes: 2

Views: 1218

Answers (3)

Rajeev Singh
Rajeev Singh

Reputation: 3332

Here in line void LinkedList::Append(Bid bid) bid is a local variable and will be deallocated after the control returns from function.

Now here this->head=&bid; //point to the bid you are assigning the address of the local variable bid to head, which doesn't exist outside of the scope of the function Append. So won't give expected results.

What you can do is dynamically allocate the node and pass its address to the Append method.

void LinkedList::Append(Bid* bid) // function signature

adding node :

Bid addBid;
bidList.Append(&addBid);

Upvotes: 2

kyriakosSt
kyriakosSt

Reputation: 1772

The argument in append() shouldn't be a Bid, but a Bid *. This is because, since arguments are passed by value in functions, your bid object is just a copy of the original.
That means that when assigning &bid somewhere, that address will be the address of the parameter (the copied object) and not the original one.
So, when the function is done and all its memory (in the stack) gets "deallocated", so is the memory of that object, making your nodes pointing to garbage.

Upvotes: 1

Petr
Petr

Reputation: 267

The problem is that you are trying to assign a pointer to a temporary variable in the append function

void LinkedList::Append(Bid bid) { //<---I'm having trouble with this method
// FIXME (3): Implement append logic
if (this->head==nullptr){ //first node in a linked list
    cout << "initialize head and tail" << endl;
    this->head=&bid; //point to the bid
    this->tail=&bid;
}
else {
    cout << "Old Tail: " << this->tail << endl;
    this->tail->nextBid=&bid; //this refers to bid
    this->tail=&bid; //update last bid
    cout << "New Tail: " << &bid << endl;
    this->tail->nextBid=nullptr; //set pointer after last bid to null
}

You are passing a Bid object to the function, NOT a pointer, then you set the tail pointer to point at that object, but this object will be deleted after the function is over as it is created locally. Hence, tail will point on a deleted object, which will result in undefined behavior (personally I get 'Segmentation fault' on Linux). As an option you can pass a pointer to the Bid object to the function and everything will work normally as the pointer will be set to a valid Bid object declared outside the function.

void LinkedList::Append(Bid* bid) { //<---I'm having trouble with this method
// FIXME (3): Implement append logic
if (this->head==nullptr){ //first node in a linked list
    cout << "initialize head and tail" << endl;
    this->head=bid; //point to the bid
    this->tail=bid;
}
else {
    cout << "Old Tail: " << this->tail << endl;
    this->tail->nextBid=bid; //this refers to bid
    this->tail=bid; //update last bid
    cout << "New Tail: " << bid << endl;
    this->tail->nextBid=nullptr; //set pointer after last bid to null
}

Upvotes: 2

Related Questions