I'm trying to implement a singly linked list on my own in C++ from scratch, having got so far with my first method append():
#include <iostream>
using namespace std;
struct ListNode{
int key_value;
ListNode *next;
};
class List{
public:
List();
void append(int key);
private:
ListNode *head;
ListNode *tail;
};
List::List(){
head = NULL;
tail = head;
}
void List::append(int key){
ListNode *newnode;
newnode->key_value = key;
newnode->next = NULL;
if(head == NULL){
head = newnode;
tail = head;
}
else{
tail->next = newnode;
tail = newnode;
}
return;
}
int main(){
try{
List l1;
l1.append(1);
//l1.append(2);
//l1.append(3);
//l1.append(5);
}
catch (exception const& ex) {
cerr << "Exception: " << ex.what() <<endl;
return -1;
}
}
It compiles without any warnings or errors, but during execution I'm only getting the message Bus error: 10
. Seems there is something wrong with the way I'm initializing and using the ListNode variables and pointers, any insights would be appreciated.
In this line:
ListNode *newnode;
you create uninitialized variable of pointer to ListNode
and then you dereference it. Any access to unintialized variable leads to UB, but with pointers you most probably will get bus error immediatly. So allocate memory:
ListNode *newnode = new ListNode;
Note: instead of initializing data members:
newnode->key_value = key;
newnode->next = NULL;
you should provide proper constructor for ListNode
:
struct ListNode {
int key_value;
ListNode *next;
ListNode( int v ) :
key_value( v ),
next( nullptr )
{
}
};
then just create it:
ListNode *newnode = new ListNode( key );
and next 2 lines can be omitted. This will make your code cleaner and prevent from creating a ListNode
instance with uninitialized data.
Note N2: as your class List
has a raw pointer and ownership of the data you should follow rule of three and create or disable copy ctor, assignment operator and dtor.