concatenate two list

[beginner20]

Hi I'm trying to concatenate two list as above but the print works well for the 2 lists and not work for the final(it shows empty list).
Can anyone help me?
Thanks in advance

MAIN
#include <iostream>
#include <stddef.h>
#include "Lista.h"

using namespace std;

int main() {
char a;
int x;
Lista l;
Lista m;
Lista n;

while(a!='e'){//manage first list
cout<<"Press 'i' to insert number,'s' to print list, 'd' to back to menu"<<endl;
cin>>a;
if(a=='i'){
cout<<"insert number";
cin>>x;
l.insert(x);}
if(a=='s')
l.stampa();
}

while(a!='d'){//manage second list
cout<<"Press 'i' to insert number,'s' to print list, 'd' to back to menu"<<endl;
cin>>a;
if(a=='i'){
cout<<"insert number";
cin>>x;
m.insert(x);}
if(a=='s')
m.stampa();
}

n.concatenate(l,m,n);//concatenate them
n.stampa();//print concatenated list

return 0;
}

header

#ifndef LISTA_H_
#define LISTA_H_
#include <stddef.h>
namespace std{
class Lista{
private:
struct nodo{
nodo *next ;
int num ;
};
public:
nodo *testa;
nodo *temp;

Lista(){
testa = NULL;
temp = NULL;
}
void insert(int);
void stampa();
void concatenate(Lista, Lista, Lista);
};
}
#endif /* LISTA_H_ */

cpp class

#include "Lista.h"
#include <iostream>
using namespace std;

void Lista::insert(int n){
nodo *inserimento = new nodo;
inserimento->num = n;
inserimento->next = NULL;
if(testa==NULL){
testa = inserimento;
testa->next = NULL;
}
else{
if(testa->num > inserimento->num){
inserimento->next = testa;
testa = inserimento;
}
else{
nodo *previous = new nodo;
temp = testa;
while(temp != NULL && temp->num <= inserimento->num) {
previous = temp;
temp = temp->next;
}
inserimento->next = temp;
previous->next = inserimento;
}
}}

void Lista::concatenate(Lista l, Lista m, Lista n){
l.temp=l.testa;
m.temp=m.testa;
while(l.temp!=NULL){
n.insert(l.temp->num);
l.temp=l.temp->next;
}
while(m.temp!=NULL){
n.insert(m.temp->num);
m.temp=m.temp->next;
}
}

void Lista::stampa(){
temp = testa;
if(testa==NULL){
cout<<"\nLista vuota\n";
}else{
cout<<"\nLista ordinata:\n";
while(temp!=NULL){
cout<<temp->num<<endl;
temp = temp->next;
}
}}

[ne555]

my comments are enclosed in /**/

#include <iostream>
#include <stddef.h>
// header

#ifndef LISTA_H_
#define LISTA_H_
#include <stddef.h>
namespace std
{
class Lista
{
	private:
	struct nodo
	{
		nodo* next;
		int num;
	};

	public:
	nodo* testa;
	nodo* temp; /*limit the scope of your variables*/

	Lista ()
	{
		testa = NULL; /*use initialization list instead*/
		temp = NULL;
	}
	void insert (int);
	void stampa (); /*if a member function does not change the object, declare it const*/
	void concatenate (Lista, Lista, Lista); /*¿why is this a _member_ function?*/
};
}
#endif /* LISTA_H_ */

// cpp class

//#include "Lista.h"
#include <iostream>
using namespace std;

void Lista::insert (int n)
{
	nodo* inserimento = new nodo;
	inserimento->num = n; /*create a constructor for nodo*/
	inserimento->next = NULL;
	if (testa == NULL)
	{
		testa = inserimento;
		testa->next = NULL;
	}
	else
	{
		if (testa->num > inserimento->num) /*ah, the list is sorted, ¿why didn't you mention it?*/
		{
			inserimento->next = testa;
			testa = inserimento;
		}
		else
		{
			nodo* previous = new nodo; /*memory leak*/
			temp = testa;
			while (temp != NULL && temp->num <= inserimento->num)
			{
				previous = temp;
				temp = temp->next;
			}
			inserimento->next = temp;
			previous->next = inserimento;
		}
	}
}

void Lista::concatenate (Lista l, Lista m, Lista n) /*parameters passed by copy*/
{
	l.temp = l.testa;
	m.temp = m.testa;
	while (l.temp != NULL)
	{
		n.insert (l.temp->num); /*modifying the local parameter*/
		l.temp = l.temp->next;
	}
	while (m.temp != NULL)
	{
		n.insert (m.temp->num); /*modifying the local parameter*/
		m.temp = m.temp->next;
	}
}

void Lista::stampa ()
{
	temp = testa;
	if (testa == NULL)
	{
		cout << "\nLista vuota\n";
	}
	else
	{
		cout << "\nLista ordinata:\n";
		while (temp != NULL)
		{
			cout << temp->num << endl;
			temp = temp->next;
		}
	}
}


using namespace std;

int main ()
{
	char a; /*use meaningful names for your variables*/
	int x;
	Lista l;
	Lista m;
	Lista n;

	while (a != 'e') /*does not correspond with the text*/
	{ // manage first list
		cout << "Press 'i' to insert number,'s' to print list, 'd' to back to menu" << endl;
		cin >> a;
		if (a == 'i')
		{
			cout << "insert number"; /*please put an space after*/
			cin >> x;
			l.insert (x);
		}
		if (a == 's') l.stampa ();
	}

	while (a != 'd') /*almost same that above*/
	{ // manage second list
		cout << "Press 'i' to insert number,'s' to print list, 'd' to back to menu" << endl;
		cin >> a;
		if (a == 'i')
		{
			cout << "insert number";
			cin >> x;
			m.insert (x);
		}
		if (a == 's') m.stampa ();
	}

	n.concatenate (l, m, n); // concatenate them
	n.stampa (); // print concatenated list

	return 0;
}

