[RESOLVED] vector copy using new

Quote:

---

Originally Posted by Mybowlcut

Could it be that buttons is a vector of objects whilst elements is a vector of object pointers? Unfortunately I can't really post a working example as my code uses a third party library.

---

No, that can't be it. As the clone function returns a pointer, the target should be a vector of pointers. As you are using mem_fun_ref, the source should be a vector of objects. Check my example.

Quote:

---

Originally Posted by Mybowlcut

Code:

---

    // Update screen elements.
    test_screen.Update(event_);
    // Draw(blit) elements to screen.
    test_screen.Draw(DE.Get_Screen());
    // Draw screen.
    DE.Draw_Screen();

---

In the real problem, they're going to be stored in a vector of base pointers (Base_XImage) which itself is stored in my Screen class. From there, each time Update or Draw gets called as above, each element in the vector of base pointers will have Update and Draw called polymorphically. The class hierarchy looks like this:

Code:

---

Base_XImage
  XImage
  XButton

---

---

Wait, what is the vector of base pointers going to be used for? Just to have a list of drawing objects that need to be updated? If that's the case, what happens if someone alters the original object? Are you expecting to see that change in the "cloned" vector?

Viggy

Quote:

---

Originally Posted by MrViggy

Wait, what is the vector of base pointers going to be used for? Just to have a list of drawing objects that need to be updated? If that's the case, what happens if someone alters the original object? Are you expecting to see that change in the "cloned" vector?

Viggy

---

The vector of base pointers holds images, buttons, etc. that can be drawn and updated, yes. I don't understand what you mean by alters the original object.

Code:

---

void Screen::Load(const std::string& directory)
{
  std::ifstream image_file;
  IO::open_read_file(image_file, directory + IMAGES_FN);
 
  const std::vector<XImage>& images =
    IO::read_data<XImage, std::vector<XImage> >(image_file);

  /*std::transform(images.begin(), images.end(),
    std::back_inserter(elements), (std::mem_fun_ref(&Base_XImage::Clone)));*/

  std::vector<XImage>::const_iterator image_it = images.begin();
  for(; image_it != images.end(); ++image_it)
  {
    elements.push_back(new XImage(*image_it));
  }

  image_file.close();
  std::ifstream button_file;
  IO::open_read_file(button_file, directory + BUTTONS_FN);

  const std::vector<XButton>& buttons =
    IO::read_data<XButton, std::vector<XButton> >(button_file);

  /*std::transform(buttons.begin(), buttons.end(),
    std::back_inserter(elements), (std::mem_fun_ref(&Base_XImage::Clone)));*/

  std::vector<XButton>::const_iterator button_it = buttons.begin();
  for(; button_it != buttons.end(); ++button_it)
  {
    elements.push_back(new XButton(*button_it));
  }

  button_file.close();
  std::ifstream audible_button_file;
  IO::open_read_file(audible_button_file, directory + AUDIBLE_BUTTONS_FN);

  const std::vector<Audible_XButton>& audible_buttons =
    IO::read_data<Audible_XButton, std::vector<Audible_XButton> >(audible_button_file);

  /*std::transform(audible_buttons.begin(), audible_buttons.end(),
    std::back_inserter(elements), (std::mem_fun_ref(&Base_XImage::Clone)));*/

  std::vector<Audible_XButton>::const_iterator audible_button_it = audible_buttons.begin();
  for(; audible_button_it != audible_buttons.end(); ++audible_button_it)
  {
    elements.push_back(new Audible_XButton(*audible_button_it));
  }

  audible_button_file.close();
}

---

That's my code... it's appalling, I know. :rolleyes: I need to sort it out so there's no duplication, but I'm trying to show you what I'm doing. If read_data could return a vector of pointers to newed objects then I'd use that instead of this whole copying thing.

Code:

---

