Access Violation Reading Error

[Whorse]

...

[kbw]

You have to post the code for the member functions.

[Whorse]

...

[kbw]

I would expect Nth to start from the beginning (this) and follow the link forward n times.

Instead it creates a couple fo nodes (that it leaks), then finds the end, then walks backwards n times, then does some other loop that I don't understand.

I'd guess the problem is ithat algorithm, which I don't quite understand. If you think that algorithm's correct, can you explain it?

Also, Insert takes a ListNode by value, why not pass in a new one like all the other interfaces? The forces you to have two Insert functions. This sort of inconsistency only serves to confuse.

Finally, you call delete on some node, your data structure seems to be a single threaded linked list, so you've broken the link, and all following nodes are lost and the preceding node points to a delete block (a dangling reference).

[Whorse]

...

[kbw]

Nth should look list Last, right?.

	ListElement* Nth(unsigned n)
	{
		ListElement *current = this;

		for (unsigned i = 0; i < n && current->Next() != NULL; ++i)
		{
			current = current->Next();
		}

		return current;
	}

[Whorse]

That's what it looks like now yes. I still have the question regarding the safe deletion of ListElement and ListElements contained within itself. Any advice regarding that?

[kbw]

To remove an element, you have to:
1. Save the link it points to
2. Find the previous item and change its link to point to that item
3. Delete the element

To remove all:
while head is not null
1. store head-> next in tmp_next
2. delete head
3. store tmp_next in head

[Whorse]

I have followed your advice step by step, but still get the same error when I attempt to print root after deleting nth.

[kbw]

As it stands, Print(0) will attempt to print the entire list, but that shouldn't make it crash.

How many elements does it print before it crashes?

[Whorse]

It's unable to even print 1 element.

[kbw]

The string doesn't have enough space for the null terminator. The consturctor should copy the string with:

	ListElement(const char *elements) : mData(NULL), mPrev(NULL), mNext(NULL)
	{
		//Get the length of the string input into the constructor.
		int length = strlen(elements);
		//Assign enough memory to mNode to copy across the string.
		mData = malloc(length + 1);
		//Copy across the new string.
		strcpy(mData, elements, length);  // strcpy writes the null terminator, memcpy doesn't
	}

or simply:

	ListElement(const char *elements) : mData(NULL), mPrev(NULL), mNext(NULL)
	{
		mData = strdup(elements);
	}

You'll also need a destructor that releases the memory:

	~ListElement()
	{
		free(mData);
	}

[Whorse]

I'm unable to use malloc or strdup since as I said, mData is a ListElement. Instead I'm assigning length + 1 to strcopy. I'm now getting a corruption of heap at free(mData);

Any ideas?

[kbw]

Mmm ... think of it this way. A linked list is a chain of nodes, and each node has one or more links and the payload.

In your case, ListElement is the node, aPrev and aNext are your nodes and mData is the payload. As such, mData cannot be a ListElement. If this payload is a string, it must be a string type.

The easier way to do this is declare

    std::stirng mData;

The the constructor becomes:

	ListElement(const char *elements) : mData(elements), mPrev(NULL), mNext(NULL)
	{
	}

You don't need that destructor and if you keep it, remove free(mData).

And in Print, replace with:

			printf("%s", iterator->mData);

with:

			std::cout << iterator->mData;

or if you want to keep printf:

			printf("%s", iterator->mData.c_str());

[Whorse]

It's part of the brief, that's why. ListElement is not a char array, it's copied a string literal across via memcpy. The member is of type ListElement, and thus cannot be accessed by any functions that a string uses.

[kbw]

It doens't make sense that ListElement would have three members that are of type ListElement*.

And whether that makes sense or not, it's an error to memcpy onto a ListElement. That memcpy corrupts the heap and causes your crash later on.

[Whorse]

Even if I change the one member to a char array, I still receive the same error messages.

Just to clarify it has a bad pointer to the iterator for some reason, the actual data displays fine.

[kbw]

Can you show what you've changed?