next time, if your program requires input, provide an example input and expected output

[beginner20]

Thanks you for the answer but my problem still exists I dont understand when you say "use inizialization list instead" in which I'm wrong?! Now the concatenate function is in the main directly. The i\o expexted is as example insert 1, insert 3, in first list and insert 2 in second, the concatenated shuold be 1 2 3

[ne555]

About initialization list

Lista ()
{
	// at this point all the members were already constructed (using the default constructor)
	testa = NULL; //this is assignment
	//...
}
Lista():
	testa(NULL) //initialization (construction)
{
	//...
}

Now there is no difference, but consider the case when you have a member variable that does not have a default constructor. Your approach will fail, you need to use initialization lists. The same can be said when you have inheritance and you need to construct your base object, you need to use initialization lists.
So better learn them now.


> Now the concatenate function is in the main directly.
You could have simply passed the parameter by reference
void concatenate (Lista l, Lista m, Lista &n);

Also, note that your current copy constructor and assignment operator (provided by the compiler) are performing a shallow copy instead of a deep copy.
You'll need to code a destructor too or you'll have memory leaks.


> The i\o expexted is as example insert 1, insert 3, in first list and insert 2 in second, the concatenated shuold be 1 2 3
Your code has a (bothersome) menu. I lose too much time with it, when just want to test your program.
What I want is something like
example input:

i 1
i 3
s
d
i 2
s
d

expected output:

1 3
2
1 2 3

current (erroneous) output:

1 3
2

[beginner20]

thanks but nevertheless the problem remains, and I can not understand why in the list n do not go to meet the elements of the other two

[ne555]

> but nevertheless the problem remains
show your updated code

[beginner20]

/*main*/
#include <iostream>
#include <stddef.h>
//#include "Lista.h"

using namespace std;
void concatenate(Lista, Lista, Lista);

void concatenate(Lista l, Lista m, Lista n){
	l.temp=l.testa;
	m.temp=m.testa;
	while(l.temp!=NULL){
		n.insert(l.temp->num);
		l.temp=l.temp->next;
		}
	while(m.temp!=NULL){
		n.insert(m.temp->num);
		m.temp=m.temp->next;
		}
}

int main() {
char a;
int x;
Lista l;
Lista m;
Lista n;

			while(a!='e'){//manage first list
				cout<<"Press 'i' to insert number,'s' to print list, 'e' to back to menu"<<endl;
				cin>>a;
				if(a=='i'){
					cout<<"\ninsert number\n";
					cin>>x;
					l.insert(x);}
				if(a=='s')
					l.stampa();
				}

			while(a!='d'){//manage second list
				cout<<"Press 'i' to insert number,'s' to print list, 'd' to back to menu"<<endl;
				cin>>a;
				if(a=='i'){
					cout<<"\ninsert number\n";
					cin>>x;
					m.insert(x);}
				if(a=='s')
					m.stampa();
				}

			concatenate(l,m,n);//concatenate them
			n.stampa();//print concatenated list

return 0;
}

/*header*/
#ifndef LISTA_H_
#define LISTA_H_
#include <stddef.h>
namespace std{
class Lista{
private:
	struct nodo{
		nodo *next ;
		int num ;
		};
public:
	nodo *testa;
	nodo *temp;

	Lista(){
		testa = NULL;
		temp = NULL;
		}
void insert(int);
void const stampa();

};
}
#endif /* LISTA_H_ */

/*class cpp*/
//#include "Lista.h"
#include <iostream>
using namespace std;

void Lista::insert(int n){
nodo *inserimento = new nodo;
inserimento->num = n;
inserimento->next = NULL;
    if(testa==NULL){
    testa = inserimento;
    testa->next = NULL;
    }
    else{
    	if(testa->num > inserimento->num){
    		inserimento->next = testa;
    		testa = inserimento;
    	}
    	else{
    	nodo *previous = new nodo;
    	temp = testa;
    	while(temp != NULL && temp->num <= inserimento->num) {
    		previous = temp;
    	    temp = temp->next;
    	}
    	inserimento->next = temp;
    	previous->next = inserimento;
    	delete[] previous;
    	}
    	}}

void const Lista::stampa(){
    temp = testa;
    if(testa==NULL){
        cout<<"\nLista vuota\n";
    }else{
        cout<<"\nLista ordinata:\n";
        while(temp!=NULL){
            cout<<temp->num<<endl;
            temp = temp->next;
        }
}}

[ne555]

You are still passing the parameters by copy.
You are not modifying `n' on main, but a copy of it. Pass by reference instead.
void concatenate(Lista, Lista, Lista&);


> void const stampa();
it should be void stampa(); const, but you cannot do that because you put the `temp' variable in a bigger scope than needed.
(stampa() modifies the state of the object)

> delete[] previous;
If you used new, then you need to delete
If you used new[], then you need to delete[]

However, at that point (line 110) it makes no sense, you'll break your list.
The error is doing nodo *previous = new nodo; in line 102, it serves no purpose and generates a leak. You could simply do nodo *previous = NULL;

[beginner20]

I've managed the problem. Thanks you.