// Reads data from a file.
  //  - Requires that Object has an overloaded >> operator.
  //  - Returns a Container of Objects.
  template <typename Object, typename Container>
  Container read_data(std::istream& file)
  {
    Container v;

    std::copy(
      std::istream_iterator<Object>(file),
      std::istream_iterator<Object>(),
      std::back_inserter(v));

    return v;
  }

---

What I mean is that your vector of base pointers is a copy of the original object. That means if some other piece of code updates the original object, you won't be drawing the object correctly.

Having said that, it looks like your code is okay. The list of original objects (the ones you are making copies of) is destroyed anyway. So, nevermind. :)

Viggy

If I understand your problem correctly, the smartest thing to do is to read directly from the istream into a vector of pointers, something for which you would need a custom iterator. The following was quickly hacked based on the std::back_inserter in my STL implementation (thus I am not 100% sure it only uses the STL public interface and will run on any STL implementation):

Code:

---

template<typename _Container>
class clone_back_insert_iterator
  : public std::iterator<std::output_iterator_tag, void, void, void, void>
{
protected:
  _Container* container;
 
public:
  typedef _Container container_type;

  explicit
  clone_back_insert_iterator(_Container& __x) : container(&__x) { }
 
  template<typename _ValueType>
  clone_back_insert_iterator& operator=(_ValueType __value)
  {
    container->push_back(new _ValueType(__value));
    return *this;
  }

  clone_back_insert_iterator& operator*()
  { return *this; }

  clone_back_insert_iterator& operator++()
  { return *this; }

  clone_back_insert_iterator operator++(int)
  { return *this; }
};

template<typename _Container>
inline clone_back_insert_iterator<_Container> clone_back_inserter(_Container& __x)
{ return clone_back_insert_iterator<_Container>(__x); }

---

With this, you should be able to use std::copy to copy objects from any stream or container to a vector of (base class) pointers:

Code:

---

#include <vector>
#include <algorithm>
#include <functional>

class Base
{
public:
  virtual Base * clone() const = 0;
  virtual ~Base() {}
};

class Derived : public Base
{
public:
  virtual Derived * clone() const { return new Derived(*this); }
  virtual ~Derived() {}
};

int main()
{
  std::vector<Derived> v1(3);

  std::vector<Base*> v2;
  std::copy(v1.begin(), v1.end(),
            clone_back_inserter(v2));
}

---

Looks great, treuss. Where did you put that code? I should put it in my own header file (not in <istream>!), shouldn't I?

Quote:

---

Originally Posted by Mybowlcut

Looks great, treuss. Where did you put that code? I should put it in my own header file (not in <istream>!), shouldn't I?

---

In its own header file and probably into some namespace as well.

Ok.

I've got some errors from your code...

Code:

---

Error        1        error C4980: '__value' : use of this keyword requires /clr:oldSyntax command line option        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        19
Error        2        error C2327: 'Clone_Back_Insert_Iterator<_Container>::container' : is not a type name, static, or enumerator        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        16
Error        3        error C2065: '__x' : undeclared identifier        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        16
Error        4        error C3861: 'container': identifier not found        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        16
Error        5        error C2656: 'Clone_Back_Insert_Iterator<_Container>::clone_back_insert_iterator' : function not allowed as a bit field        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        16
Error        6        error C2071: 'Clone_Back_Insert_Iterator<_Container>::clone_back_insert_iterator' : illegal storage class        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        16
Error        7        error C4430: missing type specifier - int assumed. Note: C++ does not support default-int        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        16
Error        8        error C2143: syntax error : missing ';' before '&'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        19
Error        9        error C4430: missing type specifier - int assumed. Note: C++ does not support default-int        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        19
Error        10        error C2238: unexpected token(s) preceding ';'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        19
Error        11        error C2059: syntax error : '__value'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        19
Error        12        error C4430: missing type specifier - int assumed. Note: C++ does not support default-int        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        19
Error        13        error C2143: syntax error : missing ';' before '<end Parse>'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        19
Error        14        error C2334: unexpected token(s) preceding '{'; skipping apparent function body        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        26
Error        15        error C2143: syntax error : missing ';' before '&'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        28
Error        16        error C4430: missing type specifier - int assumed. Note: C++ does not support default-int        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        28
Error        17        error C4430: missing type specifier - int assumed. Note: C++ does not support default-int        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        29
Error        18        error C2988: unrecognizable template declaration/definition        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        29
Error        19        error C2059: syntax error : 'return'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        29
Error        20        error C2238: unexpected token(s) preceding ';'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        29
Error        21        error C2146: syntax error : missing ';' before identifier 'clone_back_insert_iterator'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        31
Error        22        error C2143: syntax error : missing ';' before '<'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        36
Error        23        error C2433: 'clone_back_insert_iterator' : 'inline' not permitted on data declarations        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        36
Error        24        error C4430: missing type specifier - int assumed. Note: C++ does not support default-int        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        36
Error        25        error C2988: unrecognizable template declaration/definition        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        36
Error        26        error C2059: syntax error : '<'        c:\documents and settings\user\my documents\visual studio 2005\projects\sdl_game_engine\sdl_game_engine\clone_back_insert_iterator.h        36

