LinkedList copy constructor implementation details
c++, copy-constructor, implementation
Solution
I assume append will properly handle the initial head/tail details, yes? If so, what you have now is great and simple: Go through the other list, and take its item and add a copy to my list. Perfect.
Well, almost. Use an initializer list to initialize member variables:
template<typename T>
LinkedList<T>::LinkedList(const LinkedList& l) :
m_head(0), m_tail(0), m_size(0)
{
// ...
}
Also, maybe a matter of style, this woks instead of a while loop:
// construct list from given list
for (Node *n = l.m_head; n != 0; n = n->next)
append(m->data);
In fact, I'd recommend this instead. When you have iterators, you'd do something like:
for (const_iterator iter = l.begin(); iter != l.end(); ++iter)
append(*iter);
It just follows the style of a for-loop better. (Initialize something, check something, do something). Though for iterators, it'll probably be different. (More later)
Or should I access the data through the corresponding accessor? (I know that I don't have the accessor(s) defined).
Also, I intend to create a custom iterator so that it can be possible to iterate over the LinkedList. Should I use in the copy constructor to access the data on each node?
Those iterators are your accessors. You don't want to expose your internal head-tail pointers, that a recipe for disaster. The purpose of the class is to not expose the details. That said, iterators are the abstract wrapper around those details.
Once you have your iterators, then you could use them to iterate through the list instead of pointer arithmetic. This ties in to this recently asked question. In general, you should use your abstractions to deal with your data. So yes once you have your iterators in place, you should use those to iterate across the data.
Most classes that provide iterators also provide a way to insert data given a beginning and ending iterator. This is usually called `insert`, like this: `insert(iterBegin, iterEnd)`. This loops through the iterators, appending it's data to the list.
If you had such functionality, your copy-constructor would simply be:
insert(l.begin(), l.end()); // insert the other list's entire range
Where `insert` is implemented like the for-loop we had above.
Another question (completely off-topic, I know), when and/or why should we declare a pointer to a LinkedList
LinkedList *l = new LinkedList(); instead of LinkedList l;
The first is dynamic allocation, the second is automatic (stack) allocation. You should prefer stack allocation. It's almost always faster, and safer too (since you don't need to delete anything). In fact, a concept called RAII relies on automatic storage, so destructors are guaranteed to run.
Only use dynamic allocation when you have to.
Problem
I'm starting to learn C++ and as an exercise decide to implement a simple `LinkedList` class (Below there is part of the code). I have a question regarding the way the copy constructor should be implemented and the best way the data on the original `LinkedList` should be accessed. ``` template <typename T> class LinkedList { struct Node { T data; Node *next; Node(T t, Node *n) : data(t), next(n) {}; }; public: LinkedList(); LinkedList(const LinkedList&); ~LinkedList(); //member functions int size() const; //done bool empty() const; //done void append(const T&); //done void prepend(const T&); //done void insert(const T&, int i); bool contains(const T&) const; //done bool removeOne(const T&); //done int removeAll(const T&); //done void clear(); //done T& last(); //done const T& last() const; //done T& first(); //done const T& first() const; //done void removeFirst(); //done T takeFirst(); //done void removeLast(); T takeLast(); //delete when finished void print(); //end delete //operators bool operator ==(const LinkedList<T> &other) const; //done bool operator !=(const LinkedList<T> &other) const; //done LinkedList<T>& operator =(const LinkedList<T> &other); //done private: Node* m_head; Node* m_tail; int m_size; }; template<typename T> LinkedList<T>::LinkedList() : m_head(0), m_tail(0), m_size(0) { } ... ``` Should my copy constructor access the data on each node of the original `LinkedList` directly? ``` template<typename T> LinkedList<T>::LinkedList(const LinkedList& l) { m_head = 0; m_tail = 0; m_size = 0; Node *n = l.m_head; // construct list from given list while(n) { append(n->data); n = n->next; } } ``` Or should I access the data through the corresponding accessor? (I know that I don't have the accessor(s) defined). Also, I intend to create a custom iterator so that it can be possible to iterate over the `LinkedList`. Should I use in the copy constructor to access the data on each node? Another question (completely off-topic, I know), when and/or why should we declare a pointer to a `LinkedList` ``` LinkedList<int> *l = new LinkedList<int>(); ``` instead of ``` LinkedList<int> l; ```