-1

I am new to C++ and only know Java.

I am trying to create a singly linked list in C++ that will allow me to store integer numbers within the nodes. I want also to have the IntNode class and IntLinkedList class in separate files from the main file.

However, it's not working, I think it has something to do with my add() method for my IntLinkedList class.

This is the main.cpp file:

#include <iostream>
#include "IntNode.hpp"
#include "IntLinkedList.hpp"

int main(int argc, const char * argv[])
{
    //Initializing Variables
    IntLinkedList myList;
    
    myList.add(5);
    
    std::cout << "-------Done.------";
    
    return 0;
}

This is the IntNode.cpp file:

#include <stdio.h>
#include "IntNode.hpp"

IntNode::IntNode(int newData, IntNode *newLink)
{
    data = newData;
    link = newLink;
}

int IntNode::getData()
{
    return data;
}

IntNode IntNode::getLink()
{
    return *link;
}

void IntNode::setData(int newData)
{
    data = newData;
}
void IntNode::setLink(IntNode *newLink)
{
    link = newLink;
}

This is the IntNode.hpp header file:

#ifndef IntNode_h
#define IntNode_h

class IntNode
{
private:
    int data;
    IntNode *link;
public:
    IntNode(int newData, IntNode *newLink);
    int getData();
    IntNode getLink();
    void setData(int newData);
    void setLink(IntNode *newLink);
};

#endif /* IntNode_h */

This is the IntLinkedList.cpp file:

#include<iostream>
#include "IntLinkedList.hpp"
#include "IntNode.hpp"

IntLinkedList::IntLinkedList()
{
    head = nullptr;
    tail = nullptr;
    numElements = 0;
}

void IntLinkedList::add(int newElement)
{
    if(!(tail))
    {
        *head = IntNode(newElement, nullptr);
        tail = head;
    }
    else
    {
        *tail = IntNode(newElement, nullptr);
        tail->getLink() = *tail;
    }
    numElements++;
}

This is the IntLinkedList.hpp header file:

#ifndef IntLinkedList_hpp
#define IntLinkedList_hpp

#include <stdio.h>
#include "IntNode.hpp"

class IntLinkedList
{
private:
    IntNode *head;
    IntNode *tail;
    int numElements;
public:
    IntLinkedList();
    void add(int newElement);
};

#endif /* IntLinkedList_hpp */

When I run the code, it gave me this:

Thread 1: EXC_BAD_ACCESS (code=1, address=0x0)

and highlights this piece of code, which is part of the IntLinkedList::add() method:

*head = IntNode(newElement, nullptr);
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
J R
  • 1
  • Do you know what dynamic scoping is, in C++, and how to use it? Do you thoroughly understand what pointers are, in C++, how they work, and how to use them? Your constructor sets `headptr` to `nullptr`. Afterwards `add()` sets `*head = IntNode(newElement, nullptr);`. With `head` being a `nullptr`, what did you expect to happen here? – Sam Varshavchik Jul 07 '23 at 20:02
  • `*head = IntNode(newElement, nullptr);` -- You are dereferencing a null pointer `head`. – PaulMcKenzie Jul 07 '23 at 20:02
  • 3
    [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173) – Drew Dormann Jul 07 '23 at 20:03
  • `int *p = nullptr; *p = 10;` -- That essentially is what your code is doing. Do you see the issue with that simple example? – PaulMcKenzie Jul 07 '23 at 20:06
  • Java got `new`. In C++ pointers are close to what in java are variables but you have control over the stored address. – Swift - Friday Pie Jul 07 '23 at 20:14
  • Unrelated: Do what you can to hide the exitance of `IntNode` from the user. The more the user knows about the inner workings of a class, the more they will take advantage of and the more brittle their code will be should you have to change that class. – user4581301 Jul 07 '23 at 20:39
  • Since you have not gotten far enough in your list implementation to use any of the data in `IntNode`, you could -- for this question -- reduce that definition down to `class IntNode { /* placeholder */ };` and stick this definition inside `IntLinkedList.hpp` (between `#define IntLinkedList_hpp` and `class IntLinkedList` -- none of the headers should be needed after this change). Removing unnecessary code and getting people to the problematic code faster is a Good Thing for SO questions. – JaMiT Jul 07 '23 at 23:54
  • *"I want also to have the `IntNode` class and `IntLinkedList` class in separate files from the main file."* -- This is a good goal for your program. However, for your question, it would be beneficial to combine everything into a single code block, in case someone wants to copy-compile-run your code to reproduce your result. A single code block means only a single copy-paste for the person *volunteering* to help. (Work in a new project or an online compiler so that refining your minimal example does not disrupt your real project.) – JaMiT Jul 07 '23 at 23:59