---

What's this?

Code:

---

template<typename _Container>
inline clone_back_insert_iterator<_Container> clone_back_inserter(_Container& __x)
{
        return clone_back_insert_iterator<_Container>(__x);
}

---

Also, if I were to summarise the class, what should I say? I've written this:

Code:

---

// Template class for dynamically creating/storing objects.

---

But it sounds too general...

Also... :rolleyes: I've got this function, and I copied it for Clone_Back_Insert_Iterator... just wondering if the second one (new one) is correct:

Code:

---

                // Reads data from a file.
        //        - Requires that Object has an overloaded >> operator.
        //        - Returns a Container of Objects.
        template <typename Object, typename Container>
        Container read_data(std::istream& file)
        {
                Container v;

                std::copy(
                        std::istream_iterator<Object>(file),
                        std::istream_iterator<Object>(),
                        std::back_inserter(v));

                return v;
        }

        // Same as read_data, except stores newed objects.
        template <typename Object, typename Container>
        Container read_data_clone(std::istream& file)
        {
                Container v;
                std::copy(
                        std::istream_iterator<Object>(file),
                        std::istream_iterator<Object>(),
                        Clone_Back_Insert_Iterator(v));
        }

---

Quote:

---

Originally Posted by Mybowlcut

Ok.

I've got some errors from your code...

---

As mentioned, the implementation is a copy & paste from my version of the STL. Start with removing all underscores at the beginning of variable names. I am also not sure, if the new iterator has to inherit the std::iterator, so if the inheritance gives you trouble, try removing it.

Quote:

---

Originally Posted by Mybowlcut

What's this?

Code:

---

template<typename _Container>
inline clone_back_insert_iterator<_Container> clone_back_inserter(_Container& __x)
{
        return clone_back_insert_iterator<_Container>(__x);
}

---

---

It's the function that allows you to write clone_back_inserter(v) instead of clone_back_insert_iterator<std::vector<Object *> >(v)

Quote:

---

Originally Posted by Mybowlcut

Also, if I were to summarise the class, what should I say? I've written this:

---

Insert iterator class that automatically allocates copies of the objects to be inserted on the heap.

Quote:

---

Originally Posted by Mybowlcut

Also... :rolleyes: I've got this function, and I copied it for Clone_Back_Insert_Iterator... just wondering if the second one (new one) is correct:

Code:

---

                // Reads data from a file.
        //        - Requires that Object has an overloaded >> operator.
        //        - Returns a Container of Objects.
        template <typename Object, typename Container>
        Container read_data(std::istream& file)
        {
                Container v;

                std::copy(
                        std::istream_iterator<Object>(file),
                        std::istream_iterator<Object>(),
                        std::back_inserter(v));

                return v;
        }

        // Same as read_data, except stores newed objects.
        template <typename Object, typename Container>
        Container read_data_clone(std::istream& file)
        {
                Container v;
                std::copy(
                        std::istream_iterator<Object>(file),
                        std::istream_iterator<Object>(),
                        Clone_Back_Insert_Iterator(v));
        }

