debugcppMinor
An attempt to make a linked list iterator "safe"
Viewed 0 times
iteratormakeattemptsafelistlinked
Problem
I was practicing my C++ routine and decided to code up this simple linked list data structure. Furthermore, I made an attempt to make the list iterator "safe" in the sense that it throws an exception instead of tampering a memory region it should not..
safelist.h
```
#ifndef SAFE_LIST_H
#define SAFE_LIST_H
#include
namespace net {
namespace coderodde {
namespace util {
template
class Safe_list {
public:
Safe_list()
:
head{nullptr},
tail{nullptr}
{}
// Disable all the constructors + assignments:
Safe_list(const Safe_list&) = delete;
Safe_list(Safe_list&&) = delete;
Safe_list& operator=(const Safe_list&) = delete;
Safe_list& operator=(Safe_list&&) = delete;
void push_back(const T& datum)
{
Safe_list_node* new_node = new Safe_list_node(datum);
if (tail == nullptr)
{
head = new_node;
}
else
{
tail->next = new_node;
}
tail = new_node;
}
void pop_front()
{
check_list_not_empty();
Safe_list_node* removed = head;
head = head->next;
if (head == nullptr)
{
tail = nullptr;
}
delete removed;
}
const T& front() const
{
check_list_not_empty();
return head->datum;
}
T& front()
{
check_list_not_empty();
return head->datum;
}
private:
struct Safe_list_node {
T datum;
Safe_list_node* next;
Safe_list_node(const T& datum)
:
datum{datum},
next{nullptr}
{}
};
Safe_list_node* head;
Safe_list_node* tail;
void check_list_not_empty()
{
if (head == nullptr)
{
throw std::runtime_error{"Operating on an empty list."};
}
}
public:
class Safe_list_iterator {
Safe_list_node* current_node;
public:
Safe_list_iterator(Safe_lis
safelist.h
```
#ifndef SAFE_LIST_H
#define SAFE_LIST_H
#include
namespace net {
namespace coderodde {
namespace util {
template
class Safe_list {
public:
Safe_list()
:
head{nullptr},
tail{nullptr}
{}
// Disable all the constructors + assignments:
Safe_list(const Safe_list&) = delete;
Safe_list(Safe_list&&) = delete;
Safe_list& operator=(const Safe_list&) = delete;
Safe_list& operator=(Safe_list&&) = delete;
void push_back(const T& datum)
{
Safe_list_node* new_node = new Safe_list_node(datum);
if (tail == nullptr)
{
head = new_node;
}
else
{
tail->next = new_node;
}
tail = new_node;
}
void pop_front()
{
check_list_not_empty();
Safe_list_node* removed = head;
head = head->next;
if (head == nullptr)
{
tail = nullptr;
}
delete removed;
}
const T& front() const
{
check_list_not_empty();
return head->datum;
}
T& front()
{
check_list_not_empty();
return head->datum;
}
private:
struct Safe_list_node {
T datum;
Safe_list_node* next;
Safe_list_node(const T& datum)
:
datum{datum},
next{nullptr}
{}
};
Safe_list_node* head;
Safe_list_node* tail;
void check_list_not_empty()
{
if (head == nullptr)
{
throw std::runtime_error{"Operating on an empty list."};
}
}
public:
class Safe_list_iterator {
Safe_list_node* current_node;
public:
Safe_list_iterator(Safe_lis
Solution
I mentioned that your code was leaking but it still is. There is no destructor to clean up the nodes.
To cleanly get proper cleanup you can use
Consider allowing (only) the move constructor and assignment. As is (with the unique_ptr) you can just remove the disabling declarations and it'll work, though I suggest implementing them yourself to null out the tail pointer in the moved-from object.
To cleanly get proper cleanup you can use
unique_ptr:void pop_front()
{
check_list_not_empty();
Safe_list_node* next = head->next.release();
head.reset(next);
if (!head)
{
tail = nullptr;
}
}
void push_back(const T& datum)
{
std::unique_ptr new_node = std::make_unique(datum);
tail = new_node.get();
if (!tail)
{
head = std::move(new_node);
}
else
{
tail->next = std::move(new_node);
}
}
struct Safe_list_node {
T datum;
std::unique_ptr next;
Safe_list_node(const T& datum)
:
datum{datum},
next{}
{}
};
std::unique_ptr head;
Safe_list_node* tail;Consider allowing (only) the move constructor and assignment. As is (with the unique_ptr) you can just remove the disabling declarations and it'll work, though I suggest implementing them yourself to null out the tail pointer in the moved-from object.
Code Snippets
void pop_front()
{
check_list_not_empty();
Safe_list_node* next = head->next.release();
head.reset(next);
if (!head)
{
tail = nullptr;
}
}
void push_back(const T& datum)
{
std::unique_ptr<Safe_list_node> new_node = std::make_unique<Safe_list_node>(datum);
tail = new_node.get();
if (!tail)
{
head = std::move(new_node);
}
else
{
tail->next = std::move(new_node);
}
}
struct Safe_list_node {
T datum;
std::unique_ptr<Safe_list_node> next;
Safe_list_node(const T& datum)
:
datum{datum},
next{}
{}
};
std::unique_ptr<Safe_list_node> head;
Safe_list_node* tail;Context
StackExchange Code Review Q#146586, answer score: 5
Revisions (0)
No revisions yet.