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 :)
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.