---

---

As mentioned above, use clone_back_inserter(v) as third argument. Besides, there is the tiny inconvenience that you are copying into a local variable, creating a huge memory leak once the function finishes and v goes out of scope. But I'm sure you would have noticed that sooner or later ;)

Quote:

---

Originally Posted by treuss

As mentioned, the implementation is a copy & paste from my version of the STL. Start with removing all underscores at the beginning of variable names. I am also not sure, if the new iterator has to inherit the std::iterator, so if the inheritance gives you trouble, try removing it.

It's the function that allows you to write clone_back_inserter(v) instead of clone_back_insert_iterator<std::vector<Object *> >(v)
Insert iterator class that automatically allocates copies of the objects to be inserted on the heap.
As mentioned above, use clone_back_inserter(v) as third argument. Besides, there is the tiny inconvenience that you are copying into a local variable, creating a huge memory leak once the function finishes and v goes out of scope. But I'm sure you would have noticed that sooner or later ;)

---

Ah... yes... huge memory leak. Haha! Is this a correct... correction? :p

Code:

---

// Insert iterator class that automatically allocates copies
  // of the objects to be inserted on the heap.
  template <typename Object, typename Container>
  Container* read_data_clone(std::istream& file)
  {
    Container* v = new Container;

    std::copy(
      std::istream_iterator<Object>(file),
      std::istream_iterator<Object>(),
      Clone_Back_Inserter(v));

    return v;
  }

---

Or perhaps should I have a Container& as a parameter? Like I (think) I said, I'm fairly new to using new! Puns mania around here... :sick:

Ok. I've fixed up the code like you suggested:

Code:

---

#ifndef CLONE_BACK_INSERT_ITERATOR_H
#define CLONE_BACK_INSERT_ITERATOR_H

// Template class for dynamically creating/storing objects.
template<typename Container>
class Clone_Back_Insert_Iterator
  : public std::iterator<std::output_iterator_tag, void, void, void, void>
{
protected:
  Container* container;
 
public:
  typedef Container container_type;

  explicit
  Clone_Back_Insert_Iterator(Container& c) : container(&c) { }
 
  template<typename Element>
  Clone_Back_Insert_Iterator& operator=(Element e)
  {
    container->push_back(new Element(e));
    return *this;
  }

  Clone_Back_Insert_Iterator& operator*()
  { return *this; }

  Clone_Back_Insert_Iterator& operator++()
  { return *this; }

  Clone_Back_Insert_Iterator operator++(int)
  { return *this; }
};

template<typename Container>
inline Clone_Back_Insert_Iterator<Container> Clone_Back_Inserter(Container& c)
{
  return Clone_Back_Insert_Iterator<Container>(c);
}

#endif

---

Just wanting to get the final OK before I use it in my code. It compiles fine.

Cheers for your help so far... it has been very useful. :thumb:

Quote:

---

Originally Posted by Mybowlcut

Or perhaps should I have a Container& as a parameter?

---

Preferably. That way, the calling function can decide whether to create the container on the heap or on stack.

Quote:

---

Originally Posted by Mybowlcut

Just wanting to get the final OK before I use it in my code. It compiles fine.

---

Looks fine to me. I'm still not sure, if inheriting from std::iterator is necessary (or recommended). But there is certainly a is-a relationship, so why not if it works.

Quote:

---

Originally Posted by treuss

Preferably. That way, the calling function can decide whether to create the container on the heap or on stack.
Looks fine to me. I'm still not sure, if inheriting from std::iterator is necessary (or recommended). But there is certainly a is-a relationship, so why not if it works.

---

After several days of laziness and avoiding the whole thing, I got it to work. :) Thankyou! :thumb: