Search code examples
c++linked-listnullnodes

Returning NULL from a getNext(), getPrev() or getData() function in a linked list


I am trying to create a linked list and am currently testing a print function but in order to print all the nodes in the list, I need to know the length.

In order to find the length, I need to use a for loop to cycle from head to whenever current is NULL (aka tail).

The issue is whenever a node in the list is set to NULL, it causes an error or nothing happens when I use getNext() or any of the other functions that are meant to return NULL.

I have tried for a long time to figure this out but I simply can't find what I'm doing wrong. It's probably a dumb mistake but I just can't find it.

Creating a new blank list:

LL::LL()
{
    // These are all protected variables
    head = NULL;
    tail = NULL;
    current = NULL;
}

Creating a new blank node:

Node::Node()
{
    // These are all protected variables
    data = v_t(); // Value_Type
    prev = NULL;
    next = NULL;
}

Creating a new node with arguments:

Node::Node(const v_t& d, Node* n, Node* p)
{
    data = d;
    prev = p;
    next = n;
}

Length function:

int LL::length()
{
    int answer = 0;
    for (current = head; current != NULL; current = current->getNext())
    {
        answer++;
    }
    return answer;
}

Adding a new node to the tail of the list:

void LL::addToTail(const v_t& item)
{
    // ------------(data, next, prev)
    tail = new Node(item, NULL, tail);

    if (tail -> getPrev() != NULL)
    {
        tail -> getPrev() -> setNext(tail);
    }

    if (head == NULL)
    {
        head = tail;
    }
}

Next return function:

Node* Node::getNext() const
{
    return next;
}

These are in the same format for the other two getters.

Below is the full classes:

node.h

#ifndef CHRIS_NODE
#define CHRIS_NODE

#include "account.h"

class Node
{
    public: // Members that are externally visible

        typedef Account v_t;

        // Default Constructor
        Node();
        Node(const v_t& d, Node* n, Node* p);

        // Destructor
        ~Node();

        // Pointer Getters and Setters
        void setNext(Node* n);
        void setPrev(Node* p);
        Node* getNext() const;
        Node* getPrev() const;

        // Data Getters and Setters
        void setData(v_t& d);
        v_t getData() const;

    private: // Members that are internally visible

        Node* next;
        Node* prev;
        v_t data;

};

#endif

node.cpp

#include"node.h"

Node::Node()
{
    data = v_t();
    prev = NULL;
    next = NULL;
}

Node::Node(const v_t& d, Node* n, Node* p)
{
    data = d;
    prev = p;
    next = n;
}

Node::~Node(){};

void Node::setNext(Node* n)
{
    next = n;
}

void Node::setPrev(Node* p)
{
    prev = p;
}

Node* Node::getNext() const
{
    return next;
}

Node* Node::getPrev() const
{
    return prev;
}

void Node::setData(v_t& d)
{
    data = d;
}

Node::v_t Node::getData() const
{
    return data;
}

linklist.h

#ifndef CHRIS_LIST
#define CHRIS_LIST

#include "node.h"

class LL
{
    public: // Members that are externally visible
    typedef Node::v_t v_t;

    LL();
    LL(Node*  h, Node* t, Node* c);
    ~LL();

    int length();

    void addToHead(const v_t& item);

    void addToCurrent(const v_t& item);

    void addToTail(const v_t& item);

    bool search(const v_t& target);

    void removeHead();

    void removeCurrent();

    void removeTail();

    void clear();

    void printList();

    protected: // Members that are internally visible
    Node* head;
    Node* tail;
    Node* current;
};


#endif

linklist.cpp

#include "linklist.h"
#include <iostream>

using namespace std;

LL::LL()
{
    head = NULL;
    tail = NULL;
    current = NULL;
}

LL::LL(Node*  h, Node* t, Node* c)
{
    head = h;
    tail = t;
    current = c;
}

LL::~LL()
{
    clear();
        
}

int LL::length()
{
    int answer = 0;
    for (current = head; current != NULL; current = current->getNext())
    {
        answer++;
    }
    return answer;
}

void LL::addToHead(const v_t& item)
{
    head = new Node(item, head, NULL);

    if (head -> getNext() != NULL)
    {
        head -> getNext() -> setPrev(head);
    }

    if (tail == NULL)
    {
        tail = head;
    }
}

void LL::addToCurrent(const v_t& item)
{
    Node* newNode = new Node(item, current, current->getPrev());
    current->setPrev(newNode);
    newNode->getPrev()->setNext(newNode);
    current = head;
}

void LL::addToTail(const v_t& item)
{
    tail = new Node(item, NULL, tail);

    if (tail -> getPrev() != NULL)
    {
        tail -> getPrev() -> setNext(tail);
    }

    if (head == NULL)
    {
        head = tail;
    }
}

bool LL::search(const v_t& target)
{
    for (current = head; current != NULL; current = current -> getNext())
    {
        if (target == (current -> getData()))
        {
            cout << "The data is stored in " << current << "." << endl;
            return true;
        }
    }
    return false;
}

void LL::removeHead()
{
    Node* temp = head;
    head = head -> getNext();
    if (head != NULL)
    {
        head -> setPrev(NULL);
    }
    else
    {
        tail = NULL;
    }
    delete temp;
}