2 Answers2

2

First and foremost - C++ already has a native single-linked list class: std::forward_list. And a native double-linked list class: std::list. You really should use these classes instead of creating your own.

That being said, if you must create your own classes (ie, for a school assignment), then know that your classes are not using pointers correctly:

  • in IntNode::getLink(), you are dereferencing the link pointer when it should be returned as-is. This will fail if getLink() is called on the tail node of IntLinkedList since link will be nullptr in that case.

  • in IntLinkedList::add(), you are dereferencing the head pointer when it is nullptr, which is why you get the runtime error.

  • in IntLinkedList::add(), you are also using getLink() incorrectly. You are assigning a new IntNode object to the current tail object, thus changing its data and link. And then you are calling getLink() on the current tail object, which will fail for the reason mentioned further above - its link is nullptr. You should be linking the new IntNode object to the current tail object, not replacing the current tail object.

You need to dynamically allocate the IntNode objects via the new expression (just like in Java), otherwise the objects will get destroyed as soon as they go out of scope.

You also need to implement the Rule of 3/5/0 in the IntLinkedList class to manage the IntNode objects correctly when an IntLinkedList object is copied, moved, assigned to, destroyed, etc.

Your code should look more like the following:

IntNode.hpp:

#ifndef IntNode_hpp
#define IntNode_hpp

class IntNode
{
private:
    int data;
    IntNode *link;
public:
    IntNode(int newData, IntNode *newLink = nullptr);
    int getData();
    IntNode* getLink();
    void setData(int newData);
    void setLink(IntNode *newLink);
};

#endif /* IntNode_hpp */

IntNode.cpp:

#include "IntNode.hpp"

IntNode::IntNode(int newData, IntNode *newLink)
    : data(newData), link(newLink)
{
}

int IntNode::getData()
{
    return data;
}

IntNode* IntNode::getLink()
{
    return link;
}

void IntNode::setData(int newData)
{
    data = newData;
}

void IntNode::setLink(IntNode *newLink)
{
    link = newLink;
}

IntLinkedList.hpp:

#ifndef IntLinkedList_hpp
#define IntLinkedList_hpp

#include "IntNode.hpp"

class IntLinkedList
{
private:
    IntNode *head = nullptr;
    IntNode *tail = nullptr;
    int numElements = 0;
public:
    IntLinkedList() = default;
    IntLinkedList(const IntLinkedList &src);
    IntLinkedList(IntLinkedList &&src);
    ~IntLinkedList();

    IntLinkedList& operator=(IntLinkedList src);

    void add(int newElement);
};

#endif /* IntLinkedList_hpp */

IntLinkedList.cpp:

#include "IntLinkedList.hpp"
#include <utility>

IntLinkedList::IntLinkedList(const IntLinkedList &src)
    : head(nullptr), tail(nullptr), numElements(0)
{
    IntNode *node = src.head;
    while (node)
    {
        add(node->getData());
        node = node->getLink();
    }
}

IntLinkedList::IntLinkedList(IntLinkedList &&src)
    : head(src.head), tail(src.tail), numElements(src.numElements)
{
    src.head = nullptr;
    src.tail = nullptr;
    src.numElements = 0;
}

IntLinkedList::~IntLinkedList()
{
    IntNode *node = head;
    while (node)
    {
        IntNode *link = node->getLink();
        delete node;
        node = link;
    }
}

IntLinkedList& IntLinkedList::operator=(IntLinkedList src)
{
    std::swap(head, src.head);
    std::swap(tail, src.tail);
    std::swap(numElements, src.numElements);
    return *this;
}

void IntLinkedList::add(int newElement)
{
    IntNode *node = new IntNode(newElement);
    if (!head)
    {
        head = node;
    }
    else
    {
        tail->setLink(node);
    }
    tail = node;
    ++numElements;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Side note: Sooner or later you'll be tasked with removing an item from the list. [See the Community Addition in this answer](https://stackoverflow.com/a/22122095/4581301) for a simple and efficient way to do it. – user4581301 Jul 07 '23 at 20:42
-1

*head = IntNode(newElement, nullptr);

This says to create a (temporary) IntNode object, then copy that object to the object to which head points. However, the only value that has been assigned to head is nullptr, so there is no object to which head points. Copying to a non-existent object is a valid reason for crashing.

What you want is to create a new IntNode object (not a temporary), then have head point to that object. Change head not *head.

head = new IntNode(newElement, nullptr);

Don't forget to delete the object later. This means having a destructor, which leads to The Rule of Three.

Note: This is not the only bug in your code, just the one currently showing symptoms. I'll leave the rest for other questions or answers.

JaMiT
  • 14,422
  • 4
  • 15
  • 31