Skip to main content
7 of 7
added 14 characters in body
Incomputable
  • 9.7k
  • 3
  • 34
  • 73

Rule of five:

Since container has ownership of elements, it is responsible for preventing corruption of the data inside. It has to take care of those resources when copied/moved. Since explicit destructor is declared, default generation of copy and move operations is disabled. It would be great to have rule of five implemented, especially for the containers.

The container is not so generic

This is the biggest easy of use issue. It places enormous restriction on the T.

value_type data = 0;

This line tells the compiler that T should be assignable (copy or move) or in C++17 copy constructible from zero. There are way too many types that are not copy constructible or assignable from/to zero.

Too many useless copy constructor calls:

This is the biggest potential performance issue.

A few examples with some explanations:

    value_type data = 0;

    if(this->tail && this->head){
        data = this->head->get_data();
        item = this->head;
        this->head = this->head->get_next();
    }

    //excerpt from node
    value_type get_data() const{
        return this->data;
    }

So, first copy construction happens on the line mentioned in the previous paragraph. The second time is when get_data() is invoked. It copy constructs the internal data, then copy assignment happens again when data is assigned to it. This culminates the non-generic nature of the code. Then, the last copy construction happens when data is returned. 4 copy construction/assignment calls just to get the data off of the queue. Isn't it way too much?

forward_list& push_back(value_type value){
    node_pointer item = new node(value);

    if(this->tail) this->tail->set_next(item);
    this->tail = item;
    if(!this->head) this->head = this->tail;

    return *this;
}

//excerpt from node
void set_data(value_type value){
    this->data = value;
}

Roughly the same as above, just in another direction.

Actually compiler can elide some (or all unnecessary ones) copy constructor calls, but it doesn't mean that one shouldn't care about it.

Generic code:

Some people try to write special case first, then make it generic. Although this works for simple cases, it is not going to work when number of template parameters/parameter packs start increasing. The problem is that it creates that many-to-many dependency, which makes "sticking different parts together" very hard/error prone at the end. For cases like this it should work though.

I found thinking in terms of concepts (or constraints) a great way to accomplish most of the metaprogramming tasks. Knowing C++ concepts should facilitate in that.

Exception specification:

Due to copy constructive nature of the container it probably can't guarantee anything, so it might be great to fix that.

Incomputable
  • 9.7k
  • 3
  • 34
  • 73