How to return a List in C++

Question

Let's say I have a List class:

class List
{
    private:
        class Node{
            public:
                int data;
                Node* next;
            public:
                virtual ~Node()
                {
                    if (next != NULL)
                        delete next;
                }
        };

        Node* head;
    public:
        virtual ~List()
        {
            if (head != NULL)
            {
                delete head;
            }
        }
    public:
        void AddNode(int data);
        void DeleteNode(int data);
        //....  
};

Now I want to implement a function, which takes two List reference as arguments and return a new created List:

List SumTwoList(List& list_1, List& list_2)
{
    //here I create a new List list_3 and add some elements to it based on list_1 and list_2(add operation use dynamic allocation).

    //finally I want to return this list_3.
    return list_3;
}

I am confusing here. How should I create this list_3 inside SumTwoList(). If I just make it a local variable, when function returns, the destructor of list_3 will free all the nodes added to the list and destroy the whole list. However, if I do dynamic allocation to create this list_3 and return a pointer to it, it is the user's responsibility to delete this list after using.

I don't know how to choose from these two ideas and I'm pretty sure there are some much better methods to solve this problem. Thanks ahead for your advice. :-)

Answer 1

One option is to return a pointer to a new List

List * SumTwoList(List& list_1, List& list_2)
{
    List * pResultList = new List;

    // Iterate over all elements of list_1 and add (append) them to pResultList
    // Iterate over all elements of list_2 and add (append) them to pResultList

    return pResultList;
}

However, it has the drawback that the caller have to remember to delete the returned object.

Rather, it might be better to design the API slightly differently where elements of list_2 are append to the existing list, for example

// Append contents of rhs to this. rhs stays as is
List::append( List const & rhs );

Callers may call it as

List list_1;
// work on list_1, such as add elements
List list_2;
// work on list_2, such as add elements

list_1.append( list_2 );

This is close to std::list::splice(), but not entirely. splice() moves the elements from rhs to this, but append() just copies the elements.

Note, your destructor is buggy, it is delete'ing only thehead, the later elements are leaked.

A sketch for destructor implementation

List::~List() {
    while( head ) {
        struct node * oldHead = head;
        head = head->next;
        delete oldHead;
    }
    head = NULL;
}

Answer 2

Add a copy constructor and assignment operator (and fix your destructor):

class List
{
    private:
        typedef struct node{
            int data;
            struct node* next;
        }NODE;

        NODE* head;
    public:

    virtual ~List()  // Please fix this, as others have mentioned!
    { }
    List(const List& rhs) 
    {
       // this needs to be implemented
    }
    List& operator = (const List& rhs) 
    {
       // this needs to be implemented
    }
    //...
};

The "this needs to be implemented" is what you need to fill in to get the copies to work.

So what does this buy you? First, you can now write functions in the form that you were attempting to do in your question. That is, you can return a List by value safely and without doing any further coding to dynamically allocate or delete a list.

List SumTwoList(List& list_1, List& list_2)
{
   List list_3;
   // do stuff to add data to list_3...
   //...
   //finally I want to return this list_3. 
   return list_3;
}

Second, say we want to copy a list to another list. That can be done easily using the "=" or by simple copy construction

If you don't want to copy, then turn copying off by making copy-ctor and assignment op private and unimplemented*, otherwise copying will be an option to the programmer using your class and the compiler will also make copies when it needs to.

In other words, if copying is implemented correctly, then a simple program like this should work:

int main()
{
   List lis1;
   // Add some nodes to lis1...
   //...
   // Assume that lis1 now has nodes...complete the copying test
   List lis2(lis1);
   List lis3;
   lis3 = lis1;
}

The program above should have no memory leaks, and no crashes, even when main() returns. If not, then copying and/or destruction is broken.

*C++11 allows you to make the copy constructor and assignment operator disabled in an easier way than with pre C++11 code by using the delete keyword when defining the functions.

Answer 3

You will return a copy of a list. This will do what you expect if copy constructor and assignment operator of class List will be implemented.

class List
{
    private:
    //...
    public:
        List();
        List( const List& other);
        List& operator= ( const List& other);
    //...
};

Maybe better option would be however to create a constructor which takes two Lists and constructs a new one by merging them ( doing all this what SumTwoList would do):

class List
{
    private:
    //...
    public:
        List( const List& first, const List& second) { // similar to SumTwoList
        }
    //...
};