Merge two already sorted linked list

This is code to merge two sorted linked lists. l1 and l2 are sorted. While calling the merge function via l3, I am passing l3 as an object by reference in that function. The code is running fine, but can someone suggest how I can code better, with regards to dealing with class objects and passing them in a function?

```
#include
using namespace std;

class linklist{
private:
struct node{
int data;
node* link;
}*start;

public:
linklist(){
start=NULL;
}
void append(int);
void display();
void merge(linklist, linklist,linklist &);
~linklist()
{
node *q;
while(start!=NULL)
{
q=start->link;
delete start;
start=q;
}
}
};
void linklist::append(int num){

node temp, r;
temp=start;
if(start==NULL)
{temp=new node;
temp->data=num;
temp->link=NULL;
start=temp;
}
else{

while(temp->link!=NULL)
{
temp=temp->link;
}
r= new node;
r->data=num;
r->link=NULL;
temp->link=r;
}
}
void linklist::display() {
node *temp;
temp=start;
coutdatalink;
}
coutdata > q->data)
{
dat=q->data;
l3.append(dat);
q=q->link;
}

else{
dat=p->data;
l3.append(dat);
p=p->link;
}
}
if(p==NULL)
{
while(q!=NULL)
{
dat=q->data;
l3.append(dat);
q=q->link;
}
}
else{
while(p!=NULL)
{
dat=p->data;
l3.append(dat);
p=p->link;
}
}
cout<<"lists merged"<<endl;

}

int main()
{
linklist l1,l2,l3;

l1.append(2);
l1.append(5);
l1.append(23);
l1.append(34);
l1.append(45);

l2.append(6);
l2.ap

Here are some things that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Use nullptr rather than NULL

Modern C++ uses nullptr rather than NULL. See this answer for why and how it's useful.

Use more whitespace

Lines like this one:

cout<<"Linked List is"<<endl;

become much easier to read and understand with a little more whitespace:

cout << "Linked List is" << endl;

Use consistent formatting

The code seems to have inconsistent indentation and inconsistent placement of brackets {}. It doesn't matter as much which convention you use as much as it matters that you follow some convention. Doing so makes your code easier to read, understand and maintain.

Don't use std::endl if '\n' will do

Using std::endl emits a \n and flushes the stream. Unless you really need the stream flushed, you can improve the performance of the code by simply emitting '\n' instead of using the potentially more computationally costly std::endl. For instance, the line quoted above could be written like this:

std::cout << "Linked List is\n";

Prefer modern initializers for constructors

The constructor use the more modern initializer style rather than the old style you're currently using. Instead of this:

linklist(){
        start=NULL;
    }

you could use this:

linklist() : start{nullptr} {}

This uses the more consistent {} form for initializing start (and so requires a C++11 compiler) but this can be done using older C++99 if needed. There is not a significant difference in this code, but it's a good habit to get into using.

Use const where practical

The display member functions of linklist does not alter the underlying object and therefore should be declared const:

void display() const;

Make your class destructor virtual

If there's any chance that your class will be derived from, the class destructor should be virtual to avoid problems with collections of objects. See this question for more details on why. If it can't be derived from, enforce that by declaring the class final.

With that said, as @isanae has correctly pointed out, you shouldn't be left with the impression that those are the only options. For a plain old concrete class such as this one, you can reasonably just leave it as it is right now.

Return something useful from functions

The merge function takes two sorted linked lists and returns a new one; or it should. Right now it's declared like this

void linklist::merge(linklist l1, linklist l2,linklist& l3)

but it would make much more sense like this:

linklist linklist::merge(linklist l1, linklist l2)

Better still would be for the first list to be implicit so that one could write this:

l3 = l1.merge(l2);

To do that, I'd write the function like this:

linklist linklist::merge(const linklist &l2) const {
    linklist l3;
    node *nodes[2]{start, l2.start};

    while (nodes[0] && nodes[1]) {
        size_t index = nodes[0]->data data ? 0 : 1;
        l3.append(nodes[index]->data);
        nodes[index] = nodes[index]->link;
    }
    l3 += nodes[0];   // one of the lists is empty at this point
    l3 += nodes[1];
    return l3;
}

Note that this assumes the existence of an operator+= which might be implemented like this:

linklist &operator +=(const node *p) {
    while (p) {
        append(p->data);
        p = p->link;
    }
    return *this;
}

Use operator<< instead of display

It would be convenient to be able to redirect the output to something other than std::cout, so the usual idiom is to use something like this:

friend std::ostream& operatorlink) {
        out data << "  ";
    }
    return out << '\n';
}

Now we can use it within main like this:

std::cout << "finished merging\n" << l1 << l2 << l3;

Consider using a template

As the code is currently written, it can only store a single int as data, but with only a very minor change in the code, it could become a template. The first few lines could look like this:

template 
class linklist{
private:
    struct node{
        T data;
        node* link;

Thereafter, anywhere you specifically refer to an int for the data, replace it with the templated type T and you now have a generic container. For your int version, you can write this:

linklist l1;

Consider providing a std::initializer_list constructor

linklist(std::initializer_list list) : start{nullptr} {
    for (auto item : list) {
        append(item);
    }
}

Implementing all of the suggestions above give a main like this:

int main()
{
    linklist l1{2, 5, 23, 34, 45};
    linklist l2{6, 9, 35, 98};

    std::cout << "Before merging\n" << l1 << l2;
    auto l3 = l1.merge(l2);
    std::cout << "After merging\n" << l1 << l2 << l3;
}

Don't destroy pa

StackExchange Code Review Q#131435, answer score: 15