2
\$\begingroup\$

I am just learning about C++ templates and generic types, I decided it would be nice to try to create a generic container class as a challenge and test my knowledge in the process. Here is what I have come up with.


#include <iostream>
#include <cstdlib>
#include <cassert>

template <typename __T>

class List{
private:
    struct node{
        __T data;
        node* next;
        node* prev;
    };
    typedef long long unsigned int c_size;
    node* head;
    node* tail;
    node* current;
    c_size m_S;

public:
    List()
        :head{NULL},tail{NULL},current{NULL},m_S{0}
    {}

    List(std::initializer_list<__T> __l)
        :head{NULL},tail{NULL},current{NULL},m_S{0}
    {
        for(auto const& elem:__l){
            insert_back(elem);
        }
    }
    void insert_front(__T _d){
        ++m_S;
        node* new_node{new node};
        new_node->data = _d;
        new_node->next = head;
        new_node->prev = NULL;

        if (head != NULL){
            head->prev = new_node;
        }
        else
        {
            tail = new_node;
        }
        head = new_node;

    }

    void print(){
        if (m_S == 0) return;
        node *current = head;
        while (current->next != NULL){
            std::cout << current->data << ", ";
            current = current->next;
        }
        std::cout << current->data << '\n';
    }

    void insert_back(__T _d){
        ++m_S;
        node* new_node = new node;
        new_node->data = _d;
        new_node->next = NULL;
        node* last = head;

        if (head == NULL) {
            new_node->prev = NULL;
            head = new_node;
            tail = new_node;
        }
        else{
            tail->next = new_node;
            new_node->prev = tail;
            tail = new_node;
        }
    }

    constexpr __T first(){
        assert(head != NULL && "Calling .front() on an empty list");
        return head->data;
    }
    constexpr __T last(){
        assert(tail != NULL && "Calling. last() on an empty list");
        return tail->data;
    }


    constexpr c_size size(){return m_S;}


    void remove_all(){
        current = head;
        while(current->next != NULL){
            current = current->next;
            delete current;
        }
        head = NULL;
        tail = NULL;
        current = NULL;
    }

};


Thanks in advance !

\$\endgroup\$
8
  • 7
    \$\begingroup\$ __T double underscores as prefix for a symbol are reserved for compiler internal implementations. \$\endgroup\$ Oct 3, 2020 at 19:26
  • 1
    \$\begingroup\$ Hey, if you want some more practice you might want to align the interface of your List class with that of the standard std::list. \$\endgroup\$
    – slepic
    Oct 4, 2020 at 9:02
  • \$\begingroup\$ @slepic Hello, can you explain what you mean? \$\endgroup\$
    – user228914
    Oct 4, 2020 at 9:05
  • \$\begingroup\$ I mean to use standard method names (along with their signatures), like push_back, push_front, back, front, pop_back, pop_front, clear, etc. You might also want to implement iterators and generally comply with c++'s container concept... You will get many general algorithms (like sort) for free just by following the interface of all standard containers. \$\endgroup\$
    – slepic
    Oct 4, 2020 at 10:35
  • \$\begingroup\$ And btw print is usually not part of generic container because it heavily depends on the generic value type T and the application itself. \$\endgroup\$
    – slepic
    Oct 4, 2020 at 10:41

1 Answer 1

9
\$\begingroup\$

Identifiers that start with a double underscore, or a single underscore followed by a capital letter, are reserved for use by the implementation. Your use of __T could conflict with this use.

Spacing is important. The blank line between template <typename __T> and the following class List line makes it harder to recognize the class as a template class.

What is the purpose of the current member? It is unnecessary.

c_size should probably be std::size_t, not unsigned long long. Because it is a private typedef, users of your class cannot access the type and can only declare a variable to hold the returned value using auto.

You don't declare the copy constructor, copy assignment operator, move constructor, nor move assignment operators. This will cause problems with assignments of one List object to another.

insert_head and insert_back are very similar functions, but have been written somewhat differently. The implementation for insert_back can be made shorter by making it similar to insert_head. In particular, the assignment to new_node->prev can be done before the if (using new_node->prev = tail;, because when head is NULL, tail should also be), while the common tail = new_node; can be done after.

first, last, and size should also have the const modifier (e.g., constexpr c_size size() const.

remove_all will crash because you access current->next after delete current, nor do you delete the first (head) element.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy