Problems with Copy and assignment constructor

Question

I have the following code, used as a part of a Linked List:

// copy constructor:
LinkedList<T>(const LinkedList<T> &list) 
{
    // make a deep copy
    for (LinkedList<T>::Iterator i = list.begin(); i != list.end(); i++)
    {
        add(*i);
    }
}


// assignment constructor
LinkedList<T>& operator= (const LinkedList<T> &list) 
{
    // make a deep copy
    for (LinkedList<T>::Iterator i = list.begin(); i != list.end(); i++)
    {
        add(*i);
    }
}

But when I compile I get the following errors (this is when I use it as an assignment constructor):

1>------ Build started: Project: AnotherLinkedList, Configuration: Debug Win32 ------
1>main.cpp
1>c:\users\ra\source\repos\sandbox\container\anotherlinkedlist\linkedlist.h(57): error C2662: 'LinkedList<int>::Iterator LinkedList<int>::begin(void)': cannot convert 'this' pointer from 'const LinkedList<int>' to 'LinkedList<int> &'
1>c:\users\ra\source\repos\sandbox\container\anotherlinkedlist\linkedlist.h(57): note: Conversion loses qualifiers
1>c:\users\ra\source\repos\sandbox\container\anotherlinkedlist\linkedlist.h(55): note: while compiling class template member function 'LinkedList<int> &LinkedList<int>::operator =(const LinkedList<int> &)'
1>c:\users\ra\source\repos\sandbox\container\anotherlinkedlist\main.cpp(20): note: see reference to function template instantiation 'LinkedList<int> &LinkedList<int>::operator =(const LinkedList<int> &)' being compiled
1>c:\users\ra\source\repos\sandbox\container\anotherlinkedlist\main.cpp(14): note: see reference to class template instantiation 'LinkedList<int>' being compiled
1>c:\users\ra\source\repos\sandbox\container\anotherlinkedlist\linkedlist.h(57): error C2662: 'LinkedList<int>::Iterator LinkedList<int>::end(void)': cannot convert 'this' pointer from 'const LinkedList<int>' to 'LinkedList<int> &'
1>c:\users\ra\source\repos\sandbox\container\anotherlinkedlist\linkedlist.h(57): note: Conversion loses qualifiers
1>Done building project "AnotherLinkedList.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

The iterator code for begin and end looks like this:

// get root
    Iterator begin()
    {
        return Iterator(sp_Head);
    }

    // get end
    Iterator end()
    {
        return Iterator(nullptr);
    }

What do I do wrong?

FYI -- Your assignment operator does not remove the old list and fails to return a value. — PaulMcKenzie, Nov 19 '19 at 20:15
True, but that's not the reason for the compilation error. The compilation error is because begin/end are not const class methods, and they must be. Free karma to whoever wants it... — Sam Varshavchik, Nov 19 '19 at 20:17
@PaulMcKenzie: i see your point. of course i need to delete all the existing items in the list, add the new one, and return *this (correct?) — tannic, Nov 19 '19 at 20:28
You could do that, but using [copy / swap](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) is better. — PaulMcKenzie, Nov 19 '19 at 20:31
`Iterator(nullptr)` <-- this looks like a problem waiting to happen. Will the iterator always point to a nullptr at the end? — JHBonarius, Nov 19 '19 at 20:34
@JHBonarius: i will point to the node after the tail node in the list, which allways should be nullptr. since i keep track of the tail node, then i could use sp_Tail->p_Next — tannic, Nov 19 '19 at 21:33
Now my copy constructor works perfectly. I had the problem, that i kept running around with the original list, until i figured out, that the head and tail pointers was declared as static *DOH*. So now i just need my assignment operator to work — tannic, Nov 24 '19 at 16:51

score 3 · Accepted Answer · answered Nov 19 '19 at 20:17

3

Based on the error message, it would seem that your LinkedList does not have variants of begin() and end() that can be called on a const object. The parameters to your copy constructor and assignment operator are const, however. You will have to add const versions of begin() and end().

Presumably, you're looking for something like this:

    ConstIterator begin() const { Iterator(sp_Head); }
    Iterator begin() { Iterator(sp_Head); }

    ConstIterator end() const { ConstIterator(nullptr); }
    Iterator end() { Iterator(nullptr); }

Where ConstIterator is a version of your iterator type that iterates over const elements…

answered Nov 19 '19 at 20:17

Michael Kenzel

15,508
2
30
39

Should i add methods that returns const ? I have tried to add change them to const Iterator, but with the same result – tannic Nov 19 '19 at 20:23
@tannic read the error "Conversion loses qualifiers" the member function has to be const qualified. – JHBonarius Nov 19 '19 at 20:37
@tannic If you have a const list, then that usually means that the content of the list is to be logically const, i.e., someone with a `const LinkedList&` should not be able to modify the content of the list. If you return a const iterator, that just means the iterator itself cannot be modified. What you usually want is for begin and end on a const list to return iterators that iterate over `const T` rather than `T`… – Michael Kenzel Nov 19 '19 at 20:37
@MichaelKenzel so need to make a ConstIterator class, that should (allmost) be a copy of my current Iterator ? – tannic Nov 19 '19 at 21:03
1

@tannic I tend to make the iterator a class template, `template class iterator_impl { using value_type = Type; ... };` and after the iterator class template definition just do `using iterator = iterator_impl; using const_iterator = iterator_impl;` to get both. – Ted Lyngmo Nov 19 '19 at 21:39
@TedLyngmo: did as you suggested, and it worked perfect. thx. – tannic Nov 24 '19 at 16:52

Problems with Copy and assignment constructor

1 Answers1

Linked