patterncppMinor
Singly linked-list with smart pointers
Viewed 0 times
withsinglysmartpointerslistlinked
Problem
A few things:
Any comments / suggestions appreciated.
Here's my singly linked list.
I've also posted this on Reddit.
List.cpp
List.h
```
#ifndef LIST_H
#define LIST_H
#include
#include
class List {
struct Link {
- I tried to do it with smart pointers because I wanted to learn about them. I'm not sure I made the right choice of type, however (and started to regret it half-way through).
- This is for school, so some operations were required. I can't change main.cpp or the general layout.
insert()works. However, I'm suspicious it's not working properly. I haven't implemented delete functionality yet.
Any comments / suggestions appreciated.
Here's my singly linked list.
I've also posted this on Reddit.
List.cpp
#include "List.h"
void List::reverse_print(std::shared_ptr curr) const {
if (curr) {
reverse_print(curr->next);
std::cout datum new_link(new Link(val));
new_link->next = root;
root = std::move(new_link);
}
void List::push_back(int val) {
std::unique_ptr new_link(new Link(val));
if (!root)
root = std::move(new_link);
else {
std::shared_ptr curr(root);
while (curr->next) {
curr = curr->next;
}
curr->next = std::move(new_link);
}
}
void List::insert(int val, int loc){
if (loc > length( ) + 1 || loc ptr(root);
while (node_count next;
++node_count;
}
std::shared_ptr new_link(new Link(val));
ptr->next = new_link;
}
}
}
int List::length() const {
int node_count = 0;
std::shared_ptr ptr(root);
while (ptr) {
ptr = ptr->next;
++node_count;
}
return node_count;
}
void List::print() const {
std::shared_ptr temp(root);
if (!temp) {
std::cout datum next;
}
std::cout List::get_first() const {
return root;
}
void List::destroy_list() {
root.reset();
};
bool List::empty( ) const {
return root == nullptr;
}
/*
void delete_link( ){ }
void delete_first( ) { }
void delete_last( ) { }
*/List.h
```
#ifndef LIST_H
#define LIST_H
#include
#include
class List {
struct Link {
Solution
Some comments to the things you have implemented so far:
reverse_print: Printing the list in reverse order recursively is an academically interesting solution but if your list is sufficiently long then your stack will explode. Use astd::stackto temporarily store the elements and then print that.
push_back: Currently this is O(n) because you iterate the entire list to find the last element. If you store atailpointer which points to the last element in the list in addition torootthen you can avoid that and this operation becomes O(1).
length(): Keep a length property in the class which you increment/decrement when a node got added/removed. Again reduces an O(n) operation to O(1).
insert():
- While it's commendable for the (application) user experience to have 1 based indices, as a programer who might use this it will be confusing. Indices into containers like arrays and vectors are 0 based in C++ land. Deviating from the standard is bad because it is unexpected.
- Insert is broken. You will lose all items after the insert position (
new_link->nextpoints to nothing andptr->nextnow points tonew_link-> all elements starting from the oldptr->nextare now lost)
Context
StackExchange Code Review Q#33136, answer score: 5
Revisions (0)
No revisions yet.