Doubly linked list assignment operator (withoud using std::swap)

Question

So I've got to implement doubly linked list. I've got constructor, destructor, many other methods like inserting too. But as I would like to apply to The Rule of Three I would need to implement copy assignment operator and a copy constructor. I've seen many solutions on the internet, but all of them required std::swap and std::move or std::copy. I cannot use any of those as all my algorithms have to be done without STL or boost::. I know I have to start like this:

list & list::operator=(const list & that)
{
    if (this != &that)
    {

    }
    return *this;
}

Then I would to something like this:

if (this != &that)
{
    this->clear();
    node *t = this->head;
    node *o = that.head;
    while (o->next)
    {
        t.append(o.value);
        o = o.next;
        t = t->next
    }
}
return *this;

@Edit:

void menu_list()
{
    char opt;
    int *t;
    list myList = list();
    do {
        display_menu("--- LISTA ---");
        opt = _getche();        
        std::cout << '\n';
        switch (opt) {
        case '1': //reading a list from a text file
        {
            int elements;
            std::ifstream file_stream;// 
            file_stream.open("data.txt"); // 
            if (!file_stream.good()) // 
            {
                std::cout << "Nie udalo sie wczytac pliku!" << '\n';
                break;
            }
            file_stream >> elements; // 
            t = new int[elements]; // stworzenie listy
            for (int i = 0; i<elements; i++)
            {
                file_stream >> *(t + i);    // 
            }
            myList = list(t, elements); //zainicjalizowanie listy
            delete[] t; //
            std::cout << "\nObecny stan listy: "; std::cout << myList.to_string() << '\n';
            break;
        }
        case '2': //deleting an element from a list
        {
            if (myList.is_empty())
            {
                std::cout << "Lista jest pusta! \n"; break;
            }
            int index;
            std::cout << "Podaj indeks elementu do usuniecia: ";
            scanf_s("%d", &index);
            if (myList.size() <= index)
            {
                std::cout << "Lista nie ma tylu elementow!\n"; break;
            }
            else if (index < 0)
            {
                std::cout << "Indeks nie moze byc mniejszy od 0!\n"; break;
            }
            myList.remove(index);
            std::cout << "\nObecny stan listy: "; std::cout << myList.to_string() << '\n';
            break;
        }

list.cpp (implementation of list header):

#include "list.h"
#include <ctime>


list::list() : head(nullptr), tail(nullptr), n(0)
{
}

list::list(int* t, int n) : head(nullptr), tail(nullptr), n(0)
{
    if (!this->is_empty())
    {
        this->clear();
    }
    for (int i = 0; i < n; i++)
    {
        this->append(t[i]);
    }
}

list::list(const list& obj)
{

}

clear() is a method that clears the whole list (and head and tail = nullptr) and append() is a method that appends a value at the end of a list.

Now, I guess this is wrong, but I don't have any clue how to make it work and I'd like to ask you guys how can I do it

What is difficult about just implementing your own `swap`? It is merely a 3 line function. And all `copy` does is a for-loop copying from source to destination. These aren't exactly the most difficult things to write yourself. — PaulMcKenzie, Mar 27 '17 at 04:44
`this != &that` Not a good idea. It's just an extra branch to slow down your program. You should trust the user to not do something as stupid as assigning an object to itself. — DeiDei, Mar 27 '17 at 04:47
@Frynio -- Your own `swap` doesn't need to do all that. Just swap the two values, forget about "move". — PaulMcKenzie, Mar 27 '17 at 04:51
@DeiDei *You should trust the user to not do something as stupid as assigning an object to itself.* -- Famous last words. A user's interaction with a class shouldn't break it. — PaulMcKenzie, Mar 27 '17 at 04:52
`o` is a pointer variable but you use different referencing operator with it. Your object is quite unclear! — Marco167, Mar 27 '17 at 04:53
Hmm, so can't I just `t->head = o.head` and `t->tail = o.tail`? — minecraftplayer1234, Mar 27 '17 at 04:53
@Frynio -- Your assignment operator is flawed starting from the very first line. Assuming you're deallocating memory from your object in the `clear()` method, and in the `append()` you're allocating memory with `new[]`. what if `new[]` throws an exception? Oops, your object is now corrupted since the data was previously destroyed. — PaulMcKenzie, Mar 27 '17 at 04:55
You are applying Rule of Three. Can we assume you have a working copy constructor? If so,check this out: http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom — user4581301, Mar 27 '17 at 04:56
But then shoul I write my own swap similair to this in C++98? — minecraftplayer1234, Mar 27 '17 at 07:58
@Frynio -- There is no need to mention what version of C++ you need to simply write a 3 line function: `void myswap(T& item1, T& item2) { T temp = item1; item1 = item2; item2 = temp; }` — PaulMcKenzie, Mar 27 '17 at 15:06
I've got that, but how can I use it in copy assignment operator? — minecraftplayer1234, Mar 27 '17 at 15:57
@Frynio -- `{ list temp(that); swap(temp.head, head); return *this; }` -- This uses the copy constructor and destructor to implement copy-assignment. That's why you should implement copy constructor and destructor first. The assignment operator then becomes practically fool-proof. — PaulMcKenzie, Mar 27 '17 at 16:37
@PaulMcKenzie: You're trying to implement copy assignment on top of `swap` on top of copy assignment. You can't do that; you need a different `swap` implementation. — user2357112, Mar 27 '17 at 18:02
Sorry, `myswap` should be called. But I don't see what is wrong with what was posted. The assignment operator is calling swap on `head`, which are pointers. The copy constructor doesn't (or not assumed to) use the assignment operator. — PaulMcKenzie, Mar 27 '17 at 18:05
But now I need copy constructor :D And I have no idea how can I write it — minecraftplayer1234, Mar 27 '17 at 18:06
@DeiDei: [Self-assignment is standard functionality.](https://isocpp.org/wiki/faq/assignment-operators) A type's test cases are probably the only place anyone will ever explicitly self-assign that type, but self-assignment still has to work properly to prevent nasty bugs in case of aliasing. — user2357112, Mar 27 '17 at 18:07
@PaulMcKenzie: `myswap` is also implemented on top of copy assignment. — user2357112, Mar 27 '17 at 18:07
@user2357112, I checked that. I need user-defined copy assignment operator, cuz I've got a menu with `switch`, where can I do sorts of operations on my list, and if I first create it, and then i try to f.e append to it, in the second case the list will be empty... — minecraftplayer1234, Mar 27 '17 at 18:09
@user2357112 Show us 1) Your `list` header. 2) Your `list` copy constructor, 3) Your `list` destructor. This is dragging on for too long -- might as well actually see these functions instead of assuming how they're written. — PaulMcKenzie, Mar 27 '17 at 18:14
@PaulMcKenzie: I'm talking about *your* copy assignment and *your* `myswap`. *You* wrote a copy assignment in terms of swapping - `{ list temp(that); swap(temp.head, head); return *this; }` - and a swap in terms of copy assignment - `void myswap(T& item1, T& item2) { T temp = item1; item1 = item2; item2 = temp; }`. — user2357112, Mar 27 '17 at 18:17
Yes, I know I wrote it. But what exactly is being swapped in this case? Pointers, i.e. `head`. It isn't the `list` itself that is being swapped in this case. — PaulMcKenzie, Mar 27 '17 at 18:19
...wait, you're swapping `temp.head` and `head`, not `temp` and `*this`. — user2357112, Mar 27 '17 at 18:19
I thought you were trying to do the standard copy-and-swap `swap(*this, other)` with a `swap` implemented in terms of copy assignment, but you're not quite doing that. — user2357112, Mar 27 '17 at 18:22
@Frynio -- You did not post the `list` header, and you didn't post the copy constructor. You posted basically irrelevant code. — PaulMcKenzie, Mar 27 '17 at 18:26
@Frynio -- No. Do **not** leave the copy constructor empty. If it really is empty, your code has bugs. The compiler will be calling this function whenever a copy is made, and it doesn't do anything. Once you open the doors to a user-defined copy constructor, you are required to implement it correctly. — PaulMcKenzie, Mar 27 '17 at 18:28
I checked, it doesn't call it anywhere, unless I want to use yours copy assignment operator, which I need of course, but I don't know how to write copy constructor — minecraftplayer1234, Mar 27 '17 at 18:30
@Frynio Forget about the assignment operator and just write the copy constructor. Why can't you write a copy constructor, when the only difference is that you are creating a brand new object from an existing object? All you would be doing is initializing the pointers to null and calling `append` in a loop, no different than the assignment operator (except `clear()` shouldn't need to be called). — PaulMcKenzie, Mar 27 '17 at 18:34
But f.e `myList = list(t, elements);` calls copy assignment operator, right? — minecraftplayer1234, Mar 27 '17 at 18:36
@Frynio Please just write the copy constructor for now. You are jumping ahead of the game. Once you have written the copy constructor, the assignment operator is simple. — PaulMcKenzie, Mar 27 '17 at 18:37

Doubly linked list assignment operator (withoud using std::swap)

0 Answers0