C++ Unable to Print Pointer Data of a Linked List

Question

I'm dealing with a doubly Linked List. It's made up of classes and is centered around the current node (instead of a node in the beginning or end of the list). Now my print function will throw an error but only if I have traversed the list at all. My print function just prints data in the current node (providing it's not null). Here's my print function: (much more detailed description of my file hierarchy and code at the bottom)

void queue::print(){
    if (current){
        std::cout << std::endl << std::endl << "+++++++++++++++++++ Webpage +++++++++++++++++++" << std::endl
            << "URL: " << current->data.getURL() << std::endl
            << "-----------------------------------------------" << std::endl 
            << "Title: " << current->data.getTitle() << std::endl
            << "-----------------------------------------------" << std::endl
            << "Content: " << current->data.getContent() << std::endl
            << "+++++++++++++++++++++++++++++++++++++++++++++++" << std::endl << std::endl;
    }
    else{
        std::cout << std::endl << "Your not on a page. Please navigate to a page first." << std::endl;
    }

Now, if I've filled the list with two nodes of data, and I execute my print function, it'll print the data in the node just fine. However, if I traverse to the previous node with my goBack() function:

void queue::goBack(){
    if (!current->previous){
        std::cout << "No previous page to go to!" << std::endl;
    }
    else{
        temp2 = current;
        current = current->previous;
    }
}

That will execute fine, but when I try to print the data in the node (with the same print function) I receive this error:

Unhandled exception at 0x003E7926 in Web Browser.exe: 0xC0000005: Access violation reading location 0xCDCDCE19.

and visual studio opens up a file without an extension type, with what looks like C code in it, called xstring, which has a break arrow pointing to line 1754 on it.

Now let me explain my code in a little more detail. I have five files: webQueue.h, webQueue.cpp, webPage.h, webPage.cpp, & main.cpp. All of the functions I've listed are in my webQueue.cpp file.

Here's webQueue.h:

#include "webPage.h"
class queue{
public:
    queue();
    void newPage(std::string u, std::string t, std::string c);
    void goForward();
    void goBack();
    void print();
private:
    struct Node{
        webPage data;
        Node* next;
        Node* previous;
    };
    Node* temp;
    Node* current;
    Node* temp2;
};

And here's webPage.h:

#include <string>
class webPage{
public:
    webPage();
    webPage(std::string u, std::string t, std::string c);
    std::string getURL();
    void setURL(std::string u);
    std::string getTitle();
    void setTitle(std::string t);
    std::string getContent();
    void setContent(std::string c);
private:
    std::string URL;
    std::string title;
    std::string content;
};

My webQueue.cpp file includes:

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

My webPage.cpp file just includes webPage.h and my main.cpp file (which contains my executing function) includes:

#include <iostream>
#include <string>
#include "webQueue.h"

Although these relations may seem a little convoluted, they should all be valid. The cpp files are linked to their header files with the exact same name (providing the header file exists), along with main.cpp being linked to webQueue.h and webQueue.h being linked to webPage.h. I can't see what I'm doing wrong with my code—although that's probably just because I'm having such a hard time understanding exactly how pointers work. I imagine that the error is in the code of my print(), goBack(), and goForward() functions (although I can't test my goForward() funtion until I fix my goBack() function) but I can't tell what's wrong.

Any help you guys can give would be greatly appreciated, as I am stumped. Here's a dropbox link to all the files so that you can test this program for yourself and see if I have any other functions with errors: https://www.dropbox.com/s/yekrz6dln1v9npk/webQueue.zip?dl=0

Answer 1

Your Node management is wrong.

One minor issue is that queue::goBack() and queue::goForward() are not checking if current is null before accessing its fields, so your code will crash if the user chooses to "go back" or "go forward" before choosing to "go to webpage" (at least print() is checking for null). So add those checks, and maybe even update printMenu() to not even output those options when the queue does not have a current page available.

But more importantly, your queue::newPage() implementation is completely broken. When current is not null, you are setting that node's next member to point at itself instead of the new node you have created, and you are not setting the new node's previous field at all, let alone to point at the existing node that it is being inserted after.

You should also get rid of the temp and temp2 members of the queue class. They do not belong there in the first place. They are only useful inside of queue::newPage() (queue::goBack() does not need to use temp at all, just like goForward()), so they should be changed into local variables inside of queue::newPage() only. Even better, they can just be removed altogether, as queue::newPage() can be implemented without using them at all.

Your queue implementation should look more like this instead (and this is not even counting copy/move semantics, either - see rule of three/five/zero):

#include "webPage.h"

class queue {
public:
    queue();
    void newPage(std::string u, std::string t, std::string c);
    void goForward();
    void goBack();
    void print();
private:
    struct Node {
        webPage data;
        Node* next;
        Node* previous;
    };
    Node* current;
};

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

queue::queue() {
    current = nullptr;
}

void queue::newPage(std::string u, std::string t, std::string c) {
    Node* n = new Node;
    n->data = webPage(u, t, c);
    n->next = nullptr;
    n->previous = nullptr;

    if (current) {
        if (current->next) {
            n->next = current->next;
            current->next->previous = n;
        }
        n->previous = current;
        current->next = n;
    }

    current = n;
}

void queue::goBack() {
    if ((current) && (current->previous)) {
        current = current->previous;
    }
    else {
        std::cout << "No previous page to go to!" << std::endl;
    }
}

void queue::goForward() {
    if ((current) && (current->next)) {
        current = current->next;
    }
    else {
        std::cout << "No next page to go to!" << std::endl;
    }
}

void queue::print() {
    if (current) {
        std::cout << std::endl << std::endl
            << "+++++++++++++++++++ Webpage +++++++++++++++++++" << std::endl
            << "URL: " << current->data.getURL() << std::endl
            << "-----------------------------------------------" << std::endl 
            << "Title: " << current->data.getTitle() << std::endl
            << "-----------------------------------------------" << std::endl
            << "Content: " << current->data.getContent() << std::endl
            << "+++++++++++++++++++++++++++++++++++++++++++++++" << std::endl << std::endl;
    }
    else {
        std::cout << std::endl << "You are not on a page. Please navigate to a page first." << std::endl;
    }
}

And then, while you are at it, you should rewrite queue to stop using manual node management altogether and use the standard std::list class instead:

#include "webPage.h"
#include <list>

class queue {
public:
    void newPage(std::string u, std::string t, std::string c);
    void goForward();
    void goBack();
    void print();
private:
    std::list<webPage> data;
    std::list<webPage>::iterator current;
};

#include "webQueue.h"
#include <iostream>
#include <iterator>

void queue::newPage(std::string u, std::string t, std::string c) {
    webPage p(u, t, c);
    if (data.empty()) {
        data.push_back(p);
        current = data.begin();
    }
    else {
        current = data.insert(std::next(current), p);
    }
}

void queue::goBack() {
    if ((!data.empty()) && (current != data.begin()))
        current = std::prev(current);
    else
        std::cout << "No previous page to go to!" << std::endl;
}

void queue::goForward() {
    if (!data.empty()) {
        std::list<webPage>::iterator iter = std::next(current);
        if (iter != data.end()) {
            current = iter;
            return;
        }
    }
    std::cout << "No next page to go to!" << std::endl;
}

void queue::print() {
    if (!data.empty()) {
        std::cout << std::endl << std::endl
            << "+++++++++++++++++++ Webpage +++++++++++++++++++" << std::endl
            << "URL: " << current->data.getURL() << std::endl
            << "-----------------------------------------------" << std::endl 
            << "Title: " << current->data.getTitle() << std::endl
            << "-----------------------------------------------" << std::endl
            << "Content: " << current->data.getContent() << std::endl
            << "+++++++++++++++++++++++++++++++++++++++++++++++" << std::endl << std::endl;
    }
    else {
        std::cout << std::endl << "You are not on a page. Please navigate to a page first." << std::endl;
    }
}