LinkedList memory error, help?

[m0rteum]

Hi, so I'm working on a project that requires me to add new nodes to a linked list that I can use to add and remove nodes from the beginning and end of the list. Right now I keep getting a memory error when I try to run my insertAsLast function. My program will crash on one line in particular. I'm pretty sure the line of code is correct, but I can't figure out what is causing the problem. The error I get in the debugger is that it is unable to read the memory of _entry and _next, and _last is <NULL>. The debugger says "Access location violation" or something to that effect. This does not happen with my insertAsFirst function, so I'm not sure what is different.

Here is my code.

#include "NewList.h"

#include <iostream>
#include <utility>

using std::ostream;
using std::cout;
using std::endl;

List::List()
	: _first(nullptr), _last(nullptr)
{
}


List::List(const List & other)
	: _first(clone(other._first)), _last(clone(other._last)), numberOfNodes(other.numberOfNodes)
{
}


List::~List()
{
	while (!empty())
	{
		removeFirst();
	}
}


const List & List::operator=(const List & other)
{
	if (&other != this)
	{
		this -> ~List();
		_first = clone(other._first);
	}

	return *this;
}


bool List::empty() const
{
	return _first == nullptr;
}


void List::insertAsFirst(double x)
{
	_first = new Node(x, _first);
	numberOfNodes++;
}

// Problematic functino

void List::insertAsLast(double x)
{
	if (!empty())
	{
		//cout << _last == nullptr << endl;
		_last->_next = new Node(x, nullptr);    //Problem line
		_last = _last->_next;
	}
	else
	{
		_first = new Node(x);
	}
	numberOfNodes++;
}

double List::removeFirst()
{
	double item = _first->_entry;
	Node * tempPtr = _first;
	_first = _first->_next;
	delete tempPtr;
	return item;
}


void List::removeLast()
{
	if (!empty())
	{
		Node * tptr = _last;
		if (tptr->_next != nullptr)
		{
			while (tptr->_next->_next != nullptr)
			{
				tptr = tptr->_next;
			}
			delete tptr->_next;
			tptr->_next = nullptr;
		}
		else{
			delete _last;
			_last = nullptr;
		}
		numberOfNodes--;
	}
	return;
}


int List::size()
{
	return numberOfNodes;
}


double List::sum()
{
	double s = 0.0;
	if (!empty()){
		Node * ptr = _first->_next;
		while (ptr != nullptr)
		{
			s = ptr->_entry;
			ptr = ptr->_next;
		}
		return s;
	}
	else{
		return 0;
	}
}

void List::print(ostream & outfile) const
{
	outfile << "[ ";
	if (!empty())
	{
		// The first entry is printed separately because no comma
		// is needed.
		outfile << _first->_entry;
		Node * ptr = _first->_next;
		while (ptr != nullptr)
		{
			outfile << ", " << ptr->_entry;
			ptr = ptr->_next;
		}
	}
	outfile << " ]";
}


Node * List::clone(Node * ptr)
{
	if (ptr == nullptr)
	{
		return nullptr;
	}
	Node * first = new Node(ptr->_entry);
	Node * last = first;
	ptr = ptr->_next;
	while (ptr != nullptr)
	{
		last->_next = new Node(ptr->_entry);
		last = last->_next;
		ptr = ptr->_next;
	}
	return first;
}


ostream & operator<<(ostream & outfile, const List & list)
{
	list.print(outfile);
	return outfile;
}

Thank you!

[m0rteum]

Really? Nobody?

[D80DSL3A]

You must assign _last = _first when the first node is inserted into the list, otherwise _last remains = nullptr.

[SECMoG1T]

If _last is the last node with a value == nullptr this line should be transformed
_last->_next = new Node(x, nullptr); //Problem line
to
_last= new Node(x, nullptr);

Also am in doubt of your clone function, if you really have to allow copy semantics on your class then i bet you'd rather change that definition of you class cnstor to copying the values themselves else objects that are copies of others will be invalidated when the ~dstructor is run on any one of them;