Return to Answer

You could use nullptr instead of NULL: What exactly is nullptr?

You could use size_t instead of int for indizes. Currently your code will crash if you pass a negative value as pos (see deleteN, getN)

You could keep track of the size of the list, so you don't need to calculate it every time you access it via pos. Also this might make it nice getter.

Why don't you pass data in the constructor of your node. Also you could make the constructor of node take all three arguments data, prev and next and pass them accordingly when creating a node.

There is no need for default initialization of data anywhere in your code.

Your assignment operator List& operator=(List listObj) should take a const reference: List& operator=(const List& listObj) like your copy constructor.

Your assignment operator should check whether the passed object is the list itself and do nothing if so.

Your assignment operator does currently not check if there is already any data in the list. You need to clean up first (delete all nodes) before initializing the object with the new data from the other list. Otherwise you have a memory leak.

Your assigment operator doesn't return anything. Add return *this;.

Why do deleteN and getN have a fromHead yet addAfterN has not?

Instead of, or additional to, printList you might want to implement an ostream operator.

Try to avoid duplicate code.

else { if(fromHead) { node* tmp = m_head; for(int i=0; i<pos; i++){ tmp = tmp->n_next; } if(tmp->n_prev)tmp->n_prev->n_next = tmp->n_next; if(tmp->n_next)tmp->n_next->n_prev = tmp->n_prev; delete tmp; } else { node* tmp = m_tail; for(int i=0; i<pos; i++){ tmp = tmp->n_prev; } if(tmp->n_prev)tmp->n_prev->n_next = tmp->n_next; if(tmp->n_next)tmp->n_next->n_prev = tmp->n_prev; delete tmp; } }

You could use nullptr instead of NULL: What exactly is nullptr?

You could use size_t instead of int for indizes. Currently your code will crash if you pass a negative value as pos (see deleteN, getN)

You could keep track of the size of the list, so you don't need to calculate it every time you access it via pos. Also this might make it nice getter.

Why don't you pass data in the constructor of your node. Also you could make the constructor of node take all three arguments data, prev and next and pass them accordingly when creating a node.

There is no need for default initialization of data anywhere in your code.

Your assignment operator List& operator=(List listObj) should take a const reference: List& operator=(const List& listObj) like your copy constructor.

Your assignment operator should check whether the passed object is the list itself and do nothing if so.

Your assignment operator does currently not check if there is already any data in the list. You need to clean up first (delete all nodes) before initializing the object with the new data from the other list. Otherwise you have a memory leak.

Your assigment operator doesn't return anything. Add return *this;.

Why do deleteN and getN have a fromHead yet addAfterN has not?

Instead of, or additional to, printList you might want to implement an ostream operator.

Try to avoid duplicate code.

else { if(fromHead) { node* tmp = m_head; for(int i=0; i<pos; i++){ tmp = tmp->n_next; } if(tmp->n_prev)tmp->n_prev->n_next = tmp->n_next; if(tmp->n_next)tmp->n_next->n_prev = tmp->n_prev; delete tmp; } else { node* tmp = m_tail; for(int i=0; i<pos; i++){ tmp = tmp->n_prev; } if(tmp->n_prev)tmp->n_prev->n_next = tmp->n_next; if(tmp->n_next)tmp->n_next->n_prev = tmp->n_prev; delete tmp; } }