HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

STL List Implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
listimplementationstl

Problem

I've implemented a simple C++ STL like list. It's pretty simple, all the constructors and methods haven't been implemented here, but majors are.

```
#ifndef list_H
#define list_H

#include
#include

template
class list {
public:
list & operator = (const list &);
~list();
/ Modifiers /
void push_back(T&& data);
void push_back(T const& data);
void push_front(T&& data);
void push_front(T const& data);
void pop_back();
void pop_front();
void swap(list &x);
void clear();

/ Iterators /
typedef T* iterator;
typedef T* const const_iterator;

const_iterator begin() const; // cbegin
iterator begin();

const_iterator end() const; //cend()
iterator end();

const_iterator rbegin() const;
iterator rbegin();

const_iterator rend() const;
iterator rend();

/ Capacity /
size_t size() const;
bool empty() const;

/ Element Access /
T& front();
T const& front() const;

T& back();
T const& back() const;

T& at(T const indx);
T const& at(T const indx) const;

T& operator[] (T const indx);
T const& operator[] (T const indx) const;

private:
struct node {
int data;
node next, prev;
node(T const& data, node next, node prev)
: data(data)
, next(next)
, prev(prev) {
}
node(T&& data, node next, node prev)
: data(std::move(data))
, next(next)
, prev(prev) {
}
};
int elements = 0;
node *head = nullptr;
node *tail = nullptr;
};

template
list & list::operator = (const list & that) {
node* tmp = head;
while(head) {
tmp = head;
head = head->next;
delete tmp;
}
elements = that.elements;
head = that.head;
tail = that.tail;
}

template
list ::~list() {
node* tmp;
while(head) {
tmp = head;
head = head->next;
delete tmp;
}
}

Solution

Why does your node hold an int as data when there is a template argument T?

It is more of a linked list, therefore a std::list, not a std::deque.

Your assignment operator creates a so called flat copy, so that the two objects afterwards refer to the same piece of memory. That will at the latest crash if they're getting destructed.

Some of your functions contain the same code twice. You may solve that with private methods.

You may specify the exceptions text to find the affected method easier.

You return nullptr inside the operator[] case of the index being outside the valid range. The same goes for at. Why don't you check the index beforehand with the member elements, throw an exception in case of it being to big and then just proceed? You can skip the checking part in the operator[] since it's usually known as the unchecked version.

If you name your methods rbegin and so on, you also have to return a std::reverse_iterator.

Inside the class list, you don't have to write the template arguments out in full. list means inside the class' body the same.

Your iterators are wrong in general since the memory isn't consecutive. You have to implement your own.

A possible implementation of deep copy (though I didn't test it):

list(list const& Rhs)
    : elements(Rhs.size())
{
    if(!Rhs.empty())
    {
        node* RhsIt = Rhs.head;
        node* It = new node(RhsIt->data, nullptr, nullptr);
        head = It;
        while((RhsIt = RhsIt->next) != Rhs.tail)
        {
            try
            {
                node* Next = new node(RhsIt->data, It, nullptr);
                It = It->next = Next;
            }
            catch(std::bad_alloc& Exception)
            {
                for(node* Last; head != nullptr; delete Last)
                {
                    Last = head;
                    head = head->next;
                }
                throw;
            }

        }
        tail = It;
    }
}

list& operator= (list Rhs) // Call by value
{
    swap(Rhs); // Copy and swap idiom to guarantee exception safety
}

Code Snippets

list(list const& Rhs)
    : elements(Rhs.size())
{
    if(!Rhs.empty())
    {
        node* RhsIt = Rhs.head;
        node* It = new node(RhsIt->data, nullptr, nullptr);
        head = It;
        while((RhsIt = RhsIt->next) != Rhs.tail)
        {
            try
            {
                node* Next = new node(RhsIt->data, It, nullptr);
                It = It->next = Next;
            }
            catch(std::bad_alloc& Exception)
            {
                for(node* Last; head != nullptr; delete Last)
                {
                    Last = head;
                    head = head->next;
                }
                throw;
            }

        }
        tail = It;
    }
}

list& operator= (list Rhs) // Call by value
{
    swap(Rhs); // Copy and swap idiom to guarantee exception safety
}

Context

StackExchange Code Review Q#60994, answer score: 4

Revisions (0)

No revisions yet.