Singly-Linked List Add Function - Read Access Violation

Question

I'm trying to create a basic singly-linked list using a separate Node class and LinkedList class. I barely know what I'm doing as I've just started learning C++, so any help would be greatly appreciated.

The LinkedList part of the code runs on its own, but I'm sure there are some corrections to be made there too. My main problem is that, when trying to add to the linked list, I'm getting (at line 64 of LinkedList.h):

Exception thrown: read access violation. this->head was nullptr.

I'm using Microsoft Visual Studio 2015. Here's the code:

LinkedList.h (it's inline):

#pragma once
#include <iostream>

using namespace std;

class Node
{
private:
    Node *next = NULL;
    int data;



public:
    Node(int newData) {
        data = newData;
        next = NULL;
    }
    Node() {

    }
    ~Node() {
        if(next)
            delete(next);
    }
    Node(int newData, Node newNext) {
        data = newData;
        *next = newNext;
    }
    void setNext(Node newNext) {
        *next = newNext;
    }
    Node getNext() {
        return *next;
    }
    int getData() {
        return data;
    }

};


class LinkedList
{

private:
    Node *head;
    int size;
public:

    LinkedList()
    {
        head = NULL;
        size = 0;
    }

    ~LinkedList()
    {

    }

    void add(int numberToAdd)
    {
        head = new Node(numberToAdd, *head);
        ++size;
    }

    int remove()
    {
        if (size == 0) {
            return 0;
        }
        else {
            *head = (*head).getNext();
            --size;
            return 1;
        }
    }

    int remove(int numberToRemove)
    {
        if (size == 0)
            return 0;
        Node *currentNode = head;
        for (int i = 0; i < size; i++) {
            if ((*currentNode).getData() == numberToRemove) {
                *currentNode = (*currentNode).getNext();
                return 1;
            }
        }
    }

    void print()
    {
        if (size == 0) {
            return;
        }
        else {
            Node currentNode = *head;
            for (int i = 0; i < size; i++) {
                cout << currentNode.getData();
                currentNode = currentNode.getNext();
            }
            cout << endl;
        }
    }

};

List Tester.cpp

    // List Tester.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <iostream>
#include "LinkedList.h"

using namespace std;

int main()
{
    LinkedList myList;
    myList.add(4);
    system("pause");
}

Answer 1

Your whole implementation is full of errors. It should look more like this instead:

#pragma once
#include <iostream>

class Node
{
private:
    int data;
    Node *next;

public:
    Node(int newData, Node *newNext = NULL)
        : data(newData), next(newNext)
    {}

    void setNext(Node *newNext) {
        next = newNext;
    }

    Node* getNext() {
        return next;
    }

    int getData() {
        return data;
    }
};

class LinkedList
{
private:
    Node *head;
    int size;

public:
    LinkedList()
        : head(NULL), size(0)
    {
    }

    ~LinkedList()
    {
        Node *currentNode = head;
        while (currentNode)
        {
            Node *nextNode = currentNode->getNext();
            delete currentNode;
            currentNode = nextNode;
        }    
    }

    void add(int numberToAdd)
    {
        head = new Node(numberToAdd, head);
        ++size;
    }

    bool remove()
    {
        Node *currentNode = head;
        if (!currentNode)
            return false;

        head = currentNode->getNext();
        delete currentNode;
        --size;

        return true;
    }

    bool remove(int numberToRemove)
    {
        Node *currentNode = head;
        Node *previousNode = NULL;

        while (currentNode)
        {
            if (currentNode->getData() == numberToRemove)
            {
                if (head == currentNode)
                    head = currentNode->getNext();
                if (previousNode)
                    previousNode->setNext(currentNode->getNext());
                delete currentNode;
                return true;
            }

            previousNode = currentNode;
            currentNode = currentNode->getNext();
        }

        return false;
    }

    void print()
    {
        Node *currentNode = head;
        if (!currentNode) return;
        do
        {
            std::cout << currentNode->getData();
            currentNode = currentNode->getNext();
        }
        while (currentNode);
        std::cout << std::endl;
    }    
};

Which can then be simplified using the std::forward_list class (if you are using C++11 or later):

#pragma once
#include <iostream>
#include <forward_list>
#include <algorithm>

class LinkedList
{
private:
    std::forward_list<int> list;

public:
    void add(int numberToAdd)
    {
        list.push_front(numberToAdd);
    }

    bool remove()
    {
        if (!list.empty())
        {
            list.pop_front();
            return true;
        }
        return false;
    }

    bool remove(int numberToRemove)
    {
        std::forward_list<int>::iterator iter = list.begin();
        std::forward_list<int>::iterator previous = list.before_begin();

        while (iter != list.end())
        {
            if (*iter == numberToRemove)
            {
                list.erase_after(previous);
                return true;
            }

            ++previous;
            ++iter;
        }

        return false;
    }

    void print()
    {
        if (list.empty()) return;
        std::for_each(list.cbegin(), list.cend(), [](int data){ std::cout << data });
        std::cout << std::endl;
    }    
};

Answer 2

You are making copies where you should not:

This:

Node(int newData, Node newNext) {
    data = newData;
    *next = newNext;
}

should be:

Node(int newData, Node* newNext) {
    data = newData;
    next = newNext;
}

Because now this:

head = new Node(numberToAdd, *head);

becomes this:

head = new Node(numberToAdd, head);

and will work even if head is a null pointer. You may need to adjust your other code accordingly.