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

STL-compliant list class

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

Problem

This is my first attempt to create an STL-compliant container. I'd like to know whether it meets STL concepts & requirements, so feel free to critique.

list.h

```
#include
#include
#include
#include
#include
#include
#include
#include
#include

template >
class clist
{
private:
struct node
{
public:
node() : m_pT(nullptr), m_pNext(nullptr), m_pPrev(nullptr) {}

void loop() {this->m_pNext = this; this->m_pPrev = this;}

static void link(node lhs, node rhs) {lhs->m_pNext = rhs; rhs->m_pPrev = lhs;}
void unlink()
{
if (this->m_pPrev != nullptr) {this->m_pPrev->m_pNext = this->m_pNext;}
if (this->m_pNext != nullptr) {this->m_pNext->m_pPrev = this->m_pPrev;}
}

void reverse() {std::swap(this->m_pPrev, this->m_pNext);}

private:
T *m_pT;
node *m_pNext;
node *m_pPrev;

friend class clist;
};

template
class iterator_cnc : public std::iterator::type>
{
using node_ptr_type = typename std::conditional::type;
using const_reference = const T&;

public:
iterator_cnc() : m_pT(nullptr) {}
iterator_cnc(node_ptr_type p) : m_pT(p) {}
iterator_cnc(const iterator_cnc &it) : m_pT(it.m_pT) {}

bool operator == (const iterator_cnc &rhs) {return (this->m_pT == rhs.m_pT);}
bool operator != (const iterator_cnc &rhs) {return !((*this) == rhs);}
reference operator () {return (this->m_pT->m_pT);}
const_reference operator () const {return (this->m_pT->m_pT);}
iterator_cnc& operator ++ () {this->m_pT = this->m_pT->m_pNext; return (*this);}
iterator_cnc& operator -- () {this->m_pT = this->m_p

Solution

Why return a value:

value_type operator * () const {return *(this->m_pT->m_pT);}


I would return a const reference

const_reference operator * () const {return *(this->m_pT->m_pT);}


Iterator does not seem to support operator-> which is a requirement of the iterator concept.

Personally I don't like the leading '_' you put on your private members. If this is short hand to indicate they are private it just means you are not using descriptive enough names to describe your methods. The whole point in writing complex code is that the functions should be self descriptive.

allocator_type _alcv() {return allocator_type(this->m_alloc_node);}

 // why note use a descriptive name.
 // It will make the code easier to read and maintain.

allocator_type allocate_node() {return allocator_type(this->m_alloc_node);}

// Or I prefer camel case

allocator_type allocateNode() {return allocator_type(this->m_alloc_node);}


Also (another personal preference) I like type names to begin with an uppercase letter. That way you can easily spot types in comparison to objects (ie runtime Vs compile time things). Its a shame the standard libraries does not use this convention but we are now stuck for backwards compatibility (But user defined types tend to be defined with an upper case letter (but thats not universal and a style thing so follow your local guidelines)).

Personally I think size() should be O(1) not O(n). I know this means things like splice go from O(1) to O(n) so there is a trade off and you probably chose the correct one (as thats how the standard version deals with it). But you could cache the size value so you calculate it on first it can be just returned on subsequent calls; then on splice() you can mark it as dirty and recalculate the next time size() is called.

size_type size() const {return std::distance(this->cbegin(), this->cend());}


Seems like empty() should be a bit more effecient.

bool empty() const {return (this->cbegin() == this->cend());}


Creating two objects and checking for equivalence seems to be overkill. There seems like there could be a much simpler test for empty.

bool empty() const {return m_head != NULL;} /* Does that work or is there a sentanal */


You seem to do a lot of functions this way when there seem to be easier alternatives:

reference front() {return *(this->begin());}
    /*constexpr*/ const_reference front() const {return *(this->cbegin());}
    reference back() {return *(--(this->end()));}
    /*constexpr*/ const_reference back() const {return *(--(this->cend()));}

Code Snippets

value_type operator * () const {return *(this->m_pT->m_pT);}
const_reference operator * () const {return *(this->m_pT->m_pT);}
allocator_type _alcv() {return allocator_type(this->m_alloc_node);}

 // why note use a descriptive name.
 // It will make the code easier to read and maintain.

allocator_type allocate_node() {return allocator_type(this->m_alloc_node);}

// Or I prefer camel case

allocator_type allocateNode() {return allocator_type(this->m_alloc_node);}
size_type size() const {return std::distance(this->cbegin(), this->cend());}
bool empty() const {return (this->cbegin() == this->cend());}

Context

StackExchange Code Review Q#43639, answer score: 4

Revisions (0)

No revisions yet.