3

To put it simply, my compiler refuses to call the copy constructor and the assignment operator which I provided. I had a project for university to make a database-like program in C++ using architecture stratification(domain, repository, service and UI). Initially I was asked to use std::vector and I completed the program without any struggle. But now I have to replace std::vector with... a simply linked list I've pretty much finished writing the linked list and I've tested all of its functionalities. However, when I implement it in the program, replacing the std::vector, the compiler won't recognize the copy constructor and the assignment operator! Here is part of the Linked List implementation(without the Node class and ForwardIterator class which I've tested and don't seem to cause any issues)

template <class ElemType>
class LinkedList
{
    Node<ElemType> *front, *back;

public:

    LinkedList() : front{ NULL }, back{ NULL } {}

    //The problematic copy constructor
    LinkedList(LinkedList<ElemType>& lst) : LinkedList() {
        for (auto& elem : lst)
            push_back(elem);
    }

    ~LinkedList() {
        while (front != NULL)
            erase(begin());
    }

    //The problematic assignment operator
    LinkedList<ElemType>& operator=(LinkedList<ElemType>& lst) {
    while (front != NULL)
        erase(begin());
    for (auto& elem : lst)
        push_back(elem);
    return *this;
    }

I get the following errors multiple times:

E0334 class "LinkedList< Medicament >" has no suitable copy constructor

E0349 no operator "=" matches these operands

C2440 'initializing': cannot convert from 'LinkedList< Medicament >' to 'LinkedList< Medicament >'

C2679 binary '=': no operator found which takes a right-hand operand of type 'LinkedList< Medicament >' (or there is no acceptable conversion)

I would normally make the parameters and auto's const, but that gives me another error(no idea why), despite push_back() taking a const reference:

C2662 'ForwardIterator< Medicament > LinkedList< Medicament >::begin(void)': cannot convert 'this' pointer from 'const LinkedList< Medicament >' to 'LinkedList< Medicament > &'

The main errors(copy constructor and assignment) only happen when I pass the argument as a return value of a member function from within the service/repository/ui . A short example that causes this issue:

class MediRepo {                                    //repository

public:
    LinkedList<Medicament> elem;                    //I made it public for now

//............

    //Returns a copy of the medicine list
    LinkedList<Medicament> getMeds() { return elem; }
}

//main
MediRepo rep;
LinkedList<Medicament> medlist{ rep.getMeds() };    //ERROR E0334 and C2440
LinkedList<Medicament> medlist{ rep.elem };         //OK

Like I said, the linked lists(including the constructor and the assignment operator) work perfectly fine outside of the service/repository. If I spend more time, I can probably come up with a half-assed workaround to this, but I hope somebody can tell me what I am doing wrong that gives me these errors(the one with const parameters, too, if possible). I've spent way too many hours trying to solve this... Also, please ask if you would like to see more of the source code.

Jarod42
  • 203,559
  • 14
  • 181
  • 302
Răzvan Manea
  • 33
  • 1
  • 4
  • As the errors mention, copy constructors and assignment operators expect a `const` reference. – Drew Dormann Apr 18 '18 at 22:15
  • 1
    Off-topic, but your assignment operator is flawed on two levels. Self-assignment won't work correctly, and if something happens in `push_back` to throw an exception, you've corrupted your object by erasing the items. The [copy / swap](https://stackoverflow.com/questions/30857668/assignment-operator-in-linked-list-c) idiom makes this a 3 line function. – PaulMcKenzie Apr 18 '18 at 22:27
  • If you were already using the STL to begin with (via `std::vector`), why can't you use `std::list` or `std::forward_list` instead of implementing your own linked-list implementation? – Remy Lebeau Apr 18 '18 at 23:12
  • @RemyLebeau because the teacher's asked us to implement it ourselves... – Răzvan Manea Apr 18 '18 at 23:36
  • Those functions should take argument by const reference – M.M Apr 19 '18 at 08:31
  • When you are re-implementing something in `std`, a good start is declaring **exactly the same** members. You then just have to work out how to define them – Caleth Apr 19 '18 at 08:32

2 Answers2

4

This is not correct.

LinkedList(LinkedList<ElemType>& lst) : LinkedList()

Change it to this.

LinkedList(const LinkedList<ElemType>& lst) : LinkedList()

This is not correct.

LinkedList<ElemType>& operator=(LinkedList<ElemType>& lst)

Change it to this.

LinkedList<ElemType>& operator=(const LinkedList<ElemType>& lst)

Your temporaries cannot bind to mutable references.

Drew Dormann
  • 59,987
  • 13
  • 123
  • 180
  • That is where I get error C2662, it also gives me: C3536 '$L0': cannot be used before it is initialized and C2100 illegal indirection and C2664 'void LinkedList::push_back(const ElemType &)': cannot convert argument 1 from 'int' to 'const Medicament &' – Răzvan Manea Apr 18 '18 at 22:59
  • Wait, so I can't pass temporaries by reference, meaning I either have to pass the argument by value or to modify my code?` – Răzvan Manea Apr 18 '18 at 23:06
  • @RăzvanManea You pass temporaries by const reference. To fix the C2662 error, your `begin` function needs to be const as well (`begin() const`). – 1201ProgramAlarm Apr 18 '18 at 23:07
  • @1201ProgramAlarm awwwwww... damn, that was the issue へへ thanks a lot! – Răzvan Manea Apr 18 '18 at 23:42
0

Thanks @1201ProgramAlarm for helping me fix C2662. As they mentioned, I forgot to make the lists's begin() method const, which caused the said compile error at for (auto& elem : lst).

I was trying to get around this by setting the copy constructor's and the assignment operators' parameters to non-const, which caused the other errors when passing a temporary argument to a non-const reference, as @Drew Dormann mentioned.

That didn't happen when testing the LinkedList class because I didn't call the copy constructor with a temporary argument in the test's module. I'll remember to take greater care of const correctness from now on. Thank you for the support! o(_ _)o

Răzvan Manea
  • 33
  • 1
  • 4
  • I wonder why you didn't try std::forward_list. Was it your assignment to implement one basic container class? – Red.Wave Apr 19 '18 at 11:06