void LL::removeCurrent()
{
    if (current == head) 
    {
        removeHead();
    }
    else if (current == tail)
    {
        removeTail();
    }

    current -> getNext() -> setPrev(current -> getPrev());
    current -> getPrev() -> setNext(current -> getNext());

    delete current;
    current = head;
}

void LL::removeTail()
{
    Node* temp = tail;
    tail = tail -> getPrev();
    if (tail != NULL)
    {
        tail -> setNext(NULL);
    }
    else
    {
        head = NULL;
    }
    delete temp;
}

void LL::clear()
{
    while (head != NULL)
    {
        removeHead();
    }
}

void LL::printList()
{
    if (LL::length() == 0)
    {
        cout << "List Empty.\n";
    }
    else
    {
        current = head;

        for (int i = 1; i <= LL::length(); i++)
        {
            if (current != NULL)
            {
                cout << "Node " << i << ": " << current -> getData() << endl;
                current = current -> getNext();
            }
        }
    }
}

account.h

#ifndef CHRIS_ACCOUNT
#define CHRIS_ACCOUNT

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

class Account
{
public:
    // Members that are externally visible

    // These are member functions

    // Constructor
    // Precondition:    none
    // Postcondition:   A new instance of account is created and its 
    //                  instance data initialsed to either zero or a 
    //                  parameter-provided value
    Account(const string nm = "", const double initialValue = 0.0);

    // Members that mutate data

    // Precondition:    acct_balance has been initialised
    // Postcondition:   amount is added to the acct_balance
    void deposit(const double amount);

    // Precondition:    acct_balance has been initialised
    // Postcondition:   amount is subtracted from the acct_balance
    void withdraw(const double amount);

    void setName(const string nm);

    // Members that query data

    // Precondition:    acct_balance has been initialised
    // Postcondition:   The value of acct_balance is returned
    double balance() const;

    // Precondition:    acct_balance has been initialised
    // Postcondition:   Returns true if acct_balance is greater 
    //                  than zero, false otherwise
    bool has_funds() const;

    string getName() const;


private:

    double acc_balance;
    string name;
};

bool operator == (Account acc1, Account acc2);

ostream& operator << (ostream& out, const Account acc);

// close the macroguard
#endif 

account.cpp

#include "Account.h"

Account::Account(string nm, double initialValue)
{
    acc_balance = initialValue;
    name = nm;
}

void Account::deposit(double amount)
{
    acc_balance += amount;
}

void Account::withdraw(double amount)
{
    acc_balance -= amount;
}

double Account::balance() const
{
    return acc_balance;
}

bool Account::has_funds() const
{
    if (acc_balance > 0.0) 
    {
        return true;
    } 
    else 
    {
        return false;
    }
}

string Account::getName() const
{
    return name;
}

void Account::setName(string nm) 
{
    name = nm;
}

bool operator == (Account acc1, Account acc2)
{
    if (acc1.getName() == acc2.getName() && acc1.balance() == acc2.balance())
    {
        return true;
    }
    else
    {
        return false;
    }
}

ostream& operator << (ostream& out, const Account acc)
{
    out << "(" << acc.getName() << ", " << acc.balance() << ")\n";
    return out;
}

bank.cpp

#include <iostream>
#include <cstdlib>
#include <string>
#include "Account.h"
#include "node.h"
#include "linklist.h"

using namespace std;

int main()
{

   int amount = 0;

   cout << "How many accounts?\n";
   cin >> amount;

   LL* LL1 = new LL();

   for (int i = 1; i <= amount; i++)
   {
       string nm;
       double iv;

       cout << "What is the name for account " << i << "?\n";
       cin >> nm;
       cout << "What is the initial value for account " << i << "?\n";
       cin >> iv;

       Account newAcc(nm, iv);
       LL1 -> addToTail(newAcc);
   }

    LL1 -> printList();

    return EXIT_SUCCESS;
}

Let me know if you need any more information or code :)


Solution

  • nothing happens when I use getNext() or any of the other functions that are meant to return NULL.

    That is because you did not use the result of getNext() in your length method. Your loop never assigns a new reference to current. Change this:

    for (current = head; current != NULL; current->getNext())
    

    to:

    for (current = head; current != NULL; current = current->getNext())
    

    It is an antipattern to define current as an instance member of the LL class. Such a reference should be a local variable, defined in the functions that need it. It should not be part of the state of your linked list.

    So wherever you use current, define it as a local variable:

    Node * current = head;
    

    in order to print all the nodes in the list, I need to know the length.

    Not necessarily. You could change your printList function to work without length:

    void LL::printList()
    {
        Node * current = head;
        if (current == NULL)
        {
            cout << "List Empty.\n";
        }
        else
        {
            for (int i = 1; current != NULL; i++, current = current -> getNext())
            {
                cout << "Node " << i << ": " << current -> getData() << endl;
            }
        }
    }
    

    If you keep the version where you call length and keep using the instance variable current, then the loop will not loop, because the call to length() will alter current (after your initialisation), and set it to NULL.

    If you really want to have the instance variable current, then only change it in the places where it really needs to be set, like in search. But it shouldn't be altered by length() or printList(), which are not supposed to change the state of your instance. So in length() and printList() you should use a local variable, not the instance variable.