C++ How to properly copy container(vector) of pointers?
Asked Answered
C

2

6

A few times I've stumbled across the scenario where I have a container of pointers that needs to be copied.

Let's say we have the following class hierarchy:

  • Student (base class)

    • Freshman (subclass)
    • Sophmore (subclass)
    • Junior (subclass)
    • Senior (subclass)
  • StudentService

The StudentService class has a std::vector<Student*> students field and the following constructor:

StudentService::StudentService(std::vector<Student*> students) {
   // code
}

It won't be correct to just use the std::vector::operator= operator and write this->students = students, because that will only copy the pointer addresses and so if someone from the outside deletes the objects pointed to by those pointers, then the StudentService class would suffer as a result.

The solution is to loop through each pointer in the students parameter and create a new dynamic object, something like this:

for(int i = 0; i < students.size(); i++) {
   this->students.at(i) = new Student(*students.at(i));
}

But even that is not proper due to the fact that it will create ONLY Student objects. And we know that a Student can be a Freshman, Sophmore, Junior or Senior. So here is my question: what's the best solution to this problem?

I guess one way would be to place a private enum field inside each Student class and have 4 if-else statements checking what type of Student it is and then creating a new dynamic object based on that like so:

 for(int i = 0; i < students.size(); i++) {
   if(students.at(i).getType() == FRESHMAN) {
      this->students.at(i) = new Freshman(*students.at(i));
   } else if(students.at(i).getType() == SOPHMORE) {
      this->students.at(i) = new Sophmore(*students.at(i));
   } else if {
   // and so on...
   }
}

But this still seems quite cumbersome, so what would you suggest?

Carin answered 10/10, 2017 at 15:15 Comment(4)
Forget the vector. How would you get create a Freshman from a Student pointer? That's the question you need to search on SO.Perzan
boost.org/doc/libs/1_65_1/doc/html/poly_collection.htmlVolute
Are students cloned at your university because they are "used" at different places?Camp
You could use smart pointers (in this case std::shared_ptr) to ensure objects stay alive even if one copy is destroyed.Stipule
M
8

You're looking for the Clone pattern. Add a clone() virtual function to Student, which is overridden in each descendant and creates the appropriate copy. Then write deep copy of containers as you've correctly specified.

Edit: my work assumption is that your Freshman, etc. classes are descending from Student. If not, use a variant<> and apply a copy visitor.

Milksop answered 10/10, 2017 at 15:17 Comment(2)
Here's a good example #5149206Coseismal
Thanks! This was exactly what I needed.Carin
P
4

Resolving ownership issues

If you deem to shared the Student-s between your modules, then you are facing an ownership issue, and I would recommend using vector of std::shared_ptr<Student>-s to solve it.

If you have a std::vector<std::shared_ptr<Student>>, you can pass it to anyone you want. The receiver can copy the vector using an assignment operator and later on any objects he adds/removes won't affect the original container, just like you seem to desire.

Resolving cloning issues

If you are interested in each module having its own copy of a Student-s vector, you are facing a cloning issue.

You could resolve it by adding the following method to your classes:

class Student {
    [..]
    virtual Student * clone() const = 0; // Assuming Student is abstract, otherwise implement as well
};

class Freshman : public Student {
    [..]
    virtual Freshman * clone() const { return new Freshman(*this); }
};

// Same for other derived classes...

And then using std::transform to copy the vector:

// students is the original std::vector<Student *>
std::vector<Student *> copy(students.size());
std::transform(students.begin(), students.end(), copy.begin(), [](Student * s) -> Student * { return s->clone(); });

BTW, it's Sophomore, not Sophmore...

Projection answered 10/10, 2017 at 15:22 Comment(10)
I don't think this is what the OP wants. Using std::shared_ptr will give you a shallow copy when I believe the OP wants a deep copy.Stefaniestefano
@NathanOliver, according to the OP: "because that will only copy the pointer addresses and so if someone from the outside deletes the objects pointed to by those pointers, then the StudentService class would suffer as a result". And this will definitely protect against it.Projection
Yes it makes it safe but now both vectors still point to the same object. I believe the OP wants both vector to have their own objects they point to.Stefaniestefano
@Stefaniestefano It's not clear that that's what he wants. The only problem he stated explicitly was deletion of objects that are still in use. Creating a deep copy seems to be one of his suggested solutions, but is not necessarily the goal in itself. Unless he clarifies we can't really know. And using raw pointers the way he's doing is generally a bad idea anyway, so if he doesn't use shared_ptr he should probably at least be using unique_ptr...Cottar
I'm with Nathan, because otherwise, StudentServices would take a reference to the array. There would be no reason to copy anything (including pointers) if you want it to refer to the same objects.Coseismal
@NathanOliver, I added a cloning solution as wellProjection
@KennyOstrom, I added a cloning solution as wellProjection
Please, do not do this... ever, basically. This is an anti-pattern. You basically never need to write an if-chain of dynamic_cast<>() calls: it's slow, it can be replaced by a virt. fn (which can ve replaced by a visitor, which will lead to a variant), you do not subscribe to new classes being added to the hierarchy, the result might depend on the order of tests, etc. Even a switch() on an enum is better (in terms of documening the code, maintaining it and in terms of performance). TL;DR: this is not Java but C++Milksop
@lorro, I believe it is better than using an enum inside the object, but as it's no longer crucial for my answer, so I omitted this part.Projection
Thanks :), looks way better now. Also, transform is nice :).Milksop

© 2022 - 2024 — McMap. All rights reserved.