Why doesn't this list copy work?

Discussion in 'C++' started by Eric Lilja, Aug 11, 2005.

  1. Eric Lilja

    Eric Lilja Guest

    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
     
    Eric Lilja, Aug 11, 2005
    #1
    1. Advertising

  2. Eric Lilja wrote:
    > 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
     
    Jonathan Mcdougall, Aug 11, 2005
    #2
    1. Advertising

  3. Eric Lilja wrote:
    > 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
     
    n2xssvv g02gfr12930, Aug 11, 2005
    #3
  4. Eric Lilja

    Jeff Flinn Guest

    "n2xssvv g02gfr12930" <> wrote in message
    news:XgIKe.2428$...
    > Eric Lilja wrote:
    >> 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;
    >


    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
     
    Jeff Flinn, Aug 11, 2005
    #4
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. David Prowak

    Why oh why doesn't my data view work?

    David Prowak, Jan 30, 2004, in forum: ASP .Net
    Replies:
    1
    Views:
    734
    Alvin Bruney [MVP]
    Jan 30, 2004
  2. Alex
    Replies:
    2
    Views:
    1,233
  3. Mr. SweatyFinger

    why why why why why

    Mr. SweatyFinger, Nov 28, 2006, in forum: ASP .Net
    Replies:
    4
    Views:
    902
    Mark Rae
    Dec 21, 2006
  4. Mr. SweatyFinger
    Replies:
    2
    Views:
    1,963
    Smokey Grindel
    Dec 2, 2006
  5. Replies:
    26
    Views:
    2,119
    Roland Pibinger
    Sep 1, 2006
Loading...

Share This Page