Why doesn't this list copy work?

[Eric Lilja]

Hello!
Consider the following complete program (with sample output). There's
something wrong with my call to std::copy() in the copy constructor of
the Unit class. It fails to copy the list. I could solve it by looping
manually through the argument's list and call push_back for each
element, but I'd like to know what's wrong with my std::copy() call.
Here's the code with sample output:
#include <algorithm> /* std::copy() */
#include <iostream>
#include <list>

class Unit
{
public:
Unit(int n)
{
m_list.push_back(n);
}

Unit(const Unit& u)
{
/* This call fails to copy the list. */
std::copy(u.m_list.begin(), u.m_list.end(), m_list.begin());
}

void add_to_list(int n)
{
m_list.push_back(n);
}

void print_list() const
{
std::list<int>::const_iterator itr = m_list.begin();

while(itr != m_list.end())
{
std::cout << *itr << std::endl;
++itr;
}
}

private:
std::list<int> m_list;
};

int
main()
{
Unit u1(4711);

u1.add_to_list(123);
u1.add_to_list(456);

Unit u2(u1);

std::cout << "Printing u1:" << std::endl;
u1.print_list();

std::cout << "Printing u2:" << std::endl;
u2.print_list();

return 0;
}


Sample output:
$ ./copyctor.exe
Printing u1:
4711
123
456
Printing u2:

Thanks for any replies!

/ E

 

[Jonathan Mcdougall]

Hello!
Consider the following complete program (with sample output). There's
something wrong with my call to std::copy() in the copy constructor of
the Unit class. It fails to copy the list. I could solve it by looping
manually through the argument's list and call push_back for each
element, but I'd like to know what's wrong with my std::copy() call.

You can't copy something if the destination does not have enough space.
In your case, the Unit being copy-constructed is empty. You *must* use
insert() or push_back() on the list.

You have two solutions: either a manual loop or an iterator adaptor.

Here's the code with sample output:
#include <algorithm> /* std::copy() */
#include <iostream>
#include <list>

class Unit
{
public:
Unit(int n)
{
m_list.push_back(n);
}

Unit(const Unit& u)
{
/* This call fails to copy the list. */
std::copy(u.m_list.begin(), u.m_list.end(), m_list.begin());

for (std::list<int>::iterator itor=u.m_list.begin();
itor!=u.m_list.end(); ++itor)
{
u.push_back(*itor);
}

or

std::copy(u.m_list.begin(), u.m_list.end(),
std::back_inserter(m_list));

}

void add_to_list(int n)
{
m_list.push_back(n);
}

void print_list() const
{
std::list<int>::const_iterator itr = m_list.begin();

while(itr != m_list.end())
{
std::cout << *itr << std::endl;
++itr;
}
}

private:
std::list<int> m_list;
};

int
main()
{
Unit u1(4711);

u1.add_to_list(123);
u1.add_to_list(456);

Unit u2(u1);

std::cout << "Printing u1:" << std::endl;
u1.print_list();

std::cout << "Printing u2:" << std::endl;
u2.print_list();

return 0;
}


Sample output:
$ ./copyctor.exe
Printing u1:
4711
123
456
Printing u2:

Jonathan

 

[n2xssvv g02gfr12930]

Hello!
Consider the following complete program (with sample output). There's
something wrong with my call to std::copy() in the copy constructor of
the Unit class. It fails to copy the list. I could solve it by looping
manually through the argument's list and call push_back for each
element, but I'd like to know what's wrong with my std::copy() call.
Here's the code with sample output:
#include <algorithm> /* std::copy() */
#include <iostream>
#include <list>

class Unit
{
public:
Unit(int n)
{
m_list.push_back(n);
}

Unit(const Unit& u)
{
/* This call fails to copy the list. */
std::copy(u.m_list.begin(), u.m_list.end(), m_list.begin());

Use the following instead. Well implemented classes like STL will
implement safe copy constructors.

m_list = u.m_list;

}

void add_to_list(int n)
{
m_list.push_back(n);
}

void print_list() const
{
std::list<int>::const_iterator itr = m_list.begin();

while(itr != m_list.end())
{
std::cout << *itr << std::endl;
++itr;
}
}

private:
std::list<int> m_list;
};

int
main()
{
Unit u1(4711);

u1.add_to_list(123);
u1.add_to_list(456);

Unit u2(u1);

std::cout << "Printing u1:" << std::endl;
u1.print_list();

std::cout << "Printing u2:" << std::endl;
u2.print_list();

return 0;
}


Sample output:
$ ./copyctor.exe
Printing u1:
4711
123
456
Printing u2:

Thanks for any replies!

/ E

vector u2 is not updated by std:copy() hence it will still appear empty!
The copy did work and is dangerous in this case as you need to be sure
enough memory is availble to copy to!
To copy the contents of a vector to another just use an assignment as
outlined below.

std::vector<int> a,b;
..
..
// initiase a with some values
..
..
b = a;

John B

 

[Jeff Flinn]

Use the following instead. Well implemented classes like STL will
implement safe copy constructors.

m_list = u.m_list;

Better yet:

Unit(const Unit& u):m_list(u.m_list){}

and your initializer list should contain all of your class' members in the
order they are declared in your clas declaration.

Jeff