Overloading + operator to concatenate 2 doubly lists

Question

I am trying to read 2 lists and concatenate them overloading + operator. I am pretty new in classes so please be patient with me, all I want is to learn. I overloaded the stream operators so I can read and write the objects normal and reversed . My + operator does concatenate the lists but the problems seems to be in main ( list1=list1+list2; or writing list3 -after concatenating list1 and list2 ). I looked almost everywhere on the internet for an answer and I ended posting this. If you have any links or tutorials that can make me grow faster and easier I am opened to new ideas. Thank alot for reading this and tryng to help ^^

Node Class:

class nod
{
public:
    int data;
    nod *next;
    nod *prev;
    friend class Lista;
    nod();
};

List Class

class Lista
{
private:
    nod *head;
    nod *tail;
    int size;
public:
    Lista();
    ~Lista();
    friend istream &operator >>(istream &input, Lista &a)
    {
        int i;
        cout << "Size of the list: ";
        input >> a.size;
        nod *q = new nod;
        q->prev = NULL;
        cout << "\nFrist node: ";
        input >> q->data;
        a.head = q;
        a.tail = a.head;
        for (i = 1; i < a.size; i++)
        {
            nod *q = new nod;
            cout << "\nNode number " << i + 1 << " is: ";
            input >> q->data;
            a.tail->next = q;
            q->prev = a.tail;
            a.tail = q;
            a.tail->next = NULL;
        }
        return input;
    }

    friend ostream& operator <<(ostream &output, Lista &a)
    {
        output << "\n\nNormal writing: ";
        nod *q = new nod;
        q = a.head;
        while (q != NULL)
        {
            output << q->data << " ";
            q = q->next;
        }
        q = a.tail;
        output << "\n\nReverse writing: ";

        while (q != NULL)
        {
            output << q->data << " ";
            q = q->prev;
        }
        return output;
    }
    Lista &operator+(Lista &rec)
    {
        Lista temp;
        temp.head = head;
        temp.tail = tail;
        temp.tail->next = rec.head;
        rec.head->prev = temp.tail;
        temp.tail = rec.tail;
        temp.size = size;
        temp.size = temp.size + rec.size;
        return temp;
    }
};

Main:

int main()
{
    Lista a,b,c;
    cin>>a;
    cin>>b;
    c=a+b;
    cout<<c;
    return 0;
}

Answer 1

Your operator+ is not even close to being a correct implementation. For starters, you are returning a reference to a local variable that goes out of scope upon exit, destroying the list and leaving the caller with a dangling reference to invalid memory. But worse, you are making that local variable point at the nodes of your input lists, and you are manipulating the various pointers within those lists, corrupting both lists.

operator+ needs to return a copy of the data from the two input lists. That means allocating a whole new set of nodes for the output list.

Try something more like this instead:

class nod
{
public:
    int data;
    nod *next;
    nod *prev;

    nod(int value = 0) :
        data(value), next(NULL), prev(NULL)
    {
    }
};

class Lista
{
private:
    nod *head;
    nod *tail;
    int size;

public:
    Lista() :
        head(NULL), tail(NULL), size(0)
    {
    }

    Lista(const Lista &src) :
        head(NULL), tail(NULL), size(0)
    {
        *this = src;
    }

    ~Lista()
    {
        Clear();
    }

    void Add(int value)
    {
        node *n = new nod(value);
        n->prev = tail;
        if (!head) head = n;
        if (tail) tail->next = n;
        tail = n;
        ++size;
    }

    void Clear()
    {
        nod *n = head;
        head = tail = NULL;
        size = 0;
        while (n)
        {
            nod *next = n->next;
            delete n;
            n = next;
        }
    }

    Lista& operator=(const Lista &rhs)
    {
        Clear();
        nod *n = rhs.head;
        while (n)
        {
            Add(n->data);
            n = n->next;
        }
        return *this;
    }

    Lista operator+(const Lista &rhs)
    {
        Lista temp;

        nod *n = head;
        while (n)
        {
            temp.Add(n->data);
            n = n->next;
        }

        n = rhs.head;
        while (n)
        {
            temp.Add(n->data);
            n = n->next;
        }
        return temp;
    }

    friend std::istream& operator >>(std::istream &input, Lista &a);
    friend std::ostream& operator <<(std::ostream &output, const Lista &a);
};

std::istream& operator >>(std::istream &input, Lista &a)
{
    // NOTE: this kind of code really does not
    // belong in an 'operator>>' implementation!

    int i, size, data;
    std::cout << "Size of the list: ";
    input >> size;
    for (i = 0; i < size; ++i)
    {
        std::cout << "\nNode number " << i + 1 << ": ";
        input >> data;
        a.Add(data);
    }
    return input;
}

std::ostream& operator <<(std::ostream &output, const Lista &a)
{
    output << "\n\nNormal writing: ";
    nod *n = a.head;
    while (n)
    {
        output << n->data << " ";
        n = n->next;
    }

    n = a.tail;
    output << "\n\nReverse writing: ";
    while (n)
    {
        output << n->data << " ";
        n = n->prev;
    }

    return output;
}

int main()
{
    Lista a, b, c;
    std::cin >> a;
    std::cin >> b;
    c = a + b;
    std::cout << c;
    return 0;
}

Now, with that said, you should seriously get rid of the manual linked-list implementation and use the STL std::list class instead, which manages a linked-list of nodes and knows how to make copies of itself:

#include <list>
#include <algorithm>
#include <iterator>

class Lista
{
private:
    std::list<int> data;

public:
    void Add(int value)
    {
        data.push_back(value);
    }

    void Clear()
    {
        data.clear();
    }

    Lista operator+(const Lista &rhs)
    {
        Lista temp;
        std::copy(data.begin(), data.end(), std::back_inserter(temp));
        std::copy(rhs.data.begin(), rhs.data.end(), std::back_inserter(temp));
        return temp;
    }

    friend std::istream& operator >>(std::istream &input, Lista &a);
    friend std::ostream& operator <<(std::ostream &output, const Lista &a);
};

std::istream& operator >>(std::istream &input, Lista &a);
{
    // NOTE: this kind of code really does not
    // belong in an 'operator>>' implementation!

    int i, size, data;
    std::cout << "Size of the list: ";
    input >> size;
    for (i = 0; i < size; ++i)
    {
        std::cout << "\nNode number " << i + 1 << ": ";
        input >> data;
        a.Add(data);
    }
    return input;
}

std::ostream& operator <<(std::ostream &output, const Lista &a)
{
    output << "\n\nNormal writing: ";
    for (std::list<int>::const_iterator iter = a.data.begin(), end = a.data.end(); iter != end; ++iter)
    {
        output << *iter << " ";
    }

    output << "\n\nReverse writing: ";
    for (std::list<int>::const_reverse_iterator iter = a.data.rbegin(), end = a.data.rend(); iter != end; ++iter)
    {
        output << *iter << " ";
    }

    return